Build igb_uio.ko separately from building all DPDK code

[Asaf Sinai]

Hi,

How to build 'igb_uio.ko', without the need to build all DPDK code?

Thanks,
Asaf

[Radware]
Asaf Sinai
ND SW Engineer
Email: asa...@radware.com
T:+972-72-3917050
M:+972-50-6518541
F:+972-3-6488662
[Check out the latest and greatest from 
Radware]

www.radware.com

[Blog]  
[https://www.radware.com/images/signature/linkedin.jpg] 
 
[https://www.radware.com/images/signature/twitter.jpg] 
   [youtube] 

Confidentiality note: This message, and any attachments to it, contains 
privileged/confidential information of RADWARE Ltd./RADWARE Inc. and may not be 
disclosed, used, copied, or transmitted in any form or by any means without 
prior written permission from RADWARE. If you are not the intended recipient, 
delete the message and any attachments from your system without reading or 
copying it, and kindly notify the sender by e-mail. Thank you.
P Please consider your environmental responsibility before printing this e-mail

Re: [PATCH v1 01/25] drivers/net: introduce a new PMD driver

[Stephen Hemminger]

On Sat, 18 Dec 2021 10:51:28 +0800
Yanling Song  wrote:

> +#ifdef CLOCK_MONOTONIC_RAW /* Defined in glibc bits/time.h */
> +#define CLOCK_TYPE CLOCK_MONOTONIC_RAW
> +#else
> +#define CLOCK_TYPE CLOCK_MONOTONIC
> +#endif

CLOCK_MONOTONIC_RAW was defined in Linux.2.6.28
DPDK does not support any kernels that old, so the #ifdef is not needed.


+
+static inline unsigned long clock_gettime_ms(void)
+{
+   struct timespec tv;
+
+   (void)clock_gettime(CLOCK_TYPE, &tv);
+
+   return (unsigned long)tv.tv_sec * SPNIC_S_TO_MS_UNIT +
+  (unsigned long)tv.tv_nsec / SPNIC_S_TO_NS_UNIT;
+}

If all you want is jiffie accuracy,  you could use CLOCK_MONOTONIC_COARSE.


+#define jiffiesclock_gettime_ms()
+#define msecs_to_jiffies(ms)   (ms)

+#define time_before(now, end)  ((now) < (end))

Does that simple version of the macro work right if jiffies wraps around?
Less of an issue on 64 bit platforms...

The kernel version is effectively.
#define time_before(now, end) ((long)((now) - (end)) < 0)

Re: [PATCH] crypto: use single buffer for asymmetric session

[Ray Kinsella]

Ciara,

One minor niggle.

Ciara Power  writes:

> diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map
> index c50745fa8c..00b1c9ae35 100644
> --- a/lib/cryptodev/version.map
> +++ b/lib/cryptodev/version.map
> @@ -58,7 +58,6 @@ EXPERIMENTAL {
>   rte_cryptodev_asym_session_clear;
>   rte_cryptodev_asym_session_create;
>   rte_cryptodev_asym_session_free;
> - rte_cryptodev_asym_session_init;
>   rte_cryptodev_asym_xform_capability_check_modlen;
>   rte_cryptodev_asym_xform_capability_check_optype;
>   rte_cryptodev_sym_cpu_crypto_process;
> @@ -104,6 +103,11 @@ EXPERIMENTAL {
>   rte_cryptodev_remove_deq_callback;
>   rte_cryptodev_remove_enq_callback;
>  
> + # added 22.03


rte_cryptodev_asym_session_get_user_data should be before
rte_cryptodev_asym_session_pool_create, right?


> + rte_cryptodev_asym_session_pool_create;
> + rte_cryptodev_asym_session_get_user_data;
> + rte_cryptodev_asym_session_set_user_data;
> + __rte_cryptodev_trace_asym_session_pool_create;
>  };
>  
>  INTERNAL {

-- 
Regards, Ray K

Re: [PATCH v1 16/25] net/spnic: add device configure/version/info

[Stephen Hemminger]

On Sat, 18 Dec 2021 10:51:43 +0800
Yanling Song  wrote:

> +static int spnic_dev_configure(struct rte_eth_dev *dev)
> +{
> + struct spnic_nic_dev *nic_dev = SPNIC_ETH_DEV_TO_PRIVATE_NIC_DEV(dev);
> +
> + nic_dev->num_sqs =  dev->data->nb_tx_queues;
> + nic_dev->num_rqs = dev->data->nb_rx_queues;
> +
> + if (nic_dev->num_sqs > nic_dev->max_sqs ||
> + nic_dev->num_rqs > nic_dev->max_rqs) {
> + PMD_DRV_LOG(ERR, "num_sqs: %d or num_rqs: %d larger than 
> max_sqs: %d or max_rqs: %d",
> + nic_dev->num_sqs, nic_dev->num_rqs,
> + nic_dev->max_sqs, nic_dev->max_rqs);
> + return -EINVAL;
> + }
> +

This should already be covered by checks in ethedev:dev_configure.

> + /* The range of mtu is 384~9600 */
> + if (SPNIC_MTU_TO_PKTLEN(dev->data->dev_conf.rxmode.mtu) <
> + SPNIC_MIN_FRAME_SIZE ||
> + SPNIC_MTU_TO_PKTLEN(dev->data->dev_conf.rxmode.mtu) >
> + SPNIC_MAX_JUMBO_FRAME_SIZE) {
> + PMD_DRV_LOG(ERR, "Max rx pkt len out of range, mtu: %d, expect 
> between %d and %d",
> + dev->data->dev_conf.rxmode.mtu,
> + SPNIC_MIN_FRAME_SIZE, SPNIC_MAX_JUMBO_FRAME_SIZE);
> + return -EINVAL;
> + }

Already covered by eth_dev_validate_mtu called from ethdev dev_configure.

[PATCH] net/i40e: remove redundant judgment for rearm

[Feifei Wang]

Merged variable updates under the same condition. It reduces branch.

In n1sdp, there is no performance improvement with this patch.
In x86, there is also no performance improvement.

Suggested-by: Honnappa Nagarahalli 
Signed-off-by: Feifei Wang 
Reviewed-by: Ruifeng Wang 
---
 drivers/net/i40e/i40e_rxtx_vec_neon.c | 9 +
 drivers/net/i40e/i40e_rxtx_vec_sse.c  | 9 +
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/net/i40e/i40e_rxtx_vec_neon.c 
b/drivers/net/i40e/i40e_rxtx_vec_neon.c
index b951ea2dc3..c7e4222b61 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_neon.c
+++ b/drivers/net/i40e/i40e_rxtx_vec_neon.c
@@ -64,14 +64,15 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)
}
 
rxq->rxrearm_start += RTE_I40E_RXQ_REARM_THRESH;
-   if (rxq->rxrearm_start >= rxq->nb_rx_desc)
+   rx_id = rxq->rxrearm_start - 1;
+
+   if (unlikely(rxq->rxrearm_start >= rxq->nb_rx_desc)) {
rxq->rxrearm_start = 0;
+   rx_id = rxq->nb_rx_desc - 1;
+   }
 
rxq->rxrearm_nb -= RTE_I40E_RXQ_REARM_THRESH;
 
-   rx_id = (uint16_t)((rxq->rxrearm_start == 0) ?
-(rxq->nb_rx_desc - 1) : (rxq->rxrearm_start - 1));
-
rte_io_wmb();
/* Update the tail pointer on the NIC */
I40E_PCI_REG_WRITE_RELAXED(rxq->qrx_tail, rx_id);
diff --git a/drivers/net/i40e/i40e_rxtx_vec_sse.c 
b/drivers/net/i40e/i40e_rxtx_vec_sse.c
index 497b2404c6..0910039d69 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_sse.c
+++ b/drivers/net/i40e/i40e_rxtx_vec_sse.c
@@ -77,14 +77,15 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)
}
 
rxq->rxrearm_start += RTE_I40E_RXQ_REARM_THRESH;
-   if (rxq->rxrearm_start >= rxq->nb_rx_desc)
+   rx_id = rxq->rxrearm_start - 1;
+
+   if (unlikely(rxq->rxrearm_start >= rxq->nb_rx_desc)) {
rxq->rxrearm_start = 0;
+   rx_id = rxq->nb_rx_desc - 1;
+   }
 
rxq->rxrearm_nb -= RTE_I40E_RXQ_REARM_THRESH;
 
-   rx_id = (uint16_t)((rxq->rxrearm_start == 0) ?
-(rxq->nb_rx_desc - 1) : (rxq->rxrearm_start - 1));
-
/* Update the tail pointer on the NIC */
I40E_PCI_REG_WC_WRITE(rxq->qrx_tail, rx_id);
 }
