Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: fix SPD no-match is misinterpreted
> Hi Akhil,
>
> >
> > On 3/28/2019 6:17 PM, Konstantin Ananyev wrote:
> > > acl_classify() returns zero value when no matching rule was found.
> > > Currently ipsec-secgw treats it as a valid SPI value, though it has
> > > to discard such packets.
> > > Error could be easily observed by sending outbound unmatched packets,
> > > user will see something like that in the log:
> > > IPSEC: No cryptodev: core 7, cipher_algo 0, auth_algo 0, aead_algo 0
> > >
> > > To fix it we need to treat packets with zero result from acl_classify()
> > > as invalid ones. Also we can change DISCARD and BYPASS values to
> > > simplify checks and save some extra space for valid SPI values.
> > spi value =0 is invalid but zero result may have a valid packet.
> > consider a case:
> > SPI = 128 or 256 or 512 and so on => sa_idx = 0 and result will come as
> > zero, and this would be a valid packet.
> >
> > I see that the sa_idx calculation logic is not correct in first place.
> > There will be multiple spi values for same sa_idx which is not correct.
> > So we have 2 issues here:
> > 1. result = 0, means sa_idx =0 which may be correct, but as you said if
> > acl_classify fails, it also return 0.
> > 2. SPI values which are IPSEC_SA_MAX_ENTRIES apart will have same sa_idx
> > and will keep on overwriting the previous ones.
> >
>
> Ok I see what you mean.
> The easiest fix for that (till we'll have proper SAD) would be not to allow
> SPIs bigger than IPSEC_SA_MAX_ENTRIES.
> Are you ok with that?
> Konstantin
After another thought, it seems that we can easily overcome that problem
without introducing extra limitations - just need to store 'sa_idx + 1' in acl
table.
Will give it a try with v2.
Konstantin
>
>
> > So I believe the fix in this patch is not enough to resolve these
> > issues. It will work on some values and will break on other values of spi.
> >
> > -Akhil
> >
> > >
> > > Fixes: 906257e965b7 ("examples/ipsec-secgw: support IPv6")
> > > Fixes: 2a5106af132b ("examples/ipsec-secgw: fix corner case for SPI
> > > value")
> > > Cc: sta...@dpdk.org
> > >
> > > Signed-off-by: Konstantin Ananyev
> > > ---
> > > examples/ipsec-secgw/ipsec-secgw.c | 12 ++--
> > > examples/ipsec-secgw/ipsec.h | 6 ++
> > > examples/ipsec-secgw/sp4.c | 11 ---
> > > examples/ipsec-secgw/sp6.c | 11 ---
> > > 4 files changed, 24 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/examples/ipsec-secgw/ipsec-secgw.c
> > > b/examples/ipsec-secgw/ipsec-secgw.c
> > > index ffbd00b08..59e084234 100644
> > > --- a/examples/ipsec-secgw/ipsec-secgw.c
> > > +++ b/examples/ipsec-secgw/ipsec-secgw.c
> > > @@ -438,11 +438,11 @@ inbound_sp_sa(struct sp_ctx *sp, struct sa_ctx *sa,
> > > struct traffic_type *ip,
> > > for (i = 0; i < ip->num; i++) {
> > > m = ip->pkts[i];
> > > res = ip->res[i];
> > > - if (res & BYPASS) {
> > > + if (res == BYPASS) {
> > > ip->pkts[j++] = m;
> > > continue;
> > > }
> > > - if (res & DISCARD) {
> > > + if (res == DISCARD) {
> > > rte_pktmbuf_free(m);
> > > continue;
> > > }
> > > @@ -453,7 +453,7 @@ inbound_sp_sa(struct sp_ctx *sp, struct sa_ctx *sa,
> > > struct traffic_type *ip,
> > > continue;
> > > }
> > >
> > > - sa_idx = ip->res[i] & PROTECT_MASK;
> > > + sa_idx = ip->res[i];
> > > if (sa_idx >= IPSEC_SA_MAX_ENTRIES ||
> > > !inbound_sa_check(sa, m, sa_idx)) {
> > > rte_pktmbuf_free(m);
> > > @@ -541,10 +541,10 @@ outbound_sp(struct sp_ctx *sp, struct traffic_type
> > > *ip,
> > > j = 0;
> > > for (i = 0; i < ip->num; i++) {
> > > m = ip->pkts[i];
> > > - sa_idx = ip->res[i] & PROTECT_MASK;
> > > - if (ip->res[i] & DISCARD)
> > > + sa_idx = ip->res[i];
> > > + if (sa_idx == DISCARD)
> > > rte_pktmbuf_free(m);
> > > - else if (ip->res[i] & BYPASS)
> > > + else if (sa_idx == BYPASS)
> > > ip->pkts[j++] = m;
> > > else if (sa_idx < IPSEC_SA_MAX_ENTRIES) {
> > > ipsec->res[ipsec->num] = sa_idx;
> > > diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
> > > index 99f49d65f..44daf384b 100644
> > > --- a/examples/ipsec-secgw/ipsec.h
> > > +++ b/examples/ipsec-secgw/ipsec.h
> > > @@ -41,10 +41,8 @@
> > > #define SPI2IDX(spi) (spi & (IPSEC_SA_MAX_ENTRIES - 1))
> > > #define INVALID_SPI (0)
> > >
> > > -#define DISCARD (0x8000)
> > > -#define BYPASS (0x4000)
> > > -#define PROTECT_MASK (0x3fff)
> > > -#define PROTECT(sa_idx) (SPI2IDX(sa_idx) & PROTECT_MASK) /* SA idx 30
> > > bits */
> > > +#defi
Re: [dpdk-dev] [RFC v2] net: fix rte_vlan_insert with shared mbuf
Unfortunately, I think the complete fix is more complicated than this.
Drivers that use rte_vlan_insert don't anticipate that the mbuf might
change and that (hardware) transmit can fail.
They make a copy of the mbuf pointer from the incoming transmit list
and don't update the original if rte_vlan_insert creates a new mbuf.
If transmit fails, the application needs to be the given the new mbuf
for re-transmission or freeing.
On 3/28/19 4:53 PM, Stephen Hemminger wrote:
If mbuf is shared then rte_vlan_insert() would clobber the original
Ethernet header. The changed version handles this by getting
an mbuf that will hold the new Ethernet and VLAN header followed
by another mbuf (cloned) for the data.
Fixes: c974021a5949 ("ether: add soft vlan encap/decap")
Signed-off-by: Stephen Hemminger
---
v2 - compile tested only, do copy/clone.
lib/librte_net/rte_ether.h | 37 -
1 file changed, 28 insertions(+), 9 deletions(-)
diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
index c2c5e249ffe9..5fc306e5d08c 100644
--- a/lib/librte_net/rte_ether.h
+++ b/lib/librte_net/rte_ether.h
@@ -374,10 +374,10 @@ static inline int rte_vlan_strip(struct rte_mbuf *m)
* Software version of VLAN unstripping
*
* @param m
- * The packet mbuf.
+ * Pointer to the packet mbuf.
* @return
* - 0: On success
- * -EPERM: mbuf is is shared overwriting would be unsafe
+ * -ENOMEM: could not allocate mbuf for header
* -ENOSPC: not enough headroom in mbuf
*/
static inline int rte_vlan_insert(struct rte_mbuf **m)
@@ -385,15 +385,34 @@ static inline int rte_vlan_insert(struct rte_mbuf **m)
struct ether_hdr *oh, *nh;
struct vlan_hdr *vh;
- /* Can't insert header if mbuf is shared */
- if (rte_mbuf_refcnt_read(*m) > 1) {
- struct rte_mbuf *copy;
+ /* Can't safely directly insert header if mbuf is shared or indirect */
+ if (!RTE_MBUF_DIRECT(*m) || rte_mbuf_refcnt_read(*m) > 1) {
+ struct rte_mempool *mp = (*m)->pool;
+ struct rte_mbuf *md, *mh;
+
+ mh = rte_pktmbuf_alloc(mp);
+ if (unlikely(mh == NULL))
+ return -ENOMEM;
+
+ mh->tx_offload = (*m)->tx_offload;
+ mh->vlan_tci = (*m)->vlan_tci;
+ mh->vlan_tci_outer = (*m)->vlan_tci_outer;
+ mh->port = (*m)->port;
+ mh->ol_flags = (*m)->ol_flags;
+ mh->packet_type = (*m)->packet_type;
- copy = rte_pktmbuf_clone(*m, (*m)->pool);
- if (unlikely(copy == NULL))
+ md = rte_pktmbuf_clone(*m, mp);
+ if (unlikely(md == NULL)) {
+ rte_pktmbuf_free(mh);
return -ENOMEM;
- rte_pktmbuf_free(*m);
- *m = copy;
+ }
+
+ mh->next = md;
+ mh->nb_segs = md->nb_segs + 1;
+ memcpy(rte_pktmbuf_append(mh, ETHER_HDR_LEN),
+ rte_pktmbuf_mtod(md, void *), ETHER_HDR_LEN);
+ rte_pktmbuf_adj(md, ETHER_HDR_LEN);
+ *m = mh;
}
oh = rte_pktmbuf_mtod(*m, struct ether_hdr *);
Re: [dpdk-dev] [dpdk-stable] [PATCH] test/eventdev: fix sprintf with snprintf
On Wed, 2019-03-13 at 10:35 -0400, Aaron Conole wrote:
>
> > For all cases I expect truncation trigger a functional error which
> > should be
> > already handled properly, like in this case 'rte_vdev_init()' will
> > fail in
> > second call if buffer is small.
>
> And give the user a bad error ("I said net_null1038123825, not
> net_null10
> - bug in dpdk!").
>
> > There may be cases to check the return value, but that should be
> > the case with
> > 'sprintf' as well, changing API to 'snprintf' shouldn't require
> > additional check
> > by default.
>
> I agree, that's true. I think it's the right thing to do here,
> though.
Aaron,
Are you expecting next version of this patch? Or I can merge this
patch?
Re: [dpdk-dev] [PATCH v4 1/9] mbuf: new function to generate raw Tx offload value
Hi Olivier,
> > Operations to set/update bit-fields often cause compilers
> > to generate suboptimal code.
> > To help avoid such situation for tx_offload fields:
> > introduce new enum for tx_offload bit-fields lengths and offsets,
> > and new function to generate raw tx_offload value.
> >
> > Signed-off-by: Konstantin Ananyev
> > Acked-by: Akhil Goyal
>
> I understand the need. Out of curiosity, do you have any performance
> numbers to share?
On my board (SKX):
for micro-benchmark (doing nothing but setting tx_offload for 1M mbufs in a
loop)
the difference is more than 150% - from ~55 cycles to ~20 cycles per iteration.
For ipsec-secgw - ~3% improvement for tunneled outbound packets.
>
> Few cosmetic questions below.
>
> > ---
> > lib/librte_mbuf/rte_mbuf.h | 79 ++
> > 1 file changed, 72 insertions(+), 7 deletions(-)
> >
> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > index d961ccaf6..0b197e8ce 100644
> > --- a/lib/librte_mbuf/rte_mbuf.h
> > +++ b/lib/librte_mbuf/rte_mbuf.h
> > @@ -479,6 +479,31 @@ struct rte_mbuf_sched {
> > uint16_t reserved; /**< Reserved. */
> > }; /**< Hierarchical scheduler */
> >
> > +/**
> > + * enum for the tx_offload bit-fields lenghts and offsets.
> > + * defines the layout of rte_mbuf tx_offload field.
> > + */
> > +enum {
> > + RTE_MBUF_L2_LEN_BITS = 7,
> > + RTE_MBUF_L3_LEN_BITS = 9,
> > + RTE_MBUF_L4_LEN_BITS = 8,
> > + RTE_MBUF_TSO_SEGSZ_BITS = 16,
> > + RTE_MBUF_OUTL3_LEN_BITS = 9,
> > + RTE_MBUF_OUTL2_LEN_BITS = 7,
> > + RTE_MBUF_L2_LEN_OFS = 0,
> > + RTE_MBUF_L3_LEN_OFS = RTE_MBUF_L2_LEN_OFS + RTE_MBUF_L2_LEN_BITS,
> > + RTE_MBUF_L4_LEN_OFS = RTE_MBUF_L3_LEN_OFS + RTE_MBUF_L3_LEN_BITS,
> > + RTE_MBUF_TSO_SEGSZ_OFS = RTE_MBUF_L4_LEN_OFS + RTE_MBUF_L4_LEN_BITS,
> > + RTE_MBUF_OUTL3_LEN_OFS =
> > + RTE_MBUF_TSO_SEGSZ_OFS + RTE_MBUF_TSO_SEGSZ_BITS,
> > + RTE_MBUF_OUTL2_LEN_OFS =
> > + RTE_MBUF_OUTL3_LEN_OFS + RTE_MBUF_OUTL3_LEN_BITS,
> > + RTE_MBUF_TXOFLD_UNUSED_OFS =
> > + RTE_MBUF_OUTL2_LEN_OFS + RTE_MBUF_OUTL2_LEN_BITS,
> > + RTE_MBUF_TXOFLD_UNUSED_BITS =
> > + sizeof(uint64_t) * CHAR_BIT - RTE_MBUF_TXOFLD_UNUSED_OFS,
> > +};
> > +
>
> What is the advantage of defining an enum instead of #defines?
No big difference here, just looks nicer to me.
>
> In any case, I wonder if it wouldn't be clearer to change the order like
> this:
>
> enum {
> RTE_MBUF_L2_LEN_OFS = 0,
> RTE_MBUF_L2_LEN_BITS = 7,
> RTE_MBUF_L3_LEN_OFS = RTE_MBUF_L2_LEN_OFS + RTE_MBUF_L2_LEN_BITS,
> RTE_MBUF_L3_LEN_BITS = 9,
> RTE_MBUF_L4_LEN_OFS = RTE_MBUF_L3_LEN_OFS + RTE_MBUF_L3_LEN_BITS,
> RTE_MBUF_L4_LEN_BITS = 8,
> ...
NP, can do this way.
>
>
> > /**
> > * The generic rte_mbuf, containing a packet mbuf.
> > */
> > @@ -640,19 +665,24 @@ struct rte_mbuf {
> > uint64_t tx_offload; /**< combined for easy fetch */
> > __extension__
> > struct {
> > - uint64_t l2_len:7;
> > + uint64_t l2_len:RTE_MBUF_L2_LEN_BITS;
> > /**< L2 (MAC) Header Length for non-tunneling pkt.
> > * Outer_L4_len + ... + Inner_L2_len for tunneling pkt.
> > */
> > - uint64_t l3_len:9; /**< L3 (IP) Header Length. */
> > - uint64_t l4_len:8; /**< L4 (TCP/UDP) Header Length. */
> > - uint64_t tso_segsz:16; /**< TCP TSO segment size */
> > + uint64_t l3_len:RTE_MBUF_L3_LEN_BITS;
> > + /**< L3 (IP) Header Length. */
> > + uint64_t l4_len:RTE_MBUF_L4_LEN_BITS;
> > + /**< L4 (TCP/UDP) Header Length. */
> > + uint64_t tso_segsz:RTE_MBUF_TSO_SEGSZ_BITS;
> > + /**< TCP TSO segment size */
> >
> > /* fields for TX offloading of tunnels */
> > - uint64_t outer_l3_len:9; /**< Outer L3 (IP) Hdr Length.
> > */
> > - uint64_t outer_l2_len:7; /**< Outer L2 (MAC) Hdr
> > Length. */
> > + uint64_t outer_l3_len:RTE_MBUF_OUTL3_LEN_BITS;
> > + /**< Outer L3 (IP) Hdr Length. */
> > + uint64_t outer_l2_len:RTE_MBUF_OUTL2_LEN_BITS;
> > + /**< Outer L2 (MAC) Hdr Length. */
> >
> > - /* uint64_t unused:8; */
> > + /* uint64_t unused:RTE_MBUF_TXOFLD_UNUSED_BITS; */
> > };
> > };
> >
> > @@ -2243,6 +2273,41 @@ static inline int rte_pktmbuf_chain(struct rte_mbuf
> > *head, struct rte_mbuf *tail
> > return 0;
> > }
> >
> > +/*
> > + * @warning
> > + * @b EXPERIMENTAL: This API may change without prior notice.
> > + *
> > + * For given input values generate raw tx_offload value.
> > + * @param il2
> > + * l2_len value.
> > + * @param il3
> > + * l3_len value.
> > + * @param il4
> >
Re: [dpdk-dev] [PATCH v4 1/6] vfio: allow DMA map of memory for the default vfio fd
30/03/2019 01:23, Thomas Monjalon: > 10/03/2019 09:27, Shahaf Shuler: > > Enable users the option to call rte_vfio_dma_map with request to map > > to the default vfio fd. > > > > Signed-off-by: Shahaf Shuler > > Acked-by: Anatoly Burakov > > --- > > --- a/doc/guides/rel_notes/release_19_05.rst > > +++ b/doc/guides/rel_notes/release_19_05.rst > > @@ -136,6 +136,9 @@ ABI Changes > > +* vfio: Functions ``rte_vfio_container_dma_map`` and > > + ``rte_vfio_container_dma_unmap`` have been extended with an option to > > + request mapping or un-mapping to the default vfio container fd. > > Isn't it an API change rather than ABI? > It is adding -1 as a special value, I think it is not breaking > previous interface, and does not require a notification in the release notes. I will move it to the "API changes" section.
Re: [dpdk-dev] [PATCH v4 3/6] bus: introduce device level DMA memory mapping
13/03/2019 12:12, Shahaf Shuler: > From: Thomas Monjalon > > 10/03/2019 09:28, Shahaf Shuler: > > > For PCI bus devices, the pci driver can expose its own map and unmap > > > functions to be used for the mapping. In case the driver doesn't > > > provide any, the memory will be mapped, if possible, to IOMMU through > > VFIO APIs. > > > > > > Application usage with those APIs is quite simple: > > > * allocate memory > > > * call rte_extmem_register on the memory chunk. > > > * take a device, and query its rte_device. > > > * call the device specific mapping function for this device. > > > > Should we make it documented somewhere? > > The full flow to work w/ external memory is documented at > doc/guides/prog_guide/env_abstraction_layer.rst , Subchapter "Support for > Externally Allocated Memory. > The last commit in series update the right API to use. OK, then I will move this doc update in this patch.
Re: [dpdk-dev] [PATCH v4 0/6] introduce DMA memory mapping for external memory
10/03/2019 09:27, Shahaf Shuler: > The DPDK APIs expose 3 different modes to work with memory used for DMA: > > 1. Use the DPDK owned memory (backed by the DPDK provided hugepages). > This memory is allocated by the DPDK libraries, included in the DPDK > memory system (memseg lists) and automatically DMA mapped by the DPDK > layers. > > 2. Use memory allocated by the user and register to the DPDK memory > systems. Upon registration of memory, the DPDK layers will DMA map it > to all needed devices. After registration, allocation of this memory > will be done with rte_*malloc APIs. > > 3. Use memory allocated by the user and not registered to the DPDK memory > system. This is for users who wants to have tight control on this > memory (e.g. avoid the rte_malloc header). > The user should create a memory, register it through rte_extmem_register > API, and call DMA map function in order to register such memory to > the different devices. > > The scope of the patch focus on #3 above. > > Currently the only way to map external memory is through VFIO > (rte_vfio_dma_map). While VFIO is common, there are other vendors > which use different ways to map memory (e.g. Mellanox and NXP). > > The work in this patch moves the DMA mapping to vendor agnostic APIs. > Device level DMA map and unmap APIs were added. Implementation of those > APIs was done currently only for PCI devices. > > For PCI bus devices, the pci driver can expose its own map and unmap > functions to be used for the mapping. In case the driver doesn't provide > any, the memory will be mapped, if possible, to IOMMU through VFIO APIs. > > Application usage with those APIs is quite simple: > * allocate memory > * call rte_extmem_register on the memory chunk. > * take a device, and query its rte_device. > * call the device specific mapping function for this device. > > Future work will deprecate the rte_vfio_dma_map and rte_vfio_dma_unmap > APIs, leaving the rte device APIs as the preferred option for the user. > > Shahaf Shuler (6): > vfio: allow DMA map of memory for the default vfio fd > vfio: don't fail to DMA map if memory is already mapped > bus: introduce device level DMA memory mapping > net/mlx5: refactor external memory registration > net/mlx5: support PCI device DMA map and unmap > doc: deprecation notice for VFIO DMA map APIs Applied, thanks
Re: [dpdk-dev] [EXT] RE: [PATCH 2/2] test/event_timer: improve unit test compatability
On Mon, 2019-03-25 at 21:40 +, Carrillo, Erik G wrote: > > -Original Message- > > From: Pavan Nikhilesh Bhagavatula [mailto:pbhagavat...@marvell.com] > > Sent: Saturday, March 16, 2019 3:28 PM > > To: Jerin Jacob Kollanukkaran ; Carrillo, Erik > > G > > > > Cc: dev@dpdk.org; Pavan Nikhilesh Bhagavatula > > > > Subject: [dpdk-dev] [PATCH 2/2] test/event_timer: improve unit test > > compatability > > > > From: Pavan Nikhilesh > > > > Check if eventdev is open system eventdevs i.e. max_num_events = -1 > > before asserting. > > Allow event timer adapter to adjust the resolution using > > RTE_EVENT_TIMER_ADAPTER_F_ADJUST_RES and re-calculate timeout ticks > > based on the adjusted resolution. > > > > Signed-off-by: Pavan Nikhilesh > > Just one spelling nit - it should be "compatibility" in the subject > line. With that change: Fixed in apply. > > Acked-by: Erik Gabriel Carrillo Series applied to dpdk-next-eventdev/master. Thanks.
Re: [dpdk-dev] [EXT] [PATCH] event/sw: fix selftest enqueue return value checks
On Wed, 2019-03-27 at 18:45 +, Harry van Haaren wrote:
> ---
> ---
> This patch fixes a number of instances of the same return
> value mis-check, where previously we checked for a negative
> return value as error, however the API returns an unsigned
> integer, so these return value checks are invalid.
>
> The rte_event_enqueue_burst() API returns the number of
> events enqueued, so in order to identify the error case,
> we must check for != the number of intended enqueues.
>
> Fixes: cd1a9e3eab55 ("test/eventdev: add SW tests for load
> balancing")
Cc: sta...@dpdk.org
> Signed-off-by: Harry van Haaren
Applied to dpdk-next-eventdev/master. Thanks.
Re: [dpdk-dev] [PATCH v2 1/3] packet_ordering: add statistics for each worker thread
> -Original Message- > From: Pattan, Reshma > Sent: Saturday, March 30, 2019 12:40 AM > To: Phil Yang (Arm Technology China) ; dev@dpdk.org; > tho...@monjalon.net > Cc: Hunt, David ; Gavin Hu (Arm Technology China) > ; Honnappa Nagarahalli > ; nd > Subject: RE: [PATCH v2 1/3] packet_ordering: add statistics for each worker > thread > > > > > -Original Message- > > From: Phil Yang [mailto:phil.y...@arm.com] > > > > The current implementation using '__sync' built-ins to synchronize > > statistics within worker threads. '__sync' built-ins functions are > > full barriers which will affect the performance, so add a per worker packets > statistics. > > > > Enable by option --insight-worker. > > > > I don't feel the need of this new option to print per core stats. Any reason > for this? Hi Reshma, Thanks for your comment. The per core stats aims at removing the '__sync' builtin full barrier in the worker thread. It records the workload of each core (It shows the bottleneck core as well). Since the maximum core number may be more than 128, so disable the print in default and add this new option for debugging use. Anyway, if you insist there is no need to print that info out, I can remove the option. But I think using per core stats will benefit performance. Thanks, Phil > > Thanks, > Reshma
Re: [dpdk-dev] [PATCH v3 01/15] app/testpmd: replace uint with unsigned int
13/03/2019 18:06, Natanael Copa: > Improve portability by avoid use non-standard 'uint'. After running git grep '\', I see there is also an occurence in examples/tep_termination/vxlan.c
Re: [dpdk-dev] [PATCH v3 03/15] bus/pci: add fallback for out[lwb]_p for non GNU libc
13/03/2019 18:06, Natanael Copa:
> Add a fallback for non-GNU libc systems like musl libc for the
> non-standard functions outl_p, outw_p and outb_p.
>
> This ifixes the following buildtime errors when building with musl libc:
> pci_uio.c:(.text+0xaa1): undefined reference to `outw_p'
> pci_uio.c:(.text+0xac5): undefined reference to `outl_p'
> pci_uio.c:(.text+0xadf): undefined reference to `outb_p'
>
> fixes https://bugs.dpdk.org/show_bug.cgi?id=35
Please use this syntax:
Bugzilla ID: 35
[...]
> #if defined(RTE_ARCH_X86)
> #include
> +#if defined(__GLIBC__)
> +#define pci_uio_outl_p outl_p
> +#define pci_uio_outw_p outw_p
> +#define pci_uio_outb_p outb_p
> +#else
> +static inline void
> +pci_uio_outl_p(unsigned int value, unsigned short int port)
> +{
> + __asm__ __volatile__ ("outl %0,%w1\noutb %%al,$0x80" : : "a" (value),
> + "Nd" (port));
> +}
> +
> +static inline void
> +pci_uio_outw_p(unsigned short int value, unsigned short int port)
> +{
> + __asm__ __volatile__ ("outw %w0,%w1\noutb %%al,$0x80" : : "a" (value),
> + "Nd" (port));
> +}
> +
> +static inline void
> +pci_uio_outb_p(unsigned char value, unsigned short int port)
> +{
> + __asm__ __volatile__ ("outb %b0,%w1\noutb %%al,$0x80" : : "a" (value),
> + "Nd" (port));
> +}
> +#endif
> #endif
[...]
> #if defined(RTE_ARCH_X86)
> - outl_p(*(const uint32_t *)s, reg);
> + pci_uio_outl_p(*(const uint32_t *)s, reg);
> #else
> *(volatile uint32_t *)reg = *(const uint32_t *)s;
> #endif
Could we manage non-x86 case in the macros pci_uio_out?
Thanks
Re: [dpdk-dev] [PATCH v3 04/15] bus/pci: factor out various ifdefs in pci_uio_ioport_{read, write}
13/03/2019 18:06, Natanael Copa:
> define the macros so we can remove various #if defined(RTE_ARCH_X86)
>
> Ref: https://bugs.dpdk.org/show_bug.cgi?id=35#c6
>
> Signed-off-by: Natanael Copa
> ---
> drivers/bus/pci/linux/pci_uio.c | 54 +++--
> 1 file changed, 24 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c
> index e1dd8c875..b0470358d 100644
> --- a/drivers/bus/pci/linux/pci_uio.c
> +++ b/drivers/bus/pci/linux/pci_uio.c
> @@ -14,11 +14,18 @@
>
> #if defined(RTE_ARCH_X86)
> #include
> +
> +#define pci_uio_inl(reg) inl(reg)
> +#define pci_uio_inw(reg) inw(reg)
> +#define pci_uio_inb(reg) inb(reg)
> +
> #if defined(__GLIBC__)
> +
> #define pci_uio_outl_p outl_p
> #define pci_uio_outw_p outw_p
> #define pci_uio_outb_p outb_p
> -#else
> +
> +#else /* defined(__GLIBC__) */
> static inline void
> pci_uio_outl_p(unsigned int value, unsigned short int port)
> {
> @@ -39,8 +46,19 @@ pci_uio_outb_p(unsigned char value, unsigned short int
> port)
> __asm__ __volatile__ ("outb %b0,%w1\noutb %%al,$0x80" : : "a" (value),
> "Nd" (port));
> }
> -#endif
> -#endif
> +#endif /* defined(__GLIBC__) */
> +
> +#else /* RTE_ARCH_X86 */
> +
> +#define pci_uio_inl(reg) (*(volatile uint32_t *)(reg))
> +#define pci_uio_inw(reg) (*(volatile uint16_t *)(reg))
> +#define pci_uio_inb(reg) (*(volatile uint8_t *)(reg))
> +
> +#define pci_uio_outl_p(value, reg) (*(volatile uint32_t *)(reg) = (value))
> +#define pci_uio_outw_p(value, reg) (*(volatile uint16_t *)(reg) = (value))
> +#define pci_uio_outb_p(value, reg) (*(volatile uint8_t *)(reg) = (value))
> +
> +#endif /* RTE_ARCH_X86 */
I think I would prefer having the #ifdef inside the macros,
instead of redefining the macros.
Re: [dpdk-dev] [PATCH v3 10/15] bus/dpaa: use warn(3) instead of error(3) to improve portability
13/03/2019 18:06, Natanael Copa: > Fix build with musl libc by using warn(3) instead of error(3). > > This also fixes error message for kzmalloc failures which previously > would have given "Unknown error -1". I think this patch requires more explanations about what was the error, why using warn, and how it is related to kzmalloc failure log. Thanks
Re: [dpdk-dev] [PATCH v3 11/15] bus/dpaa: fix warning: "__WORDSIZE" is not defined, evaluates to 0
13/03/2019 18:06, Natanael Copa: > There is no standard saying that __WORDSIZE should be be defined or in > what include it should be defined. Use a portable way to detect 64 bit > environment. > > This fixes a warning when building with musl libc: > > warning: "__WORDSIZE" is not defined, evaluates to 0 [-Wundef] > > Signed-off-by: Natanael Copa > --- [...] > -#if (__WORDSIZE == 64) > +#if (ULONG_MAX == 0x) There is a more explicit config in DPDK: RTE_ARCH_64
Re: [dpdk-dev] [PATCH v3 13/15] crypto/dpaa2_sec: simplify pr_{debug, err, warn} macros
13/03/2019 18:06, Natanael Copa: > Simplify pr_debug, pr_err and pr_warn macros by add an intermediate > pr_msg macro. This way we only need test for SUPPRESS_PRINTS once. > > Signed-off-by: Natanael Copa Adding more Cc for review.
Re: [dpdk-dev] [PATCH v3 00/15] Build fixes for musl libc
13/03/2019 18:06, Natanael Copa:
> A set of patches to fix build with musl libc. I also did a few cleanups wrt
> macros and fixed a few scary compiler warnings while at it.
>
> Please note that those are only compile tested on x86_64 with musl libc.
>
> v2 has some code style fixes reported by checkpatch
> v3 is a full reroll of the entire patch set to enable automatted tests
>and logic for patchwork patchset.
>
> Natanael Copa (15):
> app/testpmd: replace uint with unsigned int
> net/cxgbe: replace uint with unsigned int
> bus/pci: add fallback for out[lwb]_p for non GNU libc
> bus/pci: factor out various ifdefs in pci_uio_ioport_{read,write}
> bus/fslmc: fix compile error with musl libc
> bus/fslmc: remove unused include of error.h
> net/nfp: build fix for musl libc
> app/test: include fcntl.h due to use of O_RDONLY
> app/test: fix setting of -D_GNU_SOURCE with meson
> bus/dpaa: use warn(3) instead of error(3) to improve portability
> bus/dpaa: fix warning: "__WORDSIZE" is not defined, evaluates to 0
> crypto/dpaa2_sec: build fix for musl libc
> crypto/dpaa2_sec: simplify pr_{debug,err,warn} macros
> net/netvsc: fix compile warning for fcntl.h include
> eal/linux: simplify debug message in sigbus_handler
Thanks for the cleanup.
Some patches may deserve more reviews.
If you send a new version, please use --to-cmd devtools/get-maintainer.sh
so the maintainers will be Cc'ed.
All these patches are fixes and may be interesting to backport them
in the latest LTS or other stable releases.
In order to help with backporting, please fill "Fixes:" tag
and add Cc: stable when relevant.
The exact recommendation is described in this doc chapter:
http://doc.dpdk.org/guides/contributing/patches.html#commit-messages-body
In order to help, I already fixed 7 commit messages and merged them.
The remaining patches, requiring a v4, are:
app/testpmd: replace uint with unsigned int
net/cxgbe: replace uint with unsigned int
bus/pci: add fallback for out[lwb]_p for non GNU libc
bus/pci: factor out various ifdefs in pci_uio_ioport_{read,write}
bus/dpaa: use warn(3) instead of error(3) to improve portability
bus/dpaa: fix warning: "__WORDSIZE" is not defined, evaluates to 0
crypto/dpaa2_sec: build fix for musl libc
crypto/dpaa2_sec: simplify pr_{debug,err,warn} macros
