From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id E18F5A0C4B; Fri, 13 Aug 2021 20:59:12 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7B4DE40DF4; Fri, 13 Aug 2021 20:59:12 +0200 (CEST) Received: from mail-lf1-f42.google.com (mail-lf1-f42.google.com [209.85.167.42]) by mails.dpdk.org (Postfix) with ESMTP id 3E6AA4067E for ; Fri, 13 Aug 2021 20:59:11 +0200 (CEST) Received: by mail-lf1-f42.google.com with SMTP id g13so21514138lfj.12 for ; Fri, 13 Aug 2021 11:59:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=k/2/Cta26gJdanKIsBEhT59T0gMM/p3cUrEXtLad188=; b=m9+kxasSFOn9NGr2wbR+kTsNPO4PAJZpGUsdK5vfXxsu9MIXOgL9s5ips5O0ubf8vw 2/KMzJ8rt42C4rDSyrZ5JgECRJLzY5GzKL0segpmJZHSIhRkLiKEUql3J7bGhqodWn9B GkRTc6eIu0JlbtOvdJbjrq3/Jt3nIYK63ql4wuYTL0UlYLO3AwsmGSvdFLm26UmX3YBn TeiwyvRwKQE14o0tU7lYgLrC5A4s8fG7i2OTNFP0VaALMk2LUkkKliLzruB9+KW+kzgF /yU7cdEZYXZ5ynuYXcv2ajKLeUS2xVBRGJUxDvhSCILP8Fq8jRNR/5j627uojENbsQp9 /JXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=k/2/Cta26gJdanKIsBEhT59T0gMM/p3cUrEXtLad188=; b=D0rFRJkMyLjRPkCubK0tlgaouNFjj6rvpZbeyeXhGTW3BWpRvzrI7LrECgbl3YLPwg HUoigD14a/V2sJrGA9OAiKaTrqSoLIZPomLZfkUg2JG7gkWFGhvtUhN/ueL8JAZZqumS DKNwbw7au6QtQJhFtJwWtTk4v4e0cJUWwEYy6Usj0woYzxd6aGQ25ELia3dugkVl8+F4 eCLzB63Eod+DrbBVhD1V4Uf6wjlz7cG6iSD6l4RdBamJWlwbCWIU8y1xak+IzCqbnw7d eF8HmyEC4Rhnehz6hNb8FYn2EtySBtTOxUz622Q2mldYrRCuG85oMe0Gx2jXKzt3qYFT lPIA== X-Gm-Message-State: AOAM533zG+FptlH/yEHDAtSynyuBh8JPaChTv1aiNX71Dchv+TOnytET pegdBTXwDT9kFomCQElHYfM= X-Google-Smtp-Source: ABdhPJyUe/Y4SpDUNCZ3xkVuW8WfdEaQEaJXiJKQKLDdUb5lBRyYqgoxx9qUqpTgLpGlkaYF2loX+A== X-Received: by 2002:ac2:58c5:: with SMTP id u5mr2527656lfo.603.1628881149284; Fri, 13 Aug 2021 11:59:09 -0700 (PDT) Received: from sovereign (broadband-37-110-65-23.ip.moscow.rt.ru. [37.110.65.23]) by smtp.gmail.com with ESMTPSA id x16sm222656lfa.244.2021.08.13.11.59.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Aug 2021 11:59:08 -0700 (PDT) Date: Fri, 13 Aug 2021 21:59:07 +0300 From: Dmitry Kozlyuk To: William Tu Cc: dev@dpdk.org, nick.connolly@mayadata.io, stephen@networkplumber.org Message-ID: <20210813215907.797e4553@sovereign> In-Reply-To: <20210813033627.78546-1-u9012063@gmail.com> References: <20210813010250.87528-1-u9012063@gmail.com> <20210813033627.78546-1-u9012063@gmail.com> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCHv5] eal: remove sys/queue.h from public headers. X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 2021-08-13 03:36 (UTC+0000), William Tu: > Currently there are some public headers that include 'sys/queue.h', which > is not POSIX, but usually provided by the Linux/BSD system library. > (Not in POSIX.1, POSIX.1-2001, or POSIX.1-2008. Present on the BSDs.) > The file is missing on Windows. During the windows build, DPDK uses a Typo: "Windows". > bundled copy, so building a DPDK library works fine. But when OVS or other > applications use DPDK as a library, because some DPDK public headers > include 'sys/queue.h', on Windows, it triggers an error due to no such file. > > One solution is to install the 'lib/eal/windows/include/sys/queue.h' into > Windows environment, such as [1]. However, this means DPDK exports the > functionalities of 'sys/queue.h' into the environment, which might cause > symbols, macros, headers clashing with other applications. > > The patch fixes it by removing the "#include " from > DPDK public headers, so programs including DPDK headers don't depend > on the system to provide 'sys/queue.h'. When these public headers use > macros such as TAILQ_xxx, we replace it with RTE_ prefix. "replace it by _the ones_ with RTE_ prefix"? > For Windows, we copy the definitions from to rte_os.h > under windows. Note that these RTE_ macros are compatible with "under windows" -> "in Windows EAL" > , only at the level of API (to use with "only" -> "both" > macros in C files) and ABI (to avoid breaking it). > > Additionally, the TAILQ_FOREACH_SAFE is not part of , > the patch replaces it with RTE_TAILQ_FOREACH_SAFE. > With this patch, all the public headers no longer have > "#include " or "TAILQ_xxx" macros. This is a repetition of what is stated in the previous paragraph. > > [1] http://mails.dpdk.org/archives/dev/2021-August/216304.html > > Suggested-by: Nick Connolly > Suggested-by: Dmitry Kozliuk > Signed-off-by: William Tu > --- > v4-v5 > * fix compile error due to drivers/net/ipn3ke/ipn3ke_flow.c:1234 > * run spell check 1. Please register at http://patchwork.dpdk.org with the email used for the patches and update the state of all previous versions to "Superseded". It is not currently done automatically and only you and a few maintainers can change the state. Patchwork also shows CI build failures with v5, they need to be fixed. 2. Are you using `git format-patch -v5 ...` to create patches? The subject of your patches is missing a space ("PATCH v5" vs "PATCHv5"). Not sure if tools like patchwork will properly process it. [...] > struct rte_afu_driver { > - TAILQ_ENTRY(rte_afu_driver) next; /**< Next afu driver. */ > + RTE_TAILQ_ENTRY(rte_afu_driver) next; /**< Next afu driver. */ > struct rte_driver driver; /**< Inherit core driver. */ > afu_probe_t *probe; /**< Device Probe function. */ > afu_remove_t *remove; /**< Device Remove function. */ Re: loss of comment alignment here and in other places. Firstly, it's definitely not a big deal. Current patch is good because it only changes relevant lines. Re-aligning all the comments would be worse IMO. However, in cases like this, when keeping alignment doesn't require changing neighboring lines, it could be kept. Just a nit. [...] > /* This macro permits both remove and free var within the loop safely.*/ > -#ifndef TAILQ_FOREACH_SAFE > -#define TAILQ_FOREACH_SAFE(var, head, field, tvar) \ > - for ((var) = TAILQ_FIRST((head)); \ > - (var) && ((tvar) = TAILQ_NEXT((var), field), 1); \ > +#ifndef RTE_TAILQ_FOREACH_SAFE > +#define RTE_TAILQ_FOREACH_SAFE(var, head, field, tvar) \ > + for ((var) = RTE_TAILQ_FIRST((head)); \ > + (var) && ((tvar) = RTE_TAILQ_NEXT((var), field), 1); \ > (var) = (tvar)) > #endif Why duplicate this in rte_os.h (documentation lost, BTW) and add #ifdef? RTE_TAILQ_FOREACH_SAFE is not needed in headers, it can be left here. > > diff --git a/lib/eal/linux/include/rte_os.h b/lib/eal/linux/include/rte_os.h > index 1618b4df22..1a6e5b789f 100644 > --- a/lib/eal/linux/include/rte_os.h > +++ b/lib/eal/linux/include/rte_os.h > @@ -11,6 +11,21 @@ > */ > > #include > +#include > + > +/* These macros are compatible with system's sys/queue.h. */ > +#define RTE_TAILQ_HEAD(name, type) TAILQ_HEAD(name, type) > +#define RTE_TAILQ_ENTRY(type) TAILQ_ENTRY(type) > +#define RTE_TAILQ_FOREACH(var, head, field) TAILQ_FOREACH(var, head, field) > +#define RTE_TAILQ_FOREACH_SAFE(var, head, field, tvar) \ Stray TAB here and in rte_os.h for other platforms.