-- 
2.25.1

[PATCH 0/3] reduce redundant store operation for Tx free

[Feifei Wang]

Reduce redundant store operation for buffer free in Tx path.

Feifei Wang (3):
  net/i40e: reduce redundant store operation
  net/ice: reduce redundant store operation
  net/ixgbe: reduce redundant store operation

 drivers/net/i40e/i40e_rxtx.c| 3 ---
 drivers/net/i40e/i40e_rxtx_vec_common.h | 1 -
 drivers/net/ice/ice_rxtx.c  | 2 --
 drivers/net/ixgbe/ixgbe_rxtx.c  | 1 -
 4 files changed, 7 deletions(-)

-- 
2.25.1

[PATCH 1/3] net/i40e: reduce redundant store operation

[Feifei Wang]

For free buffer operation in i40e driver, it is unnecessary to store
'NULL' into txep.mbuf. This is because when putting mbuf into Tx queue,
tx_tail is the sentinel. And when doing tx_free, tx_next_dd is the
sentinel. In all processes, mbuf==NULL is not a condition in check.
Thus reset of mbuf is unnecessary and can be omitted.

Signed-off-by: Feifei Wang 
Reviewed-by: Ruifeng Wang 
---
 drivers/net/i40e/i40e_rxtx.c| 3 ---
 drivers/net/i40e/i40e_rxtx_vec_common.h | 1 -
 2 files changed, 4 deletions(-)

diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index e4cb33dc3c..4de2be53e6 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1344,7 +1344,6 @@ i40e_tx_free_bufs(struct i40e_tx_queue *txq)
for (j = 0; j != k; j += RTE_I40E_TX_MAX_FREE_BUF_SZ) {
for (i = 0; i < RTE_I40E_TX_MAX_FREE_BUF_SZ; 
++i, ++txep) {
free[i] = txep->mbuf;
-   txep->mbuf = NULL;
}
rte_mempool_put_bulk(free[0]->pool, (void 
**)free,
RTE_I40E_TX_MAX_FREE_BUF_SZ);
@@ -1354,14 +1353,12 @@ i40e_tx_free_bufs(struct i40e_tx_queue *txq)
if (m) {
for (i = 0; i < m; ++i, ++txep) {
free[i] = txep->mbuf;
-   txep->mbuf = NULL;
}
rte_mempool_put_bulk(free[0]->pool, (void **)free, m);
}
} else {
for (i = 0; i < txq->tx_rs_thresh; ++i, ++txep) {
rte_pktmbuf_free_seg(txep->mbuf);
-   txep->mbuf = NULL;
}
}
 
diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h 
b/drivers/net/i40e/i40e_rxtx_vec_common.h
index f9a7f46550..26deb59fc4 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_common.h
+++ b/drivers/net/i40e/i40e_rxtx_vec_common.h
@@ -103,7 +103,6 @@ i40e_tx_free_bufs(struct i40e_tx_queue *txq)
if (txq->offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) {
for (i = 0; i < n; i++) {
free[i] = txep[i].mbuf;
-   txep[i].mbuf = NULL;
}
rte_mempool_put_bulk(free[0]->pool, (void **)free, n);
goto done;
-- 
2.25.1

[PATCH 2/3] net/ice: reduce redundant store operation

[Feifei Wang]

For free buffer in ice driver, it is unnecessary to store 'NULL' into
txep.mbuf. This is because when putting mbuf into Tx queue, tx_tail is
the sentinel. And when doing tx_free, tx_next_dd is the sentinel. In all
processes, mbuf==NULL is not a condition in check. Thus reset of mbuf is
unnecessary and can be omitted.

Signed-off-by: Feifei Wang 
Reviewed-by: Ruifeng Wang 
---
 drivers/net/ice/ice_rxtx.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index f6d8564ab8..e043335bad 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -2893,12 +2893,10 @@ ice_tx_free_bufs(struct ice_tx_queue *txq)
if (txq->offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) {
for (i = 0; i < txq->tx_rs_thresh; ++i, ++txep) {
rte_mempool_put(txep->mbuf->pool, txep->mbuf);
-   txep->mbuf = NULL;
}
} else {
for (i = 0; i < txq->tx_rs_thresh; ++i, ++txep) {
rte_pktmbuf_free_seg(txep->mbuf);
-   txep->mbuf = NULL;
}
}
 
-- 
2.25.1

[PATCH 3/3] net/ixgbe: reduce redundant store operation

[Feifei Wang]

For free buffer in ixgbe driver, it is unnecessary to store 'NULL' into
txep.mbuf. This is because when putting mbuf into Tx queue, tx_tail is
the sentinel. And when doing tx_free, tx_next_dd is the sentinel. In all
processes, mbuf==NULL is not a condition in check. Thus reset of mbuf is
unnecessary and can be omitted.

Signed-off-by: Feifei Wang 
Reviewed-by: Ruifeng Wang 
---
 drivers/net/ixgbe/ixgbe_rxtx.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index d7c80d4242..9f3f2e9b50 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -120,7 +120,6 @@ ixgbe_tx_free_bufs(struct ixgbe_tx_queue *txq)
for (i = 0; i < txq->tx_rs_thresh; ++i, ++txep) {
/* free buffers one at a time */
m = rte_pktmbuf_prefree_seg(txep->mbuf);
-   txep->mbuf = NULL;
 
if (unlikely(m == NULL))
continue;
-- 
2.25.1

RE: [PATCH 3/3] net/ixgbe: reduce redundant store operation

[Wang, Haiyue]

> -Original Message-
> From: Feifei Wang 
> Sent: Monday, December 20, 2021 13:51
> To: Wang, Haiyue 
> Cc: dev@dpdk.org; n...@arm.com; Feifei Wang ; Ruifeng 
> Wang 
> Subject: [PATCH 3/3] net/ixgbe: reduce redundant store operation
> 
> For free buffer in ixgbe driver, it is unnecessary to store 'NULL' into
> txep.mbuf. This is because when putting mbuf into Tx queue, tx_tail is
> the sentinel. And when doing tx_free, tx_next_dd is the sentinel. In all
> processes, mbuf==NULL is not a condition in check. Thus reset of mbuf is
> unnecessary and can be omitted.
> 
> Signed-off-by: Feifei Wang 
> Reviewed-by: Ruifeng Wang 
> ---
>  drivers/net/ixgbe/ixgbe_rxtx.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index d7c80d4242..9f3f2e9b50 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -120,7 +120,6 @@ ixgbe_tx_free_bufs(struct ixgbe_tx_queue *txq)
>   for (i = 0; i < txq->tx_rs_thresh; ++i, ++txep) {
>   /* free buffers one at a time */
>   m = rte_pktmbuf_prefree_seg(txep->mbuf);
> - txep->mbuf = NULL;

Not sure, but at least found:

static void __rte_cold
ixgbe_tx_queue_release_mbufs(struct ixgbe_tx_queue *txq)
{
unsigned i;

if (txq->sw_ring != NULL) {
for (i = 0; i < txq->nb_tx_desc; i++) {
if (txq->sw_ring[i].mbuf != NULL) {  
< ?
rte_pktmbuf_free_seg(txq->sw_ring[i].mbuf);
txq->sw_ring[i].mbuf = NULL;
}
}
}
}

> 
>   if (unlikely(m == NULL))
>   continue;
> --
> 2.25.1

[Bug 913] [dpdk-19.11.11]'mk/' makefile build failed on Freebsd13 with clang11.0.1

[bugzilla]

https://bugs.dpdk.org/show_bug.cgi?id=913

Bug ID: 913
   Summary: [dpdk-19.11.11]'mk/' makefile build failed on
Freebsd13 with clang11.0.1
   Product: DPDK
   Version: 19.11
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: normal
  Priority: Normal
 Component: core
  Assignee: dev@dpdk.org
  Reporter: longfengx.li...@intel.com
  Target Milestone: ---

1.DPDk version:
commit e247b323cf0f079c3020d19314d5a5c64fa680c5 (HEAD -> 19.11, origin/HEAD,
origin/19.11)
Author: Bruce Richardson 
Date:   Fri Jan 3 11:52:43 2020 +

contigmem: update for FreeBSD 13

[ upstream commit 03bff90ccfd6ad347f1e50901caf47890872ca28 ]

FreeBSD 13 has changed the definition of vm_page_replace so we need
to have slightly different code paths around this function depending on
the BSD version.

Signed-off-by: Bruce Richardson 
Signed-off-by: Christian Ehrhardt 

2.Based on dpdk version 19.11.11-rc1 commit details:
e247b323c (HEAD -> 19.11, origin/HEAD, origin/19.11) contigmem: update for
FreeBSD 13
f3068d161 eal/freebsd: update CPU macro for FreeBSD 13
30585e9c4 eal/freebsd: fix incorrect variable name
e99e9d1c1 kni: update kernel API to set random MAC address
f5b3f1a47 cryptodev: fix stringop-overflow build failure with gcc 10
2ce84bf86 igb_uio: fix build for switch fall through
fab80885e net/ixgbe: build failure with make and clang 13
acd0a2d2d net/ice: build failure with make and clang 13
cc089e955 net/i40e: build failure with make and clang 13
d43fa3e19 kni: fix build for SLES15-SP3 (Make)
f32335f08 ethdev: fix typos
e5075011b net/nfp: remove unused message length

3.Just verify that the three patches are build passed：
e247b323c (HEAD -> 19.11, origin/HEAD, origin/19.11) contigmem: update for
FreeBSD 13
f3068d161 eal/freebsd: update CPU macro for FreeBSD 13
30585e9c4 eal/freebsd: fix incorrect variable name

4.TestSteps:
#export RTE_TARGET=x86_64-native-bsdapp-clang
#export RTE_SDK=`pwd`
#gmake -j 70 install T=x86_64-native-bsdapp-clang

5.Error Log:
[root@freebsd13-gcc ~/dpdk-stable-queue]# export
RTE_TARGET=x86_64-native-bsdapp-clang
export RTE_SDK=`pwd`
gmake -j 70 install T=x86_64-native-bsdapp-clang
Configuration done using x86_64-native-bsdapp-clang
== Build lib
== Build lib/librte_kvargs
  CC rte_kvargs.o
  SYMLINK-FILE include/rte_kvargs.h
  AR librte_kvargs.a
  INSTALL-LIB librte_kvargs.a
== Build lib/librte_eal
== Build lib/librte_eal/common
  SYMLINK-FILE include/generic/rte_atomic.h
  SYMLINK-FILE include/generic/rte_byteorder.h
  SYMLINK-FILE include/generic/rte_cycles.h
  SYMLINK-FILE include/generic/rte_prefetch.h
  SYMLINK-FILE include/generic/rte_memcpy.h
  SYMLINK-FILE include/generic/rte_cpuflags.h
  SYMLINK-FILE include/generic/rte_mcslock.h
  SYMLINK-FILE include/generic/rte_spinlock.h
  SYMLINK-FILE include/generic/rte_rwlock.h
  SYMLINK-FILE include/generic/rte_ticketlock.h
  SYMLINK-FILE include/generic/rte_vect.h
  SYMLINK-FILE include/generic/rte_pause.h
  SYMLINK-FILE include/generic/rte_io.h
  SYMLINK-FILE include/rte_branch_prediction.h
  SYMLINK-FILE include/rte_common.h
  SYMLINK-FILE include/rte_compat.h
  SYMLINK-FILE include/rte_function_versioning.h
  SYMLINK-FILE include/rte_debug.h
  SYMLINK-FILE include/rte_eal.h
  SYMLINK-FILE include/rte_eal_interrupts.h
  SYMLINK-FILE include/rte_errno.h
  SYMLINK-FILE include/rte_launch.h
  SYMLINK-FILE include/rte_lcore.h
  SYMLINK-FILE include/rte_log.h
  SYMLINK-FILE include/rte_memory.h
  SYMLINK-FILE include/rte_memzone.h
  SYMLINK-FILE include/rte_per_lcore.h
  SYMLINK-FILE include/rte_random.h
  SYMLINK-FILE include/rte_tailq.h
  SYMLINK-FILE include/rte_interrupts.h
  SYMLINK-FILE include/rte_alarm.h
  SYMLINK-FILE include/rte_string_fns.h
  SYMLINK-FILE include/rte_version.h
  SYMLINK-FILE include/rte_eal_memconfig.h
  SYMLINK-FILE include/rte_hexdump.h
  SYMLINK-FILE include/rte_devargs.h
  SYMLINK-FILE include/rte_bus.h
  SYMLINK-FILE include/rte_dev.h
  SYMLINK-FILE include/rte_class.h
  SYMLINK-FILE include/rte_option.h
  SYMLINK-FILE include/rte_pci_dev_feature_defs.h
  SYMLINK-FILE include/rte_pci_dev_features.h
  SYMLINK-FILE include/rte_malloc.h
  SYMLINK-FILE include/rte_keepalive.h
  SYMLINK-FILE include/rte_time.h
  SYMLINK-FILE include/rte_service.h
  SYMLINK-FILE include/rte_service_component.h
  SYMLINK-FILE include/rte_bitmap.h
  SYMLINK-FILE include/rte_vfio.h
  SYMLINK-FILE include/rte_hypervisor.h
  SYMLINK-FILE include/rte_test.h
  SYMLINK-FILE include/rte_reciprocal.h
  SYMLINK-FILE include/rte_fbarray.h
  SYMLINK-FILE include/rte_uuid.h
  SYMLINK-FILE include/rte_atomic.h
  SYMLINK-FILE include/rte_atomic_32.h
  SYMLINK-FILE include/rte_atomic_64.h
  SYMLINK-FILE include/rte_byteorder.h
  SYMLINK-FILE include/rte_byteorder_32.h
  SYMLINK-FILE include/rte_byteorder_64.h
  SYMLINK-FILE include/rte_cpuflags.h
  SYMLINK-FILE include/rte_cycles.h
  SYMLINK-FILE in

RE: [dpdk-dev] [PATCH] net/ixgbe: check ixgbe filter init failure

[Wang, Haiyue]

> -Original Message-
> From: Yunjian Wang 
> Sent: Saturday, December 4, 2021 18:24
> To: dev@dpdk.org
> Cc: Wang, Haiyue ; dingxiaoxi...@huawei.com; 
> xudin...@huawei.com; Yunjian Wang
> ; sta...@dpdk.org
> Subject: [dpdk-dev] [PATCH] net/ixgbe: check ixgbe filter init failure
> 
> The function ixgbe_fdir_filter_init() and ixgbe_l2_tn_filter_init()
> could return errors, the return value need to be checked and returned.
> 
> Fixes: 080e3c0ee989 ("net/ixgbe: store flow director filter")
> Fixes: d0c0c416ef1f ("net/ixgbe: store L2 tunnel filter")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Yunjian Wang 
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 30 +-
>  1 file changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c 
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index fe61dba81d..25d6de7709 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1223,13 +1223,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void 
> *init_params __rte_unused)
> 
>   /* initialize PF if max_vfs not zero */
>   ret = ixgbe_pf_host_init(eth_dev);
> - if (ret) {
> - rte_free(eth_dev->data->mac_addrs);
> - eth_dev->data->mac_addrs = NULL;
> - rte_free(eth_dev->data->hash_mac_addrs);
> - eth_dev->data->hash_mac_addrs = NULL;
> - return ret;
> - }
> + if (ret)
> + goto err_pf_host_init;
> 
>   ctrl_ext = IXGBE_READ_REG(hw, IXGBE_CTRL_EXT);
>   /* let hardware know driver is loaded */
> @@ -1268,10 +1263,14 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void 
> *init_params __rte_unused)
>   TAILQ_INIT(&filter_info->fivetuple_list);
> 
>   /* initialize flow director filter list & hash */
> - ixgbe_fdir_filter_init(eth_dev);
> + ret = ixgbe_fdir_filter_init(eth_dev);
> + if (ret)
> + goto err_fdir_filter_init;
> 
>   /* initialize l2 tunnel filter list & hash */
> - ixgbe_l2_tn_filter_init(eth_dev);
> + ret = ixgbe_l2_tn_filter_init(eth_dev);
> + if (ret)
> + goto err_l2_tn_filter_init;
> 
>   /* initialize flow filter lists */
>   ixgbe_filterlist_init();
> @@ -1283,6 +1282,19 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void 
> *init_params __rte_unused)
>   ixgbe_tm_conf_init(eth_dev);
> 
>   return 0;
> +
> +err_l2_tn_filter_init:
> + ixgbe_fdir_filter_uninit(eth_dev);
> +err_fdir_filter_init:
> + ixgbe_pf_host_uninit(eth_dev);

The interrupt needs to be closed ?

ixgbe_disable_intr(hw);

rte_intr_disable(intr_handle);

rte_intr_callback_unregister(intr_handle, ixgbe_dev_interrupt_handler, eth_dev);

> + rte_intr_callback_unregister(intr_handle,
> + ixgbe_dev_interrupt_handler, eth_dev);
> +err_pf_host_init:
> + rte_free(eth_dev->data->mac_addrs);
> + eth_dev->data->mac_addrs = NULL;
> + rte_free(eth_dev->data->hash_mac_addrs);
> + eth_dev->data->hash_mac_addrs = NULL;
> + return ret;
>  }
> 
>  static int
> --
> 2.27.0

RE: [PATCH v2 1/7] net/ixgbe: Fix ixgbe_is_sfp() to return valid result for X550EM_a devs

[Wang, Haiyue]

> -Original Message-
> From: Stephen Douthit 
> Sent: Tuesday, December 7, 2021 06:19
> To: Wang, Haiyue ; Lu, Wenzhuo 
> Cc: dev@dpdk.org; Wen Wang ; Stephen Douthit 
> ;
> sta...@dpdk.org
> Subject: [PATCH v2 1/7] net/ixgbe: Fix ixgbe_is_sfp() to return valid result 
> for X550EM_a devs
> 
> Currently all X500EM* MAC types fallthrough to the default case and get
> reported as non-SFP regardless of media type, which isn't correct.
> 
> Fixes: 0790adeb567 ("ixgbe/base: support X550em_a device")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Stephen Douthit 
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c 
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index fe61dba81d..66f7af95de 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -781,6 +781,20 @@ ixgbe_is_sfp(struct ixgbe_hw *hw)
>   case ixgbe_phy_sfp_passive_unknown:
>   return 1;
>   default:
> + /* x550em devices may be SFP, check media type */
> + switch (hw->mac.type) {
> + case ixgbe_mac_X550EM_x:
> + case ixgbe_mac_X550EM_a:
> + switch (hw->mac.ops.get_media_type(hw)) {

Use the API 'ixgbe_get_media_type' to avoid ops null ?

> + case ixgbe_media_type_fiber:
> + case ixgbe_media_type_fiber_qsfp:
> + return 1;
> + default:
> + return 0;

Since we care 'return 1' only, then the two defaults just "break;" ?

> + }
> + default:
> + return 0; 

Just 'break;'

> + }
>   return 0;

