RE: [PATCH v6] net/i40e: add outer VLAN processing

2022-06-10 Thread Liu, KevinX



> -Original Message-
> From: Zhang, Qi Z 
> Sent: 2022年6月10日 16:07
> To: Liu, KevinX ; dev@dpdk.org
> Cc: Zhang, Yuying ; Xing, Beilei
> ; Yang, SteveX ; Zhang,
> RobinX ; Liu, KevinX 
> Subject: RE: [PATCH v6] net/i40e: add outer VLAN processing
> 
> 
> 
> > -Original Message-
> > From: Kevin Liu 
> > Sent: Friday, June 10, 2022 11:52 PM
> > To: dev@dpdk.org
> > Cc: Zhang, Yuying ; Xing, Beilei
> > ; Yang, SteveX ; Zhang,
> > RobinX ; Liu, KevinX 
> > Subject: [PATCH v6] net/i40e: add outer VLAN processing
> >
> > From: Robin Zhang 
> >
> > Outer VLAN processing is supported after firmware v8.4, kernel driver
> > also change the default behavior to support this feature. To align
> > with kernel driver, add support for outer VLAN processing in DPDK.
> >
> > But it is forbidden for firmware to change the Inner/Outer VLAN
> > configuration while there are MAC/VLAN filters in the switch table.
> > Therefore, we need to clear the MAC table before setting config, and
> > then restore the MAC table after setting.
> >
> > This will not impact on an old firmware.
> >
> > Signed-off-by: Robin Zhang 
> > Signed-off-by: Kevin Liu 
> > ---
> >  drivers/net/i40e/i40e_ethdev.c | 94
> > --
> >  drivers/net/i40e/i40e_ethdev.h |  3 ++
> >  2 files changed, 92 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 755786dc10..c708bdb0d4 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -2575,6 +2575,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
> > struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> > >dev_private);
> > struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> > struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
> > +   struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> > struct i40e_filter_control_settings settings;
> > struct rte_flow *p_flow;
> > uint32_t reg;
> > @@ -2587,6 +2588,18 @@ i40e_dev_close(struct rte_eth_dev *dev)
> > if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > return 0;
> >
> > +   /*
> > +* It is a workaround, when firmware > v8.3, if the double VLAN
> > +* is disabled when the program exits, an abnormal error will occur
> > +* on the NIC. Need to enable double VLAN when dev is closed.
> > +*/
> > +   if (pf->is_outer_vlan_processing) {
> > +   if (!(rxmode->offloads &
> > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
> > +   rxmode->offloads |=
> > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> > +   i40e_vlan_offload_set(dev,
> > RTE_ETH_VLAN_EXTEND_MASK);
> > +   }
> > +   }
> > +
> > ret = rte_eth_switch_domain_free(pf->switch_domain_id);
> > if (ret)
> > PMD_INIT_LOG(WARNING, "failed to free switch
> domain: %d", ret); @@
> > -3909,6 +3922,7 @@ i40e_vlan_tpid_set(struct rte_eth_dev *dev,
> > struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-
> > >dev_private);
> > int qinq = dev->data->dev_conf.rxmode.offloads &
> >RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> > +   u16 sw_flags = 0, valid_flags = 0;
> > int ret = 0;
> >
> > if ((vlan_type != RTE_ETH_VLAN_TYPE_INNER && @@ -3927,15
> > +3941,32 @@ i40e_vlan_tpid_set(struct rte_eth_dev *dev,
> > /* 802.1ad frames ability is added in NVM API 1.7*/
> > if (hw->flags & I40E_HW_FLAG_802_1AD_CAPABLE) {
> > if (qinq) {
> > +   if (pf->is_outer_vlan_processing) {
> > +   sw_flags =
> > I40E_AQ_SET_SWITCH_CFG_OUTER_VLAN;
> > +   valid_flags =
> > I40E_AQ_SET_SWITCH_CFG_OUTER_VLAN;
> > +   }
> > if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
> > hw->first_tag = rte_cpu_to_le_16(tpid);
> > else if (vlan_type == RTE_ETH_VLAN_TYPE_INNER)
> > hw->second_tag = rte_cpu_to_le_16(tpid);
> > } else {
> > -   if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
> > -   hw->second_tag = rte_cpu_to_le_16(tpid);
> > +   /*
> > +* When firmware > 8.3, if tpid is equal to 0x88A8,
>

RE: [PATCH v7] net/i40e: add outer VLAN processing

2022-06-12 Thread Liu, KevinX
Hi, Ben
This patch can only take effect after firmware v8.6. It cannot be used on 
firmware v8.4 and v8.5. The reason is that the firmware team gave a clear reply 
that there are many related bugs in firmware v8.4 and v8.5, and they were fixed 
in firmware v8.6. The firmware team recommends using v8.6.

Regards
Kevin

From: Ben Magistro 
Sent: 2022年6月10日 22:27
To: Liu, KevinX 
Cc: dev@dpdk.org; Zhang, Yuying ; Xing, Beilei 
; Yang, SteveX ; Zhang, RobinX 

Subject: Re: [PATCH v7] net/i40e: add outer VLAN processing

I'm trying to understand if this change is at all related to an issue we are 
experiencing with newer firmwares 
(https://mails.dpdk.org/archives/dev/2022-April/238621.html) that happened to 
start with 8.4 and affected qinq offload processing.  I can say loading this 
patch and running testpmd does not seem to have any effect on the issue we are 
experiencing.

Side note, there is also a minor typo in the comment block "i40e_vlan_tpie_set" 
vs "i40e_vlan_tpid_set".

Thanks

On Fri, Jun 10, 2022 at 4:30 AM Kevin Liu 
mailto:kevinx@intel.com>> wrote:
From: Robin Zhang mailto:robinx.zh...@intel.com>>

Outer VLAN processing is supported after firmware v8.4, kernel driver
also change the default behavior to support this feature. To align with
kernel driver, add support for outer VLAN processing in DPDK.

But it is forbidden for firmware to change the Inner/Outer VLAN
configuration while there are MAC/VLAN filters in the switch table.
Therefore, we need to clear the MAC table before setting config,
and then restore the MAC table after setting.

This will not impact on an old firmware.

Signed-off-by: Robin Zhang 
mailto:robinx.zh...@intel.com>>
Signed-off-by: Kevin Liu mailto:kevinx@intel.com>>
---
 drivers/net/i40e/i40e_ethdev.c | 94 --
 drivers/net/i40e/i40e_ethdev.h |  3 ++
 2 files changed, 92 insertions(+), 5 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 755786dc10..4cae163cb9 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2575,6 +2575,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
+   struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
struct i40e_filter_control_settings settings;
struct rte_flow *p_flow;
uint32_t reg;
@@ -2587,6 +2588,18 @@ i40e_dev_close(struct rte_eth_dev *dev)
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return 0;

+   /*
+* It is a workaround, if the double VLAN is disabled when
+* the program exits, an abnormal error will occur on the
+* NIC. Need to enable double VLAN when dev is closed.
+*/
+   if (pf->fw8_3gt) {
+   if (!(rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
+   rxmode->offloads |= RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
+   i40e_vlan_offload_set(dev, RTE_ETH_VLAN_EXTEND_MASK);
+   }
+   }
+
ret = rte_eth_switch_domain_free(pf->switch_domain_id);
if (ret)
PMD_INIT_LOG(WARNING, "failed to free switch domain: %d", ret);
@@ -3909,6 +3922,7 @@ i40e_vlan_tpid_set(struct rte_eth_dev *dev,
struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
int qinq = dev->data->dev_conf.rxmode.offloads &
   RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
+   u16 sw_flags = 0, valid_flags = 0;
int ret = 0;

if ((vlan_type != RTE_ETH_VLAN_TYPE_INNER &&
@@ -3927,15 +3941,32 @@ i40e_vlan_tpid_set(struct rte_eth_dev *dev,
/* 802.1ad frames ability is added in NVM API 1.7*/
if (hw->flags & I40E_HW_FLAG_802_1AD_CAPABLE) {
if (qinq) {
+   if (pf->fw8_3gt) {
+   sw_flags = I40E_AQ_SET_SWITCH_CFG_OUTER_VLAN;
+   valid_flags = I40E_AQ_SET_SWITCH_CFG_OUTER_VLAN;
+   }
if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
hw->first_tag = rte_cpu_to_le_16(tpid);
else if (vlan_type == RTE_ETH_VLAN_TYPE_INNER)
hw->second_tag = rte_cpu_to_le_16(tpid);
} else {
-   if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
-   hw->second_tag = rte_cpu_to_le_16(tpid);
+   /*
+* If tpid is equal to 0x88A8, indicates that the
+* disable double VLAN operation is in progress.
+* Need set

RE: [PATCH v7] net/i40e: add outer VLAN processing

2022-06-13 Thread Liu, KevinX
Hi, Yuying

> -Original Message-
> From: Zhang, Yuying 
> Sent: 2022年6月14日 10:44
> To: Liu, KevinX ; dev@dpdk.org
> Cc: Xing, Beilei ; Yang, SteveX
> ; Zhang, RobinX 
> Subject: RE: [PATCH v7] net/i40e: add outer VLAN processing
> 
> Hi Kevin,
> 
> > -----Original Message-
> > From: Liu, KevinX 
> > Sent: Saturday, June 11, 2022 12:30 AM
> > To: dev@dpdk.org
> > Cc: Zhang, Yuying ; Xing, Beilei
> > ; Yang, SteveX ; Zhang,
> > RobinX ; Liu, KevinX 
> > Subject: [PATCH v7] net/i40e: add outer VLAN processing
> >
> > From: Robin Zhang 
> >
> > Outer VLAN processing is supported after firmware v8.4, kernel driver
> > also
> 
> Since this patch can only be enabled with firmware v8.6, should you sync with
> dpdk here?
OK, I'll revise it here.
> 
> > change the default behavior to support this feature. To align with
> > kernel driver, add support for outer VLAN processing in DPDK.
> >
> > But it is forbidden for firmware to change the Inner/Outer VLAN
> > configuration while there are MAC/VLAN filters in the switch table.
> > Therefore, we need to clear the MAC table before setting config, and
> > then restore the MAC table after setting.
> >
> > This will not impact on an old firmware.
> >
> > Signed-off-by: Robin Zhang 
> > Signed-off-by: Kevin Liu 
> > ---
> >  drivers/net/i40e/i40e_ethdev.c | 94
> > --
> >  drivers/net/i40e/i40e_ethdev.h |  3 ++
> >  2 files changed, 92 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 755786dc10..4cae163cb9 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -2575,6 +2575,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
> > struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> > >dev_private);
> > struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> > struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
> > +   struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> > struct i40e_filter_control_settings settings;
> > struct rte_flow *p_flow;
> > uint32_t reg;
> > @@ -2587,6 +2588,18 @@ i40e_dev_close(struct rte_eth_dev *dev)
> > if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > return 0;
> >
> > +   /*
> > +* It is a workaround, if the double VLAN is disabled when
> > +* the program exits, an abnormal error will occur on the
> > +* NIC. Need to enable double VLAN when dev is closed.
> > +*/
> 
> What is the root cause of this error, I suggest finding a true fix instead of
> adding additonal process here.
About this error, dpdk has reported a known issue. Because it doesn't know the 
root cause of the problem, it adds a workaround here to temporarily avoid some 
problems.
> 
> > +   if (pf->fw8_3gt) {
> > +   if (!(rxmode->offloads &
> > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
> > +   rxmode->offloads |=
> > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> > +   i40e_vlan_offload_set(dev,
> > RTE_ETH_VLAN_EXTEND_MASK);
> > +   }
> > +   }
> > +
> > ret = rte_eth_switch_domain_free(pf->switch_domain_id);
> > if (ret)
> > PMD_INIT_LOG(WARNING, "failed to free switch
> > domain: %d", ret); @@ -3909,6 +3922,7 @@ i40e_vlan_tpid_set(struct
> > rte_eth_dev *dev,
> > struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-
> > >dev_private);
> > int qinq = dev->data->dev_conf.rxmode.offloads &
> >RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> > +   u16 sw_flags = 0, valid_flags = 0;
> > int ret = 0;
> >
> > if ((vlan_type != RTE_ETH_VLAN_TYPE_INNER && @@ -3927,15
> > +3941,32 @@ i40e_vlan_tpid_set(struct rte_eth_dev *dev,
> > /* 802.1ad frames ability is added in NVM API 1.7*/
> > if (hw->flags & I40E_HW_FLAG_802_1AD_CAPABLE) {
> > if (qinq) {
> > +   if (pf->fw8_3gt) {
> > +   sw_flags =
> > I40E_AQ_SET_SWITCH_CFG_OUTER_VLAN;
> > +   valid_flags =
> > I40E_AQ_SET_SWITCH_CFG_OUTER_VLAN;
> > +   }
> > if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
> > hw->first_tag = rte_cpu_to_le_16(tpid);
> > else if (vlan_type == RTE_ETH_VLAN_TY

RE: [PATCH v7] net/i40e: add outer VLAN processing

2022-06-14 Thread Liu, KevinX
Yes, that is necessary. Thank you very much for your review!

> -Original Message-
> From: Zhang, Yuying 
> Sent: 2022年6月14日 16:40
> To: Liu, KevinX ; dev@dpdk.org
> Cc: Xing, Beilei ; Yang, SteveX
> ; Zhang, RobinX 
> Subject: RE: [PATCH v7] net/i40e: add outer VLAN processing
> 
> Hi Kevin,
> 
> Workaround should be replaced when root cause be found.
> 
> Best regards,
> Yuying
> 
> > -Original Message-
> > From: Liu, KevinX 
> > Sent: Tuesday, June 14, 2022 11:07 AM
> > To: Zhang, Yuying ; dev@dpdk.org
> > Cc: Xing, Beilei ; Yang, SteveX
> > ; Zhang, RobinX 
> > Subject: RE: [PATCH v7] net/i40e: add outer VLAN processing
> >
> > Hi, Yuying
> >
> > > -Original Message-
> > > From: Zhang, Yuying 
> > > Sent: 2022年6月14日 10:44
> > > To: Liu, KevinX ; dev@dpdk.org
> > > Cc: Xing, Beilei ; Yang, SteveX
> > > ; Zhang, RobinX 
> > > Subject: RE: [PATCH v7] net/i40e: add outer VLAN processing
> > >
> > > Hi Kevin,
> > >
> > > > -Original Message-
> > > > From: Liu, KevinX 
> > > > Sent: Saturday, June 11, 2022 12:30 AM
> > > > To: dev@dpdk.org
> > > > Cc: Zhang, Yuying ; Xing, Beilei
> > > > ; Yang, SteveX ;
> > > > Zhang, RobinX ; Liu, KevinX
> > > > 
> > > > Subject: [PATCH v7] net/i40e: add outer VLAN processing
> > > >
> > > > From: Robin Zhang 
> > > >
> > > > Outer VLAN processing is supported after firmware v8.4, kernel
> > > > driver also
> > >
> > > Since this patch can only be enabled with firmware v8.6, should you
> > > sync with dpdk here?
> > OK, I'll revise it here.
> > >
> > > > change the default behavior to support this feature. To align with
> > > > kernel driver, add support for outer VLAN processing in DPDK.
> > > >
> > > > But it is forbidden for firmware to change the Inner/Outer VLAN
> > > > configuration while there are MAC/VLAN filters in the switch table.
> > > > Therefore, we need to clear the MAC table before setting config,
> > > > and then restore the MAC table after setting.
> > > >
> > > > This will not impact on an old firmware.
> > > >
> > > > Signed-off-by: Robin Zhang 
> > > > Signed-off-by: Kevin Liu 
> 
> Acked-by: Yuying Zhang 
> 
> > > > ---
> > > >  drivers/net/i40e/i40e_ethdev.c | 94
> > > > --
> > > >  drivers/net/i40e/i40e_ethdev.h |  3 ++
> > > >  2 files changed, 92 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > > b/drivers/net/i40e/i40e_ethdev.c index 755786dc10..4cae163cb9
> > > > 100644
> > > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > > @@ -2575,6 +2575,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
> > > > struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> > > > >dev_private);
> > > > struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> > > > struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
> > > > +   struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> > > > struct i40e_filter_control_settings settings;
> > > > struct rte_flow *p_flow;
> > > > uint32_t reg;
> > > > @@ -2587,6 +2588,18 @@ i40e_dev_close(struct rte_eth_dev *dev)
> > > > if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > > > return 0;
> > > >
> > > > +   /*
> > > > +* It is a workaround, if the double VLAN is disabled when
> > > > +* the program exits, an abnormal error will occur on the
> > > > +* NIC. Need to enable double VLAN when dev is closed.
> > > > +*/
> > >
> > > What is the root cause of this error, I suggest finding a true fix
> > > instead of adding additonal process here.
> > About this error, dpdk has reported a known issue. Because it doesn't
> > know the root cause of the problem, it adds a workaround here to
> > temporarily avoid some problems.
> > >
> > > > +   if (pf->fw8_3gt) {
> > > > +   if (!(rxmode->offloads &
> > > > RTE_ETH_RX_OFFL

RE: [PATCH] net/i40e: restore disable double VLAN by default

2022-07-06 Thread Liu, KevinX
Ok, I'll tidy it up again.

> -Original Message-
> From: Yang, Qiming 
> Sent: 2022年7月7日 14:55
> To: Liu, KevinX ; dev@dpdk.org
> Cc: Xing, Beilei ; Zhang, Yuying
> ; Yang, SteveX ; Liu,
> KevinX 
> Subject: RE: [PATCH] net/i40e: restore disable double VLAN by default
> 
> Hi,
> 
> > -Original Message-
> > From: Kevin Liu 
> > Sent: Thursday, July 7, 2022 18:48
> > To: dev@dpdk.org
> > Cc: Xing, Beilei ; Zhang, Yuying
> > ; Yang, SteveX ; Liu,
> > KevinX 
> > Subject: [PATCH] net/i40e: restore disable double VLAN by default
> >
> > Restore disable double VLAN by default.
> 
> Please add detail reason, like will caused performance drop issue.
> >
> > Fixes: ae97b8b89826 ("net/i40e: fix error disable double VLAN")
> > Signed-off-by: Kevin Liu 
> > ---
> >  doc/guides/nics/i40e.rst   |  6 +++---
> >  drivers/net/i40e/i40e_ethdev.c | 12 
> >  2 files changed, 3 insertions(+), 15 deletions(-)
> >
> > diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst index
> > 85fdc4944d..f61c327726 100644
> > --- a/doc/guides/nics/i40e.rst
> > +++ b/doc/guides/nics/i40e.rst
> > @@ -969,10 +969,10 @@ it will fail and return the info "Conflict with
> > the first rule's input set",  which means the current rule's input set
> > conflicts with the first rule's.
> >  Remove the first rule if want to change the input set of the PCTYPE.
> >
> > -Disable QinQ is not supported when FW >= 8.4 -
> > 
> > +To use VLAN functions, need to enable QinQ when FW >= 8.4
> 
> Not only this issue, you should cover all the issue we know.
> 
> Vlan related feature miss when FW>=8.4
> ~
> If upgrade FW to version 8.4 and higher, some vlan related issue exist:
> 1.vlan tci input set not work
> 2.tpid set fail
> 3.need enable qinq before use vlan filter 4.outer vlan strip fail
> 
> >
> +~
> >
> > -If upgrade FW to version 8.4 and higher, enable QinQ by default and
> > disable QinQ is not supported.
> > +If upgrade FW to version 8.4 and higher, when using VLAN functions,
> > +need to
> > enable QinQ.
> >
> >
> >  Example of getting best performance with l3fwd example diff --git
> > a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> > index
> > 684e095026..117dd85c11 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -4027,12 +4027,6 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev,
> > int
> > mask)
> > }
> >
> > if (mask & RTE_ETH_VLAN_EXTEND_MASK) {
> > -   /* Double VLAN not allowed to be disabled.*/
> > -   if (pf->fw8_3gt && !(rxmode->offloads &
> > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)) {
> > -   PMD_DRV_LOG(WARNING,
> > -   "Disable double VLAN is not allowed after
> > firmwarev8.3!");
> > -   return 0;
> > -   }
> > i = 0;
> > num = vsi->mac_num;
> > mac_filter = rte_zmalloc("mac_filter_info_data",
> > @@ -6296,7 +6290,6 @@ int i40e_vsi_cfg_inner_vlan_stripping(struct
> > i40e_vsi *vsi, bool on)  static int  i40e_dev_init_vlan(struct rte_eth_dev
> *dev)  {
> > -   struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-
> > >dev_private);
> > struct rte_eth_dev_data *data = dev->data;
> > int ret;
> > int mask = 0;
> > @@ -6307,11 +6300,6 @@ i40e_dev_init_vlan(struct rte_eth_dev *dev)
> >RTE_ETH_VLAN_FILTER_MASK |
> >RTE_ETH_VLAN_EXTEND_MASK;
> >
> > -   /* Double VLAN be enabled by default.*/
> > -   if (pf->fw8_3gt) {
> > -   struct rte_eth_rxmode *rxmode = &dev->data-
> > >dev_conf.rxmode;
> > -   rxmode->offloads |= RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> > -   }
> > ret = i40e_vlan_offload_set(dev, mask);
> > if (ret) {
> > PMD_DRV_LOG(INFO, "Failed to update vlan offload");
> > --
> > 2.34.1



RE: [PATCH] net/ice: fix missing MTU value setting

2022-05-17 Thread Liu, KevinX



> -Original Message-
> From: Zhang, Qi Z 
> Sent: 2022年5月17日 15:50
> To: Liu, KevinX ; dev@dpdk.org
> Cc: Yang, Qiming ; Yang, SteveX
> ; sta...@dpdk.org
> Subject: RE: [PATCH] net/ice: fix missing MTU value setting
> 
> 
> 
> > -----Original Message-
> > From: Liu, KevinX 
> > Sent: Friday, April 8, 2022 3:57 AM
> > To: dev@dpdk.org
> > Cc: Yang, Qiming ; Zhang, Qi Z
> > ; Yang, SteveX ; Liu,
> > KevinX ; sta...@dpdk.org
> > Subject: [PATCH] net/ice: fix missing MTU value setting
> >
> > In the DCF module, Missing maximum and minimum MTU value settings.
> >
> > This patch adds the settings of the maximum and minimum MTU to
> > correctly calculate the MTU value.
> >
> > Fixes: 2fe6f1b76279 ("drivers/net: advertise no support for keeping
> > flow rules")
> 
> This does not looks like a correct fixline
> 
I reconfirmed that fixline is correct.



RE: [PATCH] net/ice: fix missing MTU value setting

2022-05-17 Thread Liu, KevinX
Sorry, I misunderstood, I will send v2

> -Original Message-
> From: Liu, KevinX
> Sent: 2022年5月17日 16:00
> To: Zhang, Qi Z ; dev@dpdk.org
> Cc: Yang, Qiming ; Yang, SteveX
> ; sta...@dpdk.org
> Subject: RE: [PATCH] net/ice: fix missing MTU value setting
> 
> 
> 
> > -Original Message-
> > From: Zhang, Qi Z 
> > Sent: 2022年5月17日 15:50
> > To: Liu, KevinX ; dev@dpdk.org
> > Cc: Yang, Qiming ; Yang, SteveX
> > ; sta...@dpdk.org
> > Subject: RE: [PATCH] net/ice: fix missing MTU value setting
> >
> >
> >
> > > -Original Message-
> > > From: Liu, KevinX 
> > > Sent: Friday, April 8, 2022 3:57 AM
> > > To: dev@dpdk.org
> > > Cc: Yang, Qiming ; Zhang, Qi Z
> > > ; Yang, SteveX ; Liu,
> > > KevinX ; sta...@dpdk.org
> > > Subject: [PATCH] net/ice: fix missing MTU value setting
> > >
> > > In the DCF module, Missing maximum and minimum MTU value settings.
> > >
> > > This patch adds the settings of the maximum and minimum MTU to
> > > correctly calculate the MTU value.
> > >
> > > Fixes: 2fe6f1b76279 ("drivers/net: advertise no support for keeping
> > > flow rules")
> >
> > This does not looks like a correct fixline
> >
> I reconfirmed that fixline is correct.



RE: [PATCH] net/ice: fix error forwarding IPv6 VXLAN packet

2022-01-04 Thread Liu, KevinX



> -Original Message-
> From: Zhang, Qi Z 
> Sent: 2022年1月2日 16:46
> To: Liu, KevinX ; dev@dpdk.org
> Cc: Zhang, RobinX ; Wang, Jie1X
> ; Liu, KevinX 
> Subject: RE: [PATCH] net/ice: fix error forwarding IPv6 VXLAN packet
> 
> 
> 
> > -Original Message-
> > From: Kevin Liu 
> > Sent: Wednesday, December 8, 2021 5:56 PM
> > To: dev@dpdk.org
> > Cc: Zhang, RobinX ; Wang, Jie1X
> > ; Liu, KevinX 
> > Subject: [PATCH] net/ice: fix error forwarding IPv6 VXLAN packet
> >
> > In ice_txd_enable_offload(), when set tunnel packet Tx checksum
> > offload enable, td_offset should be set with outer l2/l3 len instead of 
> > inner
> l2/l3 len.
> >
> > This patch fix the bug that the checksum engine can forward tunnle packets.
> 
> s/tunnle/tunnel
> 
> >
> > Fixes: 28f9002ab67f ("net/ice: add Tx AVX512 offload path")
> >
> > Signed-off-by: Kevin Liu 
> > ---
> >  drivers/net/ice/ice_rxtx_vec_common.h | 52
> > +++
> >  1 file changed, 37 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/net/ice/ice_rxtx_vec_common.h
> > b/drivers/net/ice/ice_rxtx_vec_common.h
> > index dfe60c81d9..8ff01046e1 100644
> > --- a/drivers/net/ice/ice_rxtx_vec_common.h
> > +++ b/drivers/net/ice/ice_rxtx_vec_common.h
> > @@ -364,23 +364,45 @@ ice_txd_enable_offload(struct rte_mbuf *tx_pkt,
> > uint32_t td_offset = 0;
> >
> >  /* Tx Checksum Offload */
> > -/* SET MACLEN */
> > -td_offset |= (tx_pkt->l2_len >> 1) <<
> > +/*Tunnel package usage outer len enable L2/L3 checksum offload*/ if
> > +(ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK) {
> > +/* SET MACLEN */
> > +td_offset |= (tx_pkt->outer_l2_len >> 1) <<
> >  ICE_TX_DESC_LEN_MACLEN_S;
> >
> > -/* Enable L3 checksum offload */
> > -if (ol_flags & RTE_MBUF_F_TX_IP_CKSUM) { -td_cmd |=
> > ICE_TX_DESC_CMD_IIPT_IPV4_CSUM; -td_offset |= (tx_pkt->l3_len >> 2)
> <<
> > -ICE_TX_DESC_LEN_IPLEN_S; -} else if (ol_flags & RTE_MBUF_F_TX_IPV4) {
> > -td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV4; -td_offset |= (tx_pkt->l3_len >>
> > 2) << -ICE_TX_DESC_LEN_IPLEN_S; -} else if (ol_flags &
> > RTE_MBUF_F_TX_IPV6) { -td_cmd |= ICE_TX_DESC_CMD_IIPT_IPV6; -
> td_offset
> > |= (tx_pkt->l3_len >> 2) << -ICE_TX_DESC_LEN_IPLEN_S;
> > +/* Enable L3 checksum offload */
> > +if (ol_flags & RTE_MBUF_F_TX_IP_CKSUM) { td_cmd |=
> > +ICE_TX_DESC_CMD_IIPT_IPV4_CSUM; td_offset |= (tx_pkt-
> >outer_l3_len >>
> > +2) << ICE_TX_DESC_LEN_IPLEN_S;
> 
> Is this fix also cover IPv4 Tx checksum offload?
Yes, cover it.
> Please refine the title to reflect what exactly the patch does if necessary.
> 
> Btw, could you also check if any fix needed in ice_txd_enable_checksum?
> I saw inconsistent offset configure on l3_len between scaler path and vector
> path.
> Its better to always keep them identical.
> 
> 
I checked code in ice_txd_enable_checksum, and it should also be fixed here. I 
will send a new patch of v2.

Thanks
Kevin


RE: [PATCH] net/i40e: fix incorrect VLAN stripping for QinQ

2022-09-01 Thread Liu, KevinX



> -Original Message-
> From: Yang, Qiming 
> Sent: Friday, September 2, 2022 11:29 AM
> To: Liu, KevinX ; dev@dpdk.org
> Cc: Zhang, Yuying ; Xing, Beilei
> ; Yang, SteveX ; Liu, KevinX
> 
> Subject: RE: [PATCH] net/i40e: fix incorrect VLAN stripping for QinQ
> 
> 
> 
> > -Original Message-
> > From: Kevin Liu 
> > Sent: Thursday, September 1, 2022 6:06 PM
> > To: dev@dpdk.org
> > Cc: Zhang, Yuying ; Xing, Beilei
> > ; Yang, SteveX ; Liu,
> > KevinX 
> > Subject: [PATCH] net/i40e: fix incorrect VLAN stripping for QinQ
> >
> > QinQ enable, when enable strip function, it is wrong to strip inner
> > VLAN of double VLAN package. The correct action is outer VLAN is
> > stripped. So, need to configure 'outer_vlan_flags' to update vsi.
> >
> 
> This commit message don't explain why we need to change the inner vlan strip
> to outer vlan.
> We should align with kernel driver's new behavior.
> And from my point of view, we have no need to add too many new functions for
> this fix.
> Can you work out another simple design?

Thanks for your suggestion. I'll rework it.
> 
> > When enable QinQ strip function, need to set 'port_vlan_flags' to
> > configure inner VLAN strip.
> >
> > Signed-off-by: Kevin Liu 
> > ---
> >  doc/guides/nics/i40e.rst   |   2 -
> >  drivers/net/i40e/i40e_ethdev.c | 170 +++--
> >  drivers/net/i40e/i40e_ethdev.h |   4 +
> >  3 files changed, 164 insertions(+), 12 deletions(-)
> >
> > diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst index
> > 15b796e67a..ffcb2a2220 100644
> > --- a/doc/guides/nics/i40e.rst
> > +++ b/doc/guides/nics/i40e.rst
> > @@ -982,8 +982,6 @@ Vlan related Features miss when FW >= 8.4  If FW
> > version >= 8.4, there'll be some Vlan related issues:
> >
> >  #. TCI input set for QinQ  is invalid.
> > -#. Fail to configure TPID for QinQ.
> > -#. Fail to strip outer Vlan.
> >
> >  Example of getting best performance with l3fwd example
> >  --
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 27cfda6ff8..0c3009ebfa 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -52,6 +52,8 @@
> >  #define I40E_VSI_TSR_QINQ_STRIP0x4010
> >  #define I40E_VSI_TSR(_i)   (0x00050800 + ((_i) * 4))
> >
> > +#define I40E_OVLAN_EMOD_SHIFT(x) ((x) <<
> > I40E_AQ_VSI_OVLAN_EMOD_SHIFT)
> > +
> >  /* Maximun number of capability elements */
> >  #define I40E_MAX_CAP_ELE_NUM   128
> >
> > @@ -4011,10 +4013,15 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev,
> > int mask)
> >
> > if (mask & RTE_ETH_VLAN_STRIP_MASK) {
> > /* Enable or disable VLAN stripping */
> > -   if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_STRIP)
> > -   i40e_vsi_config_vlan_stripping(vsi, TRUE);
> > -   else
> > +   if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_STRIP)
> > {
> > +   if (rxmode->offloads &
> > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND)
> > +   i40e_vsi_config_vlan_stripping_v1(vsi, TRUE);
> > +   else
> > +   i40e_vsi_config_vlan_stripping(vsi, TRUE);
> > +   } else {
> > i40e_vsi_config_vlan_stripping(vsi, FALSE);
> > +   i40e_vsi_config_vlan_stripping_v1(vsi, FALSE);
> > +   }
> > }
> >
> > if (mask & RTE_ETH_VLAN_EXTEND_MASK) { @@ -4068,6 +4075,10
> @@
> > i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
> > if (rxmode->offloads &
> > RTE_ETH_RX_OFFLOAD_VLAN_EXTEND) {
> > if (pf->fw8_3gt) {
> > i40e_vsi_config_qinq(vsi, TRUE);
> > +   if (rxmode->offloads &
> > RTE_ETH_RX_OFFLOAD_VLAN_STRIP) {
> > +   i40e_vsi_config_vlan_stripping(vsi,
> > FALSE);
> > +   i40e_vsi_config_vlan_stripping_v1(vsi,
> > TRUE);
> > +   }
> > } else {
> > i40e_vsi_config_double_vlan(vsi, TRUE);
> > /* Set global registers with default ethertype.
> > */ @@ -4077,10 +4088,15 @@ i40e_vlan_offload_set(struct rte_eth_de

RE: [PATCH] net/i40e: fix single VLAN cannot work normal

2022-09-05 Thread Liu, KevinX



> -Original Message-
> From: Zhang, Yuying 
> Sent: Tuesday, September 6, 2022 10:16 AM
> To: Liu, KevinX ; dev@dpdk.org
> Cc: Xing, Beilei ; Yang, SteveX 
> Subject: RE: [PATCH] net/i40e: fix single VLAN cannot work normal
> 
> Hi,
> 
> > -Original Message-
> > From: Liu, KevinX 
> > Sent: Friday, August 19, 2022 12:04 AM
> > To: dev@dpdk.org
> > Cc: Zhang, Yuying ; Xing, Beilei
> > ; Yang, SteveX ; Liu,
> > KevinX 
> > Subject: [PATCH] net/i40e: fix single VLAN cannot work normal
> >
> > After disable QinQ, single VLAN can not work normal.
> > The reason is that QinQ is not disabled correctly.
> >
> > Before configuring QinQ, need to back up and clean MAC/VLAN filters of
> > all ports. After configuring QinQ, restore MAC/VLAN filters of all
> > ports. When disable QinQ, need to set valid_flags to 0x0008 and set 
> > first_tag
> to 0x88a8.
> >
> > Signed-off-by: Kevin Liu 
> > ---
> >  doc/guides/nics/i40e.rst   |   1 -
> >  drivers/net/i40e/i40e_ethdev.c | 159
> > +++--
> >  2 files changed, 111 insertions(+), 49 deletions(-)
> >
> > diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst index
> > abb99406b3..15b796e67a 100644
> > --- a/doc/guides/nics/i40e.rst
> > +++ b/doc/guides/nics/i40e.rst
> > @@ -983,7 +983,6 @@ If FW version >= 8.4, there'll be some Vlan related
> issues:
> >
> >  #. TCI input set for QinQ  is invalid.
> >  #. Fail to configure TPID for QinQ.
> > -#. Need to enable QinQ before enabling Vlan filter.
> >  #. Fail to strip outer Vlan.
> >
> >  Example of getting best performance with l3fwd example diff --git
> > a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> > index
> > 67d79de08d..27cfda6ff8 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -325,6 +325,7 @@ static int i40e_veb_release(struct i40e_veb *veb);
> > static struct i40e_veb *i40e_veb_setup(struct i40e_pf *pf,
> > struct i40e_vsi *vsi);
> >  static int i40e_vsi_config_double_vlan(struct i40e_vsi *vsi, int on);
> > +static int i40e_vsi_config_qinq(struct i40e_vsi *vsi, int on);
> >  static inline int i40e_find_all_mac_for_vlan(struct i40e_vsi *vsi,
> >  struct i40e_macvlan_filter *mv_f,
> >  int num,
> > @@ -3909,7 +3910,6 @@ i40e_vlan_tpid_set(struct rte_eth_dev *dev,
> > struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-
> > >dev_private);
> > int qinq = dev->data->dev_conf.rxmode.offloads &
> >RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> > -   u16 sw_flags = 0, valid_flags = 0;
> > int ret = 0;
> >
> > if ((vlan_type != RTE_ETH_VLAN_TYPE_INNER && @@ -3928,10
> > +3928,6 @@ i40e_vlan_tpid_set(struct rte_eth_dev *dev,
> > /* 802.1ad frames ability is added in NVM API 1.7*/
> > if (hw->flags & I40E_HW_FLAG_802_1AD_CAPABLE) {
> > if (qinq) {
> > -   if (pf->fw8_3gt) {
> > -   sw_flags =
> > I40E_AQ_SET_SWITCH_CFG_OUTER_VLAN;
> > -   valid_flags =
> > I40E_AQ_SET_SWITCH_CFG_OUTER_VLAN;
> > -   }
> > if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
> > hw->first_tag = rte_cpu_to_le_16(tpid);
> > else if (vlan_type == RTE_ETH_VLAN_TYPE_INNER) @@
> > -3940,8 +3936,8 @@ i40e_vlan_tpid_set(struct rte_eth_dev *dev,
> > if (vlan_type == RTE_ETH_VLAN_TYPE_OUTER)
> > hw->second_tag = rte_cpu_to_le_16(tpid);
> > }
> > -   ret = i40e_aq_set_switch_config(hw, sw_flags,
> > -   valid_flags, 0, NULL);
> > +   ret = i40e_aq_set_switch_config(hw, 0,
> > +   0, 0, NULL);
> > if (ret != I40E_SUCCESS) {
> > PMD_DRV_LOG(ERR,
> > "Set switch config failed aq_err: %d", @@ -
> > 3993,11 +3989,15 @@ static int  i40e_vlan_offload_set(struct
> > rte_eth_dev *dev, int mask)  {
> > struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-
> > >dev_private);
> > +   struct i40e_mac_filter_info *vmac_filter[RTE_MAX_ETHPORTS];
> > +   struct i40e_vsi *vvsi[RTE_MAX_ETHPORTS];
> >   

RE: [PATCH v4] net/ice: refactor proto_ext to remove global variable

2022-08-29 Thread Liu, KevinX



> -Original Message-
> From: Zhang, Qi Z 
> Sent: 2022年8月30日 10:04
> To: Liu, KevinX ; dev@dpdk.org
> Cc: Yang, Qiming ; Yang, SteveX
> 
> Subject: RE: [PATCH v4] net/ice: refactor proto_ext to remove global variable
> 
> 
> 
> > -----Original Message-
> > From: Liu, KevinX 
> > Sent: Friday, August 26, 2022 6:15 PM
> > To: dev@dpdk.org
> > Cc: Yang, Qiming ; Zhang, Qi Z
> > ; Yang, SteveX ; Liu,
> > KevinX 
> > Subject: [PATCH v4] net/ice: refactor proto_ext to remove global
> > variable
> >
> > The ice has the feature to extract protocol fields into flex
> > descriptor by programming per queue. However, the dynamic field for
> > proto_ext are allocated by PMD, it is the responsibility of
> > application to reserved the field, before start DPDK.
> >
> > Application with parse the offset and proto_ext name to PMD with devargs.
> > Remove related private API in 'rte_pmd_ice.h' and 'rte_pmd_ice.h' file.
> >
> > Signed-off-by: Kevin Liu 
> >
> > ---
> > v2: Delete doc content related to 'rte_pmd_ice.h'.
> > ---
> > v3: Delete doc content related to 'rte_pmd_ice.h'.
> > ---
> > v4: refine code and change the check mode of dynamic field.
> > ---
> >  doc/api/doxy-api-index.md |   1 -
> >  doc/api/doxy-api.conf.in  |   1 -
> >  doc/guides/nics/ice.rst   |  33 ++--
> >  drivers/net/ice/ice_ddp_package.c |   1 -
> >  drivers/net/ice/ice_ethdev.c  | 113 ++
> >  drivers/net/ice/ice_ethdev.h  |   7 +
> >  drivers/net/ice/ice_rxtx.c|  45 ++
> >  drivers/net/ice/ice_rxtx.h|   1 +
> >  drivers/net/ice/ice_testpmd.c |   2 +-
> >  drivers/net/ice/meson.build   |   2 -
> >  drivers/net/ice/rte_pmd_ice.h | 247 --
> >  drivers/net/ice/version.map   |   7 -
> >  12 files changed, 122 insertions(+), 338 deletions(-)  delete mode
> > 100644 drivers/net/ice/rte_pmd_ice.h
> >
> > diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
> > index 186a258be4..e578800cea 100644
> > --- a/doc/api/doxy-api-index.md
> > +++ b/doc/api/doxy-api-index.md
> > @@ -44,7 +44,6 @@ The public API headers are grouped by topics:
> >[KNI](@ref rte_kni.h),
> >[ixgbe](@ref rte_pmd_ixgbe.h),
> >[i40e](@ref rte_pmd_i40e.h),
> > -  [ice](@ref rte_pmd_ice.h),
> >[iavf](@ref rte_pmd_iavf.h),
> >[ioat](@ref rte_ioat_rawdev.h),
> >[bnxt](@ref rte_pmd_bnxt.h),
> > diff --git a/doc/api/doxy-api.conf.in b/doc/api/doxy-api.conf.in index
> > 608494a7c0..6fab7436d7 100644
> > --- a/doc/api/doxy-api.conf.in
> > +++ b/doc/api/doxy-api.conf.in
> > @@ -18,7 +18,6 @@ INPUT   = @TOPDIR@/doc/api/doxy-api-
> > index.md \
> >@TOPDIR@/drivers/net/dpaa2 \
> >@TOPDIR@/drivers/net/i40e \
> >@TOPDIR@/drivers/net/iavf \
> > -  @TOPDIR@/drivers/net/ice \
> >@TOPDIR@/drivers/net/ixgbe \
> >@TOPDIR@/drivers/net/mlx5 \
> >@TOPDIR@/drivers/net/softnic \ diff --git
> > a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst index
> > 6b903b9bbc..432c6fd7ed 100644
> > --- a/doc/guides/nics/ice.rst
> > +++ b/doc/guides/nics/ice.rst
> > @@ -110,29 +110,43 @@ Runtime Config Options
> >
> >The argument format is::
> >
> > -  -a 18:00.0,proto_xtr=[...]
> > -  -a 18:00.0,proto_xtr=
> > +
> > 18:00.0,proto_xtr=[...],field_offs=<
> > offset>,
> > \
> > +  field_name=
> > +
> > + 18:00.0,proto_xtr=,field_offs=,field_name=
> >
> >Queues are grouped by ``(`` and ``)`` within the group. The ``-`` 
> > character
> >is used as a range separator and ``,`` is used as a single number 
> > separator.
> >The grouping ``()`` can be omitted for single element group. If no queues
> are
> >specified, PMD will use this protocol extraction type for all queues.
> > +  ``field_offs`` is the offset of mbuf dynamic field for protocol 
> > extraction
> data.
> > +  ``field_name`` is the name of mbuf dynamic field for protocol
> > + extraction
> > data.
> > +  ``field_offs`` and ``field_name`` will be checked whether it is
> > + valid. If it is invalid  the value needs to be reconfigured.
> 
> It is not clear here, better just describe when the configure is invalid what
> kind of failure will happen
Ok, I will modify it according to your suggestion. Thanks!


RE: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in multi-segments

2022-03-10 Thread Liu, KevinX
Hi, Ferruh

Yuying has already reviewed it days ago.
If you can, I hope you can change the status as soon as possible and try to 
merge the code in RC4.
Thank you.

> -Original Message-
> From: Zhang, Yuying 
> Sent: 2022年3月3日 14:30
> To: Zhang, Qi Z ; Liu, KevinX ;
> dev 
> Cc: Yang, Qiming ; Yang, SteveX
> ; Yigit, Ferruh ; Xing, Beilei
> ; Li, Xiaoyun ; dpdk stable
> 
> Subject: RE: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in multi-
> segments
> 
> LGTM.
> 
> > > -Original Message-
> > > From: Liu, KevinX 
> > > Sent: Wednesday, December 29, 2021 5:37 PM
> > > To: dev@dpdk.org
> > > Cc: Yang, Qiming ; Zhang, Qi Z
> > > ; Yang, SteveX ; Yigit,
> > > Ferruh ; Liu, KevinX ;
> > > sta...@dpdk.org
> > > Subject: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in
> > > multi-segments
> > >
> > > Testpmd forwards packets in checksum mode that it needs to calculate
> > > the checksum of each layer's protocol.
> > >
> > > In process_inner_cksums, when parsing tunnel packets, inner L4
> > > offset should be outer_l2_len + outer_l3_len + l2_len + l3_len.
> > >
> > > In process_outer_cksums, when parsing tunnel packets, outer L4
> > > offset should be outer_l2_len + outer_l3_len.
> > >
> > > Fixes: e6b9d6411e91 ("app/testpmd: add SW L4 checksum in multi-
> > > segments")
> > > Cc: sta...@dpdk.org
> > >
> > > Signed-off-by: Kevin Liu 
> 
> Acked-by: Yuying Zhang 
> 
> > > ---
> > >  app/test-pmd/csumonly.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> > > 02bc3929c7..c235456e58 100644
> > > --- a/app/test-pmd/csumonly.c
> > > +++ b/app/test-pmd/csumonly.c
> >
> >
> > > @@ -513,7 +513,7 @@ process_inner_cksums(void *l3_hdr, const struct
> > > testpmd_offload_info *info,  ol_flags |=
> RTE_MBUF_F_TX_UDP_CKSUM;  }
> > > else {  if (info->is_tunnel) -l4_off = info->l2_len +
> > > +l4_off = info->outer_l2_len +
> > >   info->outer_l3_len +
> > >   info->l2_len + info->l3_len;
> > >  else
> > > @@ -536,7 +536,7 @@ process_inner_cksums(void *l3_hdr, const struct
> > > testpmd_offload_info *info,  ol_flags |=
> RTE_MBUF_F_TX_TCP_CKSUM;  }
> > > else {  if (info->is_tunnel) -l4_off = info->l2_len +
> > > info->outer_l3_len +
> > > +l4_off = info->outer_l2_len + info-
> > > >outer_l3_len +
> > >   info->l2_len + info->l3_len;
> > >  else
> > >  l4_off = info->l2_len + info->l3_len; @@ -
> > > 625,7 +625,7 @@ process_outer_cksums(void *outer_l3_hdr, struct
> > > testpmd_offload_info *info,  if (udp_hdr->dgram_cksum != 0) {
> > > udp_hdr->dgram_cksum = 0;  udp_hdr->dgram_cksum =
> > > get_udptcp_checksum(m, outer_l3_hdr,
> > > -info->l2_len + info->outer_l3_len,
> > > +info->outer_l2_len + info-
> > > >outer_l3_len,
> > >  info->outer_ethertype);
> > >  }
> > >
> > > --
> > > 2.33.1
> >



RE: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in multi-segments

2022-03-11 Thread Liu, KevinX


> -Original Message-
> From: Singh, Aman Deep 
> Sent: 2022年3月11日 16:02
> To: Liu, KevinX ; Zhang, Qi Z ;
> dev ; Yigit, Ferruh 
> Cc: Yang, Qiming ; Yang, SteveX
> ; Xing, Beilei ; Li, Xiaoyun
> ; dpdk stable ; Zhang, Yuying
> 
> Subject: Re: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in multi-
> segments
> 
> Hi Kevin,
> 
> On 3/11/2022 12:34 PM, Liu, KevinX wrote:
> > Hi, Ferruh
> >
> > Yuying has already reviewed it days ago.
> > If you can, I hope you can change the status as soon as possible and try to
> merge the code in RC4.
> > Thank you.
> >
> >> -Original Message-
> >> From: Zhang, Yuying 
> >> Sent: 2022年3月3日 14:30
> >> To: Zhang, Qi Z ; Liu, KevinX
> >> ; dev 
> >> Cc: Yang, Qiming ; Yang, SteveX
> >> ; Yigit, Ferruh ;
> >> Xing, Beilei ; Li, Xiaoyun
> >> ; dpdk stable 
> >> Subject: RE: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in multi-
> >> segments
> >>
> >> LGTM.
> >>
> >>>> -Original Message-
> >>>> From: Liu, KevinX 
> >>>> Sent: Wednesday, December 29, 2021 5:37 PM
> >>>> To: dev@dpdk.org
> >>>> Cc: Yang, Qiming ; Zhang, Qi Z
> >>>> ; Yang, SteveX ;
> >>>> Yigit, Ferruh ; Liu, KevinX
> >>>> ; sta...@dpdk.org
> >>>> Subject: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in
> >>>> multi-segments
> >>>>
> >>>> Testpmd forwards packets in checksum mode that it needs to
> >>>> calculate the checksum of each layer's protocol.
> >>>>
> >>>> In process_inner_cksums, when parsing tunnel packets, inner L4
> >>>> offset should be outer_l2_len + outer_l3_len + l2_len + l3_len.
> >>>>
> >>>> In process_outer_cksums, when parsing tunnel packets, outer L4
> >>>> offset should be outer_l2_len + outer_l3_len.
> >>>>
> >>>> Fixes: e6b9d6411e91 ("app/testpmd: add SW L4 checksum in multi-
> >>>> segments")
> >>>> Cc: sta...@dpdk.org
> >>>>
> >>>> Signed-off-by: Kevin Liu 
> >> Acked-by: Yuying Zhang 
> >>
> >>>> ---
> >>>>   app/test-pmd/csumonly.c | 6 +++---
> >>>>   1 file changed, 3 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> >>>> index
> >>>> 02bc3929c7..c235456e58 100644
> >>>> --- a/app/test-pmd/csumonly.c
> >>>> +++ b/app/test-pmd/csumonly.c
> >>>
> >>>> @@ -513,7 +513,7 @@ process_inner_cksums(void *l3_hdr, const
> struct
> >>>> testpmd_offload_info *info,  ol_flags |=
> >> RTE_MBUF_F_TX_UDP_CKSUM;  }
> >>>> else {  if (info->is_tunnel) -l4_off = info->l2_len +
> >>>> +l4_off = info->outer_l2_len +
> >>>>info->outer_l3_len +
> >>>>info->l2_len + info->l3_len;
> This seems OK. A similar miss is present for TCP case also.
> Can you please do the same for that. Line 537
> >>>>   else
> >>>> @@ -536,7 +536,7 @@ process_inner_cksums(void *l3_hdr, const
> struct
> >>>> testpmd_offload_info *info,  ol_flags |=
> >> RTE_MBUF_F_TX_TCP_CKSUM;  }
> >>>> else {  if (info->is_tunnel) -l4_off = info->l2_len +
> >>>> info->outer_l3_len +
> >>>> +l4_off = info->outer_l2_len + info-
> >>>>> outer_l3_len +
> >>>>info->l2_len + info->l3_len;
> >>>>   else
> >>>>   l4_off = info->l2_len + info->l3_len; @@ -
> This change might not be required. As for normal packet (non-tunnel case)
> l4_off = info->l2_len + info->l3_len;  should be valid.
> Please re-check.
I don't understand what you mean. I fix the code under the tunnel case, and I 
didn't modify the code for the non-tunnel case.
> 
> 
> >>>> 625,7 +625,7 @@ process_outer_cksums(void *outer_l3_hdr, struct
> >>>> testpmd_offload_info *info,  if (udp_hdr->dgram_cksum != 0) {
> >>>> udp_hdr->dgram_cksum = 0;  udp_hdr->dgram_cksum =
> >>>> get_udptcp_checksum(m, outer_l3_hdr,
> >>>> -info->l2_len + info->outer_l3_len,
> >>>> +info->outer_l2_len + info-
> >>>>> outer_l3_len,
> >>>>   info->outer_ethertype);
> >>>>   }
> >>>>
> >>>> --
> >>>> 2.33.1