Then this default '0' will be used.

>   }
>  }
> --
> 2.31.1

RE: [PATCH v2 3/7] net/ixgbe: Check that SFF-8472 soft rate select is supported before write

[Wang, Haiyue]

> -Original Message-
> From: Stephen Douthit 
> Sent: Tuesday, December 7, 2021 06:19
> To: Wang, Haiyue ; Lu, Wenzhuo ; 
> Changchun Ouyang
> ; Zhang, Helin 
> Cc: dev@dpdk.org; Wen Wang ; Stephen Douthit 
> ;
> sta...@dpdk.org
> Subject: [PATCH v2 3/7] net/ixgbe: Check that SFF-8472 soft rate select is 
> supported before write
> 
> Make sure an SFP is really a SFF-8472 device that supports the optional
> soft rate select feature before just blindly poking those I2C registers.
> 
> Skip all I2C traffic if we know there's no SFP.
> 
> Fixes: f3430431aba ("ixgbe/base: add SFP+ dual-speed support")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Stephen Douthit 
> ---


>   /* Set RS0 */
>   status = hw->phy.ops.read_i2c_byte(hw, IXGBE_SFF_SFF_8472_OSCB,
>  IXGBE_I2C_EEPROM_DEV_ADDR2,
> diff --git a/drivers/net/ixgbe/base/ixgbe_phy.h 
> b/drivers/net/ixgbe/base/ixgbe_phy.h
> index ceefbb3e68..cd57ce040f 100644
> --- a/drivers/net/ixgbe/base/ixgbe_phy.h
> +++ b/drivers/net/ixgbe/base/ixgbe_phy.h
> @@ -21,6 +21,7 @@
>  #define IXGBE_SFF_CABLE_TECHNOLOGY   0x8
>  #define IXGBE_SFF_CABLE_SPEC_COMP0x3C
>  #define IXGBE_SFF_SFF_8472_SWAP  0x5C
> +#define IXGBE_SFF_SFF_8472_EOPT  0x5D

Looks like this is YOUR platform specific, then this patchset can't be
merged. : - (

>  #define IXGBE_SFF_SFF_8472_COMP  0x5E
>  #define IXGBE_SFF_SFF_8472_OSCB  0x6E
>  #define IXGBE_SFF_SFF_8472_ESCB  0x76
> @@ -48,6 +49,8 @@
>  #define IXGBE_SFF_SOFT_RS_SELECT_10G 0x8
> --
> 2.31.1