[dpdk-dev] [PATCH] net/ixgbe: fix flow ctrl mode setting

2019-12-18 Thread Sun GuinanX
When the port starts, the hw register is reset first,
and then the required parameters are set again.
If the parameters to be used are not set after resetting the register,
a read register error will occur. This patch is used to fix the problem.

Fixes: af75078fece3 ("first public release")
Cc: sta...@dpdk.org

Signed-off-by: Sun GuinanX 
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 20 
 drivers/net/ixgbe/ixgbe_ethdev.h |  1 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 2c6fd0f13..e602df02b 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2539,6 +2539,7 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 {
struct ixgbe_hw *hw =
IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+   struct ixgbe_adapter *adapter = dev->data->dev_private;
struct ixgbe_vf_info *vfinfo =
*IXGBE_DEV_PRIVATE_TO_P_VFDATA(dev->data->dev_private);
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
@@ -2555,6 +2556,7 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
IXGBE_DEV_PRIVATE_TO_TM_CONF(dev->data->dev_private);
struct ixgbe_macsec_setting *macsec_setting =
IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private);
+   uint32_t mflcn;
 
PMD_INIT_FUNC_TRACE();
 
@@ -2665,6 +2667,20 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
}
 
ixgbe_restore_statistics_mapping(dev);
+   err = ixgbe_fc_enable(hw);
+   if ((err == IXGBE_SUCCESS) || (err == IXGBE_ERR_FC_NOT_NEGOTIATED)) {
+
+   mflcn = IXGBE_READ_REG(hw, IXGBE_MFLCN);
+
+   /* set or clear MFLCN.PMCF bit depending on configuration */
+   if (adapter->mac_ctrl_frame_fwd != 0)
+   mflcn |= IXGBE_MFLCN_PMCF;
+   else
+   mflcn &= ~IXGBE_MFLCN_PMCF;
+
+   IXGBE_WRITE_REG(hw, IXGBE_MFLCN, mflcn);
+   IXGBE_WRITE_FLUSH(hw);
+   }
 
err = ixgbe_dev_rxtx_start(dev);
if (err < 0) {
@@ -2893,6 +2909,8 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
 
adapter->rss_reta_updated = 0;
 
+   adapter->mac_ctrl_frame_fwd = 0;
+
hw->adapter_stopped = true;
 }
 
@@ -4646,6 +4664,7 @@ static int
 ixgbe_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 {
struct ixgbe_hw *hw;
+   struct ixgbe_adapter *adapter = dev->data->dev_private;
int err;
uint32_t rx_buf_size;
uint32_t max_high_water;
@@ -4682,6 +4701,7 @@ ixgbe_flow_ctrl_set(struct rte_eth_dev *dev, struct 
rte_eth_fc_conf *fc_conf)
hw->fc.low_water[0]   = fc_conf->low_water;
hw->fc.send_xon   = fc_conf->send_xon;
hw->fc.disable_fc_autoneg = !fc_conf->autoneg;
+   adapter->mac_ctrl_frame_fwd = fc_conf->mac_ctrl_frame_fwd;
 
err = ixgbe_fc_enable(hw);
 
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 76a1b9d18..5af584f9e 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -510,6 +510,7 @@ struct ixgbe_adapter {
 * mailbox status) link status.
 */
uint8_t pflink_fullchk;
+   uint8_t mac_ctrl_frame_fwd;
 };
 
 struct ixgbe_vf_representor {
-- 
2.17.1



Re: [dpdk-dev] [PATCH] net/ixgbe: add or remove MAC address

2019-12-23 Thread Sun, GuinanX
Hi Xiaolong

> -Original Message-
> From: Ye, Xiaolong
> Sent: Monday, December 23, 2019 3:25 PM
> To: Sun, GuinanX 
> Cc: dev@dpdk.org; Lu, Wenzhuo ; Yang, Qiming
> ; Xing, Beilei 
> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: add or remove MAC address
> 
> Hi, guinan
> 
> For the title, better to use
> 
> Add support for vf MAC address add and remove
> 
> or something like so.

I agree with you and I will fix it in V2's patch

> 
> On 12/03, Guinan Sun wrote:
> >Ixgbe PMD pf host code needs to support ixgbevf mac address add and
> >remove. For this purpose, a response was added between pf and vf to
> >update the mac address.
> 
> Does this mean each one vf can have multiple MAC addresses after this patch, 
> or
> this `add` actually means to replace the old mac address with the new one?

It means each one vf can have multiple MAC addresses after this patch

> 
> >
> >Signed-off-by: Guinan Sun 
> >---
> > drivers/net/ixgbe/ixgbe_ethdev.h |  1 +
> > drivers/net/ixgbe/ixgbe_pf.c | 35 
> > 2 files changed, 36 insertions(+)
> >
> >diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h
> >b/drivers/net/ixgbe/ixgbe_ethdev.h
> >index 76a1b9d18..e1cd8fd16 100644
> >--- a/drivers/net/ixgbe/ixgbe_ethdev.h
> >+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> >@@ -270,6 +270,7 @@ struct ixgbe_vf_info {
> > uint8_t api_version;
> > uint16_t switch_domain_id;
> > uint16_t xcast_mode;
> >+uint16_t mac_count;
> 
> How is this mac_count initialized?

This variable is initialized to 0 when the ixgbe_adapter structure is 
initialized, and the method is similar to vlan_count.

> 
> > };
> >
> > /*
> >diff --git a/drivers/net/ixgbe/ixgbe_pf.c
> >b/drivers/net/ixgbe/ixgbe_pf.c index d0d85e138..76dbed2ab 100644
> >--- a/drivers/net/ixgbe/ixgbe_pf.c
> >+++ b/drivers/net/ixgbe/ixgbe_pf.c
> >@@ -748,6 +748,37 @@ ixgbe_set_vf_mc_promisc(struct rte_eth_dev *dev,
> uint32_t vf, uint32_t *msgbuf)
> > return 0;
> > }
> >
> >+static int
> >+ixgbe_set_vf_macvlan_msg(struct rte_eth_dev *dev, uint32_t vf,
> >+uint32_t *msgbuf) {
> >+struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> >+struct ixgbe_vf_info *vf_info =
> >+*(IXGBE_DEV_PRIVATE_TO_P_VFDATA(dev->data-
> >dev_private));
> >+uint8_t *new_mac = (uint8_t *)(&msgbuf[1]);
> >+int index = (msgbuf[0] & IXGBE_VT_MSGINFO_MASK) >>
> >+IXGBE_VT_MSGINFO_SHIFT;
> >+
> >+if (index) {
> >+if (!rte_is_valid_assigned_ether_addr(
> >+(struct rte_ether_addr *)new_mac)) {
> >+RTE_LOG(ERR, PMD, "set invalid mac vf:%d\n", vf);
> >+return -1;
> >+}
> >+
> >+if (new_mac == NULL)
> >+return -1;
> 
> I feel the null check should be in front of valid ether addr check, otherwise 
> there
> might be null pointer reference issue.

Ok,I will fix it in V2's patch.

> 
> Thanks,
> Xiaolong
> 
> >+
> >+vf_info[vf].mac_count++;
> >+
> >+hw->mac.ops.set_rar(hw, vf_info[vf].mac_count,
> >+new_mac, vf, IXGBE_RAH_AV);
> >+} else {
> >+hw->mac.ops.clear_rar(hw, vf_info[vf].mac_count);
> >+vf_info[vf].mac_count = 0;
> >+}
> >+return 0;
> >+}
> >+
> > static int
> > ixgbe_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t vf)  { @@
> >-835,6 +866,10 @@ ixgbe_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t
> vf)
> > if (retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED)
> > retval = ixgbe_set_vf_mc_promisc(dev, vf, msgbuf);
> > break;
> >+case IXGBE_VF_SET_MACVLAN:
> >+if (retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED)
> >+retval = ixgbe_set_vf_macvlan_msg(dev, vf, msgbuf);
> >+break;
> > default:
> > PMD_DRV_LOG(DEBUG, "Unhandled Msg %8.8x",
> (unsigned)msgbuf[0]);
> > retval = IXGBE_ERR_MBX;
> >--
> >2.17.1
> >


Re: [dpdk-dev] [PATCH 04/21] net/ixgbe/base: x550em 10G NIC driver issue

2020-06-30 Thread Sun, GuinanX
Hi Ferruh

> -Original Message-
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 7:59 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Skajewski, PiotrX 
> Subject: Re: [dpdk-dev] [PATCH 04/21] net/ixgbe/base: x550em 10G NIC driver
> issue
> 
> On 6/12/2020 4:23 AM, Guinan Sun wrote:
> > With the NVM image for x550em XFI ethtool will not report the
> > auto-negotiation feature correctly. The auto-negotiation should be
> > "No" for supports and advertised items.
> 
> This is not 'ethtool' issue in this context, right? It also affects the 
> reported
> value for the DPDK?

It is not only 'ethtool' issue but also affects the reported value for the DPDK.

> 
> And "driver issue" in the patch title gives only a little value, what do you 
> think
> something like:
> "net/ixgbe/base: fix x550em 10G NIC auto-negotiation report"
> 

I agree with your opinion, patch v2 will fix it.

> Also does it only affects the auto-negotiation, I can see speed is updated 
> too?

The speed update has an impact on the report, and patch v2 will modify the 
commit information to show the impact on speed.

> 
> >
> > Signed-off-by: Piotr Skajewski 
> > Signed-off-by: Guinan Sun 
> > ---
> >  drivers/net/ixgbe/base/ixgbe_x550.c | 8 +++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > index 3de406fd3..9fa999e01 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > @@ -1891,7 +1891,14 @@ s32 ixgbe_get_link_capabilities_X550em(struct
> ixgbe_hw *hw,
> > else
> > *speed = IXGBE_LINK_SPEED_10GB_FULL;
> > } else {
> > +   *autoneg = true;
> > +
> > switch (hw->phy.type) {
> > +   case ixgbe_phy_x550em_xfi:
> > +   *speed = IXGBE_LINK_SPEED_1GB_FULL |
> > +IXGBE_LINK_SPEED_10GB_FULL;
> > +   *autoneg = false;
> > +   break;
> > case ixgbe_phy_ext_1g_t:
> >  #ifdef PREBOOT_SUPPORT
> > *speed = IXGBE_LINK_SPEED_1GB_FULL; @@ -1925,7
> +1932,6 @@ s32
> > ixgbe_get_link_capabilities_X550em(struct ixgbe_hw *hw,
> >  IXGBE_LINK_SPEED_1GB_FULL;
> > break;
> > }
> > -   *autoneg = true;
> > }
> >
> > return IXGBE_SUCCESS;
> >



Re: [dpdk-dev] [PATCH 13/21] net/ixgbe/base: modify Klocwork hits for DDK 7.0

2020-06-30 Thread Sun, GuinanX
Hi Ferruh

> -Original Message-
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 8:00 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Chylkowski, JakubX 
> Subject: Re: [dpdk-dev] [PATCH 13/21] net/ixgbe/base: modify Klocwork hits
> for DDK 7.0
> 
> On 6/12/2020 4:24 AM, Guinan Sun wrote:
> > Fix issues found by Klocwork for DDK 7.0
> 
> This doesn't say much on its own, can you please document the actual issues
> fixed, like "fix unchecked return value" and I think can drop the Klocwork
> reference, and even not asking what DDK is ...
> 

We classified the inspection results and divided them into the following two 
types.
The first one is the return value check.
The second is type conversion.

In addition, regarding this part of the content, we think it is to improve the 
code style.
If it is a fix, the modified content is more scattered, and it is not easy to 
collect the fix version.
So, the title is tentatively 'modify coding style'.

> Feel free to split patch into multiple patches if there are multiple types of 
> issues
> have been fixed.

First of all, I agree with you, but for the improvement of coding style, it is 
not necessary to split into multiple patches, 
because this has nothing to do with the function, it is just an improvement 
point.
If the function is different, it will be split into different patches.

> 
> >
> > Signed-off-by: Jakub Chylkowski 
> > Signed-off-by: Guinan Sun 
> 
> <...>



Re: [dpdk-dev] [PATCH 14/21] net/ixgbe/base: add defines for min rollback revision fields

2020-06-30 Thread Sun, GuinanX
Hi Ferruh

> -Original Message-
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 8:00 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Naczyk, Jacek 
> Subject: Re: [dpdk-dev] [PATCH 14/21] net/ixgbe/base: add defines for min
> rollback revision fields
> 
> On 6/12/2020 4:24 AM, Guinan Sun wrote:
> > Add defines for Minimum Rollback Revision fields as defined in SGVL.
> 
> SGVL?

Sorry, I don't know this, I will investigate later.
But this patch was removed in the latest drop package.
So patch V2 will remove this code.

> 
> >
> > Signed-off-by: Jacek Naczyk 
> > Signed-off-by: Guinan Sun 
> 
> <...>


Re: [dpdk-dev] [PATCH 17/21] net/ixgbe/base: improve log about autonego being disabled

2020-06-30 Thread Sun, GuinanX
Hi  Ferruh

> -Original Message-
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 8:01 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Chylkowski, JakubX 
> Subject: Re: [dpdk-dev] [PATCH 17/21] net/ixgbe/base: improve log about
> autonego being disabled
> 
> On 6/12/2020 4:24 AM, Guinan Sun wrote:
> > On ESXi OS, when user disables auto negotiation, the following log
> > appears: "(unsupported) Flow control autoneg is disabled".
> 
> Is ESXi reference valid for the DPDK commit?

I think this is no problem, because there are a lot of commit information 
including'ESXi' in dpdk .
For example the following version:
{
commit dfedf3e3f9d281fbe493cc98fd9ef82bec42fdec
Author: Viacheslav Ovsiienko 
Date: Tue Jul 30 09:20:24 2019 +

 net/mlx5: add workaround for VLAN in virtual machine
}

{
commit 18c8e84d7726ad16bb81fdb7649e13202997cd41
Author: Gautam Dawar 
Date: Mon Jun 10 08:38:39 2019 +0100

 net/sfc/base: support proxy auth operations for SR-IOV
}...

So there should be no correction.

> 
> > It is true that AN is disabled but it is not necessarily true that it
> > is not supported.
> >
> > Signed-off-by: Jakub Chylkowski 
> > Signed-off-by: Guinan Sun 
> 
> <...>


Re: [dpdk-dev] [PATCH 18/21] net/ixgbe/base: ipv6 Mask for purpose FDIR VLAN Port Feature

2020-06-30 Thread Sun, GuinanX
Hi Ferruh

> -Original Message-
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 8:01 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Skajewski, PiotrX 
> Subject: Re: [dpdk-dev] [PATCH 18/21] net/ixgbe/base: ipv6 Mask for purpose
> FDIR VLAN Port Feature
> 
> On 6/12/2020 4:24 AM, Guinan Sun wrote:
> > Allow Flow Director Filter to set IPv6 rules without setting IPv6
> > source/destination address.
> 
> Patch title is hard to understand, commit log is more clear, but title talks 
> about
> the VLAN, but description looks different. Can you please clarify?
> 

Patch v2 will modify the patch title.

> >
> > Signed-off-by: Piotr Skajewski 
> > Signed-off-by: Guinan Sun 
> > ---
> >  drivers/net/ixgbe/base/ixgbe_82599.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/ixgbe/base/ixgbe_82599.c
> > b/drivers/net/ixgbe/base/ixgbe_82599.c
> > index e425f28af..69fd4cd3f 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_82599.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_82599.c
> > @@ -1832,6 +1832,7 @@ s32 ixgbe_fdir_set_input_mask_82599(struct
> ixgbe_hw *hw,
> >  ~input_mask->formatted.src_ip[0]);
> > IXGBE_WRITE_REG_BE32(hw, IXGBE_FDIRDIP4M,
> >  ~input_mask->formatted.dst_ip[0]);
> > +   IXGBE_WRITE_REG_BE32(hw, IXGBE_FDIRIP6M, 0x);
> > }
> > return IXGBE_SUCCESS;
> >  }
> >



Re: [dpdk-dev] [PATCH 19/21] net/ixgbe/base: remove default advertising for 2.5G and 5G

2020-06-30 Thread Sun, GuinanX
Hi Ferruh

> -Original Message-
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 8:01 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Fujinaka, Todd 
> Subject: Re: [dpdk-dev] [PATCH 19/21] net/ixgbe/base: remove default
> advertising for 2.5G and 5G
> 
> On 6/12/2020 4:24 AM, Guinan Sun wrote:
> > We are seeing interoperability issues with switches when 2.5G and 5G
> > are advertised by default, so default to off.
> 
> This is only for 'X550' device, right? If so can you please clarify this in 
> the patch
> title and the commit log?
> 
> >
> > LINUX ONLY:
> >
> > We will need to add a note to our README on how to enable 2.5G and 5G.
> > The netwmask is a combination of:
> >
> > 0x020   1000baseT Full
> > 0x8000  2500baseT Full
> > 0x1 5000baseT Full
> > 0x1000  1baseT Full
> >
> > Combine the two into the advertisement flags:
> > ethtool -s advertise ethX 
> 
> I can see this is Linux only so we can drop from commit log, but do we need to
> document something similar for DPDK?

The content related to linux only will be dropped from the commit log.
In addition, regarding the doc, this part is not the content of the limitation, 
just off the default value, 
it is no need to document something in DPDK.

> 
> >
> > Signed-off-by: Todd Fujinaka 
> > Signed-off-by: Guinan Sun 
> > ---
> >  drivers/net/ixgbe/base/ixgbe_phy.c | 4 
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c
> > b/drivers/net/ixgbe/base/ixgbe_phy.c
> > index f859b152e..8d4d9bbfe 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> > @@ -915,10 +915,6 @@ static s32
> ixgbe_get_copper_speeds_supported(struct ixgbe_hw *hw)
> > hw->phy.speeds_supported |= IXGBE_LINK_SPEED_100_FULL;
> >
> > switch (hw->mac.type) {
> > -   case ixgbe_mac_X550:
> > -   hw->phy.speeds_supported |=
> IXGBE_LINK_SPEED_2_5GB_FULL;
> > -   hw->phy.speeds_supported |= IXGBE_LINK_SPEED_5GB_FULL;
> > -   break;
> > case ixgbe_mac_X550EM_x:
> > case ixgbe_mac_X550EM_a:
> > hw->phy.speeds_supported &=
> ~IXGBE_LINK_SPEED_100_FULL;
> >



Re: [dpdk-dev] [PATCH 21/21] net/ixgbe/base: update version

2020-06-30 Thread Sun, GuinanX
Hi Ferruh

> -Original Message-
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 8:01 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 21/21] net/ixgbe/base: update version
> 
> On 6/12/2020 4:24 AM, Guinan Sun wrote:
> > Update base code version in readme.
> >
> > Signed-off-by: Guinan Sun 
> > ---
> >  drivers/net/ixgbe/base/README | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ixgbe/base/README
> > b/drivers/net/ixgbe/base/README index a48b14ed2..c8d6b39ac 100644
> > --- a/drivers/net/ixgbe/base/README
> > +++ b/drivers/net/ixgbe/base/README
> > @@ -6,7 +6,7 @@ Intel® IXGBE driver
> >  ===
> >
> >  This directory contains source code of FreeBSD ixgbe driver of
> > version -cid-ixgbe.2018.08.28.tar.gz released by the team which
> > develop
> > +not-released-cid-ixgbe.2020.06.05.tar.gz released by the team which
> > +develop
> 
> What does 'not-released' mean?

not-released means they are drops of shared code to the DPDK team.

> Since this is to clarify the version of the base code, why not just 'cid-
> ixgbe.2020.06.05.tar.gz'?

The drop package is named like this, not our modification.

> 
> >  basic drivers for any ixgbe NIC. The sub-directory of base/  contains
> > the original source package.
> >  This driver is valid for the product(s) listed below
> >



Re: [dpdk-dev] [PATCH v2] net/i40e: enable port filter by switch filter

2020-07-06 Thread Sun, GuinanX
Hi beilei

> -Original Message-
> From: Xing, Beilei
> Sent: Tuesday, June 30, 2020 2:16 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Guo, Jia 
> Subject: RE: [PATCH v2] net/i40e: enable port filter by switch filter
> 
> 
> 
> > -Original Message-
> > From: Sun, GuinanX 
> > Sent: Tuesday, June 30, 2020 12:42 PM
> > To: dev@dpdk.org
> > Cc: Xing, Beilei ; Guo, Jia
> > ; Sun, GuinanX 
> > Subject: [PATCH v2] net/i40e: enable port filter by switch filter
> 
> How about 'support cloud filter with l4 port'?

I agree with you , patch v3 will fix it.

> 
> Please also fix all the warnings in patchwork:
> http://mails.dpdk.org/archives/test-report/2020-June/139555.html.

Issues that affect readability have not been modified, others have been 
modified.

> 
> >
> > This patch enables the filter that supports to create following two
> > rules for the same packet type:
> 
> Better to clarify which packet types will be supported.

Patch V3 will fix it.

> 
> > One is to select source port only as input set and the other is for
> > destination port only.
> >
> > Signed-off-by: Guinan Sun 
> > ---
> > v2:
> > * Fixed code style and variable naming
> > ---
> >  doc/guides/rel_notes/release_20_08.rst |   8 +
> >  drivers/net/i40e/i40e_ethdev.c | 195 -
> >  drivers/net/i40e/i40e_ethdev.h |  17 ++
> >  drivers/net/i40e/i40e_flow.c   | 223 +
> >  4 files changed, 442 insertions(+), 1 deletion(-)
> >
> > diff --git a/doc/guides/rel_notes/release_20_08.rst
> > b/doc/guides/rel_notes/release_20_08.rst
> > index 3c40424cc..c4d094eac 100644
> > --- a/doc/guides/rel_notes/release_20_08.rst
> > +++ b/doc/guides/rel_notes/release_20_08.rst
> > @@ -87,6 +87,14 @@ New Features
> >
> >* Added support for DCF datapath configuration.
> >
> > +* **Updated Intel i40e driver.**
> > +
> > +  Updated i40e PMD with new features and improvements, including:
> > +
> > +  * Added a new type of cloud filter to support the coexistence of the
> > +following two rules. One selects L4 destination as input set and
> 
> L4 destination port

Patch V3 will fix it.

> 
> > +the other one selects L4 source port.
> > +
> 
> BTW, replace filter is used for the patch, so which protocol is impacted with
> this change?
> e.g. If using this feature, can cloud filter for vxlan work well?
> If there's any impact, please add the limitation in the doc.

Limitation has been added in the doc.

> 
> >  Removed Items
> >  -
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 970a31cb2..cea7f6b59 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -7956,6 +7956,13 @@ i40e_dev_tunnel_filter_set(struct i40e_pf *pf,
> > #define I40E_TR_GRE_KEY_MASK0x400  #define
> > I40E_TR_GRE_KEY_WITH_XSUM_MASK0x800
> >  #define I40E_TR_GRE_NO_KEY_MASK0x8000
> > +#define I40E_AQC_REPLACE_CLOUD_CMD_INPUT_PORT_TR_WORD0 0x49
> > #define
> > +I40E_AQC_REPLACE_CLOUD_CMD_INPUT_DIRECTION_WORD0 0x41
> > #define
> > +I40E_AQC_REPLACE_CLOUD_CMD_INPUT_INGRESS_WORD0 0x80 #define
> > +I40E_DIRECTION_INGRESS_KEY0x8000 #define I40E_TR_L4_TYPE_TCP0x2
> > +#define I40E_TR_L4_TYPE_UDP0x4 #define I40E_TR_L4_TYPE_SCTP0x8
> >
> >  static enum
> >  i40e_status_code i40e_replace_mpls_l1_filter(struct i40e_pf *pf) @@ -
> > 8254,6 +8261,131 @@ i40e_status_code
> > i40e_replace_gtp_cloud_filter(struct
> > i40e_pf *pf)
> >  return status;
> >  }
> >
> > +static enum i40e_status_code
> > +i40e_replace_port_l1_filter(struct i40e_pf *pf, enum
> > +i40e_l4_port_type
> > +l4_port_type) {
> > +struct i40e_aqc_replace_cloud_filters_cmd_buf  filter_replace_buf;
> > +struct i40e_aqc_replace_cloud_filters_cmd  filter_replace; enum
> > +i40e_status_code status = I40E_SUCCESS; struct i40e_hw *hw =
> > +I40E_PF_TO_HW(pf); struct rte_eth_dev *dev = ((struct i40e_adapter
> > +*)hw->back)-
> > >eth_dev;
> > +
> > +if (pf->support_multi_driver) {
> > +PMD_DRV_LOG(ERR, "Replace l1 filter is not supported."); return
> > +I40E_NOT_SUPPORTED; }
> > +
> > +memset(&filter_replace, 0,
> > +   sizeof(struct i40e_aqc_replace_cloud_filters_cmd));
> > +memset(&filter_replace_buf, 0,
> > +   sizeof(struct i40e_aqc_replace_cloud_filters_cmd_buf));
> > +
> > +/* create L1 filter */
> > +if 

Re: [dpdk-dev] [PATCH v2 12/20] net/ixgbe/base: modify coding style

2020-07-09 Thread Sun, GuinanX
Hi Ferruh

> -Original Message-
> From: Yigit, Ferruh
> Sent: Wednesday, July 8, 2020 11:26 PM
> To: Sun, GuinanX ; dev@dpdk.org; Zhang, Qi Z
> 
> Cc: Guo, Jia ; Zhao1, Wei ;
> Chylkowski, JakubX 
> Subject: Re: [dpdk-dev] [PATCH v2 12/20] net/ixgbe/base: modify coding style
> 
> On 7/2/2020 4:13 AM, Guinan Sun wrote:
> > Fix unchecked return value.
> > Add cast for type mismatch.
> >
> > Signed-off-by: Jakub Chylkowski 
> > Signed-off-by: Guinan Sun 
> > ---
> >  drivers/net/ixgbe/base/ixgbe_82599.c | 12 +---
> >  drivers/net/ixgbe/base/ixgbe_common.c|  6 ++
> >  drivers/net/ixgbe/base/ixgbe_common.h|  2 +-
> >  drivers/net/ixgbe/base/ixgbe_dcb_82598.c |  2 +-
> > drivers/net/ixgbe/base/ixgbe_dcb_82599.c |  2 +-
> >  drivers/net/ixgbe/base/ixgbe_phy.c   | 25 +++-
> >  drivers/net/ixgbe/base/ixgbe_x540.c  |  2 +-
> >  drivers/net/ixgbe/base/ixgbe_x550.c  |  2 +-
> >  8 files changed, 19 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/base/ixgbe_82599.c
> > b/drivers/net/ixgbe/base/ixgbe_82599.c
> > index 193233746..e425f28af 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_82599.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_82599.c
> > @@ -1547,7 +1547,7 @@ void ixgbe_fdir_add_signature_filter_82599(struct
> ixgbe_hw *hw,
> >  * is for FDIRCMD.  Then do a 64-bit register write from FDIRHASH.
> >  */
> > fdirhashcmd = (u64)fdircmd << 32;
> > -   fdirhashcmd |= ixgbe_atr_compute_sig_hash_82599(input, common);
> > +   fdirhashcmd |= (u64)ixgbe_atr_compute_sig_hash_82599(input,
> common);
> 
> Hi Guinan, Qi,
> 
> These are not coding style changes, as commit log says they are fixes in the
> code.
> 
> Can you please properly separate the patch based on the fix type and provide
> relevant patch title?

First, we will make changes to the commit log.
The reason is that this is not a fix, but some unnecessary return value check 
is deleted.
Secondly, we split this patch into two. This will make the expression clearer.


Re: [dpdk-dev] [PATCH 0/2] fix defects of macro in VF

2020-05-07 Thread Sun, GuinanX
Hi ZhaoWei

> -Original Message-
> From: Zhao1, Wei
> Sent: Friday, May 8, 2020 10:19 AM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Sun, GuinanX ; Guo, Jia 
> Subject: RE: [dpdk-dev] [PATCH 0/2] fix defects of macro in VF
> 
> Hi,
> Please alaign " \" in each line.

It will be modified in V2 patch.
Remark: It can't be aligned in vim tool and email at the same time.
It will be aligned in vim tool in v2 patch.

> 
> 
> > -Original Message-
> > From: dev  On Behalf Of Guinan Sun
> > Sent: Friday, May 8, 2020 9:59 AM
> > To: dev@dpdk.org
> > Cc: Sun, GuinanX 
> > Subject: [dpdk-dev] [PATCH 0/2] fix defects of macro in VF
> >
> > The defects of UPDATE_VF_STAT and UPDATE_VF_STAT_36BIT exist.
> > If latest is less than last, we will get wrong result.
> > The issues exist only in ixgbe and e1000 NICs.
> >
> > Guinan Sun (2):
> >   net/ixgbe: fix defects of macro in VF
> >   net/e1000: fix defects of macro in VF
> >
> >  drivers/net/e1000/igb_ethdev.c   | 14 +++---
> >  drivers/net/ixgbe/ixgbe_ethdev.c | 20 ++--
> >  2 files changed, 29 insertions(+), 5 deletions(-)
> >
> > --
> > 2.17.1
> 



Re: [dpdk-dev] [PATCH v3] net/ixgbe: add support for VF MAC address add and remove

2020-05-08 Thread Sun, GuinanX
Hi zhaowei

> -Original Message-
> From: Zhao1, Wei
> Sent: Friday, May 8, 2020 2:58 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Lu, Wenzhuo ; Yang, Qiming
> ; Sun, GuinanX ; Guo, Jia
> 
> Subject: RE: [dpdk-dev] [PATCH v3] net/ixgbe: add support for VF MAC address
> add and remove
> 
> Hi, Guinan
> 
> There is a bug for this patch:
> The second input parameter of function hw->mac.ops.set_rar() should be index
> of MAC address register IXGBE_RAL/H.
> This index should be management by pf host for all the vf, not only index for 
> one
> vf, or vf1 maybe overwrite this MAC address of vf0.
> Although this patch has been merged, please commit fix patch for it.
> 

I will double check for this function.
Thank you very much.

> 
> > -Original Message-
> > From: dev  On Behalf Of Guinan Sun
> > Sent: Tuesday, December 24, 2019 11:24 AM
> > To: dev@dpdk.org
> > Cc: Lu, Wenzhuo ; Yang, Qiming
> > ; Sun, GuinanX 
> > Subject: [dpdk-dev] [PATCH v3] net/ixgbe: add support for VF MAC
> > address add and remove
> >
> > Ixgbe PMD pf host code needs to support ixgbevf mac address add and remove.
> > For this purpose, a response was added between pf and vf to update the
> > mac address.
> >
> > Signed-off-by: Guinan Sun 
> > ---
> > v3:
> > * Changed from `RTE_LOG` to `PMD_DRV_LOG`.
> > v2:
> > * Changed the title of commit message.
> > * Checked null in front of valid ether addr check.
> > ---
> >  drivers/net/ixgbe/ixgbe_ethdev.h |  1 +
> >  drivers/net/ixgbe/ixgbe_pf.c | 35
> > 
> >  2 files changed, 36 insertions(+)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h
> > b/drivers/net/ixgbe/ixgbe_ethdev.h
> > index 76a1b9d18..e1cd8fd16 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> > @@ -270,6 +270,7 @@ struct ixgbe_vf_info {  uint8_t api_version;
> > uint16_t switch_domain_id;  uint16_t xcast_mode;
> > +uint16_t mac_count;
> >  };
> >
> >  /*
> > diff --git a/drivers/net/ixgbe/ixgbe_pf.c
> > b/drivers/net/ixgbe/ixgbe_pf.c index
> > d0d85e138..c93c0fdc2 100644
> > --- a/drivers/net/ixgbe/ixgbe_pf.c
> > +++ b/drivers/net/ixgbe/ixgbe_pf.c
> > @@ -748,6 +748,37 @@ ixgbe_set_vf_mc_promisc(struct rte_eth_dev *dev,
> > uint32_t vf, uint32_t *msgbuf)  return 0;  }
> >
> > +static int
> > +ixgbe_set_vf_macvlan_msg(struct rte_eth_dev *dev, uint32_t vf,
> > +uint32_t
> > +*msgbuf) {
> > +struct ixgbe_hw *hw =
> > IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> > +struct ixgbe_vf_info *vf_info =
> > +*(IXGBE_DEV_PRIVATE_TO_P_VFDATA(dev->data->dev_private));
> > +uint8_t *new_mac = (uint8_t *)(&msgbuf[1]); int index = (msgbuf[0] &
> > +IXGBE_VT_MSGINFO_MASK) >>
> > +IXGBE_VT_MSGINFO_SHIFT;
> > +
> > +if (index) {
> > +if (new_mac == NULL)
> > +return -1;
> > +
> > +if (!rte_is_valid_assigned_ether_addr(
> > +(struct rte_ether_addr *)new_mac)) {
> > +PMD_DRV_LOG(ERR, "set invalid mac vf:%d\n", vf); return -1; }
> > +
> > +vf_info[vf].mac_count++;
> > +
> > +hw->mac.ops.set_rar(hw, vf_info[vf].mac_count,
> > +new_mac, vf, IXGBE_RAH_AV);
> > +} else {
> > +hw->mac.ops.clear_rar(hw, vf_info[vf].mac_count);
> > +vf_info[vf].mac_count = 0;
> > +}
> > +return 0;
> > +}
> > +
> >  static int
> >  ixgbe_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t vf)  { @@
> > -835,6
> > +866,10 @@ ixgbe_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t vf)
> >  if (retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED)  retval =
> > ixgbe_set_vf_mc_promisc(dev, vf, msgbuf);  break;
> > +case IXGBE_VF_SET_MACVLAN:
> > +if (retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED) retval =
> > +ixgbe_set_vf_macvlan_msg(dev, vf, msgbuf); break;
> >  default:
> >  PMD_DRV_LOG(DEBUG, "Unhandled Msg %8.8x", (unsigned)msgbuf[0]);
> > retval = IXGBE_ERR_MBX;
> > --
> > 2.17.1
> 



Re: [dpdk-dev] [PATCH v4] net/ixgbe: fix flow ctrl mode setting

2020-05-09 Thread Sun, GuinanX
Hi ,zhaowei

> -Original Message-
> From: Zhao1, Wei
> Sent: Saturday, May 9, 2020 3:35 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Lu, Wenzhuo ; Yang, Qiming
> ; Zhang, Qi Z ; Sun, GuinanX
> ; sta...@dpdk.org; Guo, Jia 
> Subject: RE: [dpdk-dev] [PATCH v4] net/ixgbe: fix flow ctrl mode setting
> 
> Hi, guinan
> 
>  In this patch, you have add a new parameter of mac_ctrl_frame_fwd, it should
> not be clear in ixgbe_dev_stop(), Or it will be over write when do port 
> reset, and
> also you should add mac_ctrl_frame_fwd in ixgbe_flow_ctrl_get() for FC info
> get.
> Although this patch has been merged, please commit fix patch for it, thanks!
> 

For this question, I need to confirm the requirements with Konieczny TomaszX 
before I can decide whether to make changes.

> 
> 
> > -Original Message-
> > From: dev  On Behalf Of Guinan Sun
> > Sent: Tuesday, February 18, 2020 11:40 AM
> > To: dev@dpdk.org
> > Cc: Lu, Wenzhuo ; Yang, Qiming
> > ; Zhang, Qi Z ; Sun,
> > GuinanX ; sta...@dpdk.org
> > Subject: [dpdk-dev] [PATCH v4] net/ixgbe: fix flow ctrl mode setting
> >
> > When the port starts, the hw register is reset first, and then the
> > required parameters are set again.
> > If the parameters to be used are not set after resetting the register,
> > a read register error will occur. This patch is used to fix the problem.
> >
> > Fixes: af75078fece3 ("first public release")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Guinan Sun 
> > ---
> > v4: changes
> > * rebase to dpdk-next-net-intel
> >
> > v3: changes
> > * wrap duplication code into a function
> > * Modify checkpatch warnings
> >
> > v2: changes
> > * Modify the initial value of requested_mode and current_mode
> > ---
> >  drivers/net/ixgbe/ixgbe_ethdev.c | 76
> > +--- drivers/net/ixgbe/ixgbe_ethdev.h |  1
> > +
> >  2 files changed, 51 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index 3aab24e82..08b4cc689 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +}
> > +
> >  err = ixgbe_dev_rxtx_start(dev);
> >  if (err < 0) {
> >  PMD_INIT_LOG(ERR, "Unable to start rxtx queues"); @@ -2900,6
> > +2939,8 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
> >
> >  adapter->rss_reta_updated = 0;
> >
> > +adapter->mac_ctrl_frame_fwd = 0;
> > +
> 
> Delete it please.
> 
> 
> 
> 
> 



Re: [dpdk-dev] [PATCH v4] net/ixgbe: fix flow ctrl mode setting

2020-05-09 Thread Sun, GuinanX
Hi, zhaowei

> -Original Message-
> From: Zhao1, Wei
> Sent: Saturday, May 9, 2020 5:30 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Lu, Wenzhuo ; Yang, Qiming
> ; Zhang, Qi Z ;
> sta...@dpdk.org; Guo, Jia 
> Subject: RE: [dpdk-dev] [PATCH v4] net/ixgbe: fix flow ctrl mode setting
> 
> Hi,guinan
> 
> More info for this problem:
> If some user set mac_ctrl_frame_fwd to 1 from fc ops, then he do a port reset
> process of "
> 1. testpmd> start
> 2. testpmd> set flow_ctrl mac_ctrl_frame_fwd on 0 3. testpmd> stop 4. testpmd>
> port stop 0 5. testpmd> port start 0 6. testpmd> start "
> Then after this process, the mac_ctrl_frame_fwd has been change to "off", so
> we should delete " adapter->mac_ctrl_frame_fwd = 0;" from dev-stop.
> 
Thank you very much , I get it.

> Another problem is "add mac_ctrl_frame_fwd in ixgbe_flow_ctrl_get() for FC
> info get", it is Independent on the above one.
> 
> 
> > -Original Message-
> > From: Sun, GuinanX 
> > Sent: Saturday, May 9, 2020 4:55 PM
> > To: Zhao1, Wei ; dev@dpdk.org
> > Cc: Lu, Wenzhuo ; Yang, Qiming
> > ; Zhang, Qi Z ;
> > sta...@dpdk.org; Guo, Jia 
> > Subject: RE: [dpdk-dev] [PATCH v4] net/ixgbe: fix flow ctrl mode
> > setting
> >
> > Hi ,zhaowei
> >
> > > -Original Message-
> > > From: Zhao1, Wei
> > > Sent: Saturday, May 9, 2020 3:35 PM
> > > To: Sun, GuinanX ; dev@dpdk.org
> > > Cc: Lu, Wenzhuo ; Yang, Qiming
> > > ; Zhang, Qi Z ; Sun,
> > > GuinanX ; sta...@dpdk.org; Guo, Jia
> > > 
> > > Subject: RE: [dpdk-dev] [PATCH v4] net/ixgbe: fix flow ctrl mode
> > > setting
> > >
> > > Hi, guinan
> > >
> > >  In this patch, you have add a new parameter of mac_ctrl_frame_fwd,
> > > it should not be clear in ixgbe_dev_stop(), Or it will be over write
> > > when do port reset, and also you should add mac_ctrl_frame_fwd in
> > > ixgbe_flow_ctrl_get() for FC info get.
> > > Although this patch has been merged, please commit fix patch for it, 
> > > thanks!
> > >
> >
> > For this question, I need to confirm the requirements with Konieczny
> > TomaszX before I can decide whether to make changes.
> >
> > >
> > >
> > > > -Original Message-
> > > > From: dev  On Behalf Of Guinan Sun
> > > > Sent: Tuesday, February 18, 2020 11:40 AM
> > > > To: dev@dpdk.org
> > > > Cc: Lu, Wenzhuo ; Yang, Qiming
> > > > ; Zhang, Qi Z ; Sun,
> > > > GuinanX ; sta...@dpdk.org
> > > > Subject: [dpdk-dev] [PATCH v4] net/ixgbe: fix flow ctrl mode
> > > > setting
> > > >
> > > > When the port starts, the hw register is reset first, and then the
> > > > required parameters are set again.
> > > > If the parameters to be used are not set after resetting the
> > > > register, a read register error will occur. This patch is used to
> > > > fix the
> > problem.
> > > >
> > > > Fixes: af75078fece3 ("first public release")
> > > > Cc: sta...@dpdk.org
> > > >
> > > > Signed-off-by: Guinan Sun 
> > > > ---
> > > > v4: changes
> > > > * rebase to dpdk-next-net-intel
> > > >
> > > > v3: changes
> > > > * wrap duplication code into a function
> > > > * Modify checkpatch warnings
> > > >
> > > > v2: changes
> > > > * Modify the initial value of requested_mode and current_mode
> > > > ---
> > > >  drivers/net/ixgbe/ixgbe_ethdev.c | 76
> > > > +--- drivers/net/ixgbe/ixgbe_ethdev.h
> > > > +|
> > > > +1
> > > > +
> > > >  2 files changed, 51 insertions(+), 26 deletions(-)
> > > >
> > > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > > > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > > > index 3aab24e82..08b4cc689 100644
> > > > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > > > +}
> > > > +
> > > >  err = ixgbe_dev_rxtx_start(dev);
> > > >  if (err < 0) {
> > > >  PMD_INIT_LOG(ERR, "Unable to start rxtx queues"); @@ -2900,6
> > > > +2939,8 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
> > > >
> > > >  adapter->rss_reta_updated = 0;
> > > >
> > > > +adapter->mac_ctrl_frame_fwd = 0;
> > > > +
> > >
> > > Delete it please.
> > >
> > >
> > >
> > >
> > >
> >
> 



Re: [dpdk-dev] [PATCH] net/ixgbe: fix statistics error in flow control mode

2020-05-12 Thread Sun, GuinanX
Hi zhaowei

> -Original Message-
> From: Zhao1, Wei
> Sent: Monday, May 11, 2020 9:03 AM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Sun, GuinanX ; sta...@dpdk.org; Guo, Jia
> 
> Subject: RE: [dpdk-dev] [PATCH] net/ixgbe: fix statistics error in flow 
> control
> mode
> 
> 
> Hi, Guinan
> 
> > -Original Message-
> > From: dev  On Behalf Of Guinan Sun
> > Sent: Friday, May 8, 2020 4:30 PM
> > To: dev@dpdk.org
> > Cc: Sun, GuinanX ; sta...@dpdk.org
> > Subject: [dpdk-dev] [PATCH] net/ixgbe: fix statistics error in flow
> > control mode
> >
> > The register autoneg can't be updated synchronously with flow control
> > mode setting in the state of port start , so NIC statistics error
> > occurs. The patch fixes the issue.
> >
> > Fixes: a524f550da6e ("net/ixgbe: fix flow control mode setting")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Guinan Sun 
> > ---
> >  drivers/net/ixgbe/ixgbe_ethdev.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index cf5f1fe70..e6c747aef 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -2543,6 +2543,8 @@ ixgbe_flow_ctrl_enable(struct rte_eth_dev *dev,
> > struct ixgbe_hw *hw)  int err;  uint32_t mflcn;
> >
> > +hw->mac.ops.setup_fc(hw);
> 
> 1. please use base code API ixgbe_setup_fc(), do not use internal function
> directly.
> 2. please more info for this patch, what register error you want to fix, and 
> why it
> can fix.

V2 patch will be send to resolve the problems.

> 
> 
> > +
> >  err = ixgbe_fc_enable(hw);
> >
> >  /* Not negotiated is not an error case */
> > --
> > 2.17.1
> 



Re: [dpdk-dev] [PATCH] net/ixgbe: delete MAC control frame fwd in struct adapter

2020-05-22 Thread Sun, GuinanX
Hi

> -Original Message-
> From: Zhao1, Wei
> Sent: Friday, May 22, 2020 5:47 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: sta...@dpdk.org; Yang, Qiming ; Ye, Xiaolong
> 
> Subject: RE: [PATCH] net/ixgbe: delete MAC control frame fwd in struct adapter
> 
> Hi,
> 
> > -----Original Message-
> > From: Sun, GuinanX 
> > Sent: Friday, May 22, 2020 2:12 PM
> > To: dev@dpdk.org
> > Cc: Zhao1, Wei ; Sun, GuinanX
> > ; sta...@dpdk.org
> > Subject: [PATCH] net/ixgbe: delete MAC control frame fwd in struct
> > adapter
> >
> > If some user set mac_ctrl_frame_fwd to 1 from fc ops, then he do a
> > port reset process of
> >
> > testpmd> start
> > testpmd> set flow_ctrl mac_ctrl_frame_fwd on 0 stop port stop 0 port
> > testpmd> start 0 start
> >
> > Then after this process, the mac_ctrl_frame_fwd has been change to
> > "off", so we should delete "adapter->mac_ctrl_frame_fwd = 0;" from dev-stop.
> >
> > In addition, add a value to mac_ctrl_frame_fwd in the
> > ixgbe_flow_ctrl_get () function.
> >
> > Fixes: a524f550da6e ("net/ixgbe: fix flow control mode setting")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: SunGuinan 
> > ---
> >  drivers/net/ixgbe/ixgbe_ethdev.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index f8a84c565..dd4023f01 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -2939,8 +2939,6 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
> >
> >  adapter->rss_reta_updated = 0;
> >
> > -adapter->mac_ctrl_frame_fwd = 0;
> > -
> >  hw->adapter_stopped = true;
> >  }
> >
> > @@ -4754,6 +4752,8 @@ ixgbe_flow_ctrl_get(struct rte_eth_dev *dev,
> > struct rte_eth_fc_conf *fc_conf)
> >   * MFLCN register.
> >   */
> >  mflcn_reg = IXGBE_READ_REG(hw, IXGBE_MFLCN);
> > +fc_conf->mac_ctrl_frame_fwd = mflcn_reg;
> > +
> 
> 
> Error, that is not right!!

You are right, I will make corrections later.
In addition, the problem of ixgbe_flow_ctrl_get () and the timing of 
mac_ctrl_frame_fwd are two problems. 
I will make a patch set and release it later.

> 
> >  if (mflcn_reg & (IXGBE_MFLCN_RPFCE | IXGBE_MFLCN_RFCE))  rx_pause =
> > 1;  else
> > --
> > 2.17.1
> 



Re: [dpdk-dev] [PATCH 00/21] update ixgbe base code

2020-06-14 Thread Sun, GuinanX
Hi zhaowei

> -Original Message-
> From: Zhao1, Wei
> Sent: Monday, June 15, 2020 1:47 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Sun, GuinanX 
> Subject: RE: [dpdk-dev] [PATCH 00/21] update ixgbe base code
> 
> Hi, Guinan
> 
> > -Original Message-
> > From: dev  On Behalf Of Guinan Sun
> > Sent: Friday, June 12, 2020 11:24 AM
> > To: dev@dpdk.org
> > Cc: Sun, GuinanX 
> > Subject: [dpdk-dev] [PATCH 00/21] update ixgbe base code
> >
> > update ixgbe base code.
> >
> > Guinan Sun (21):
> >   net/ixgbe/base: clear VFMBMEM and toggle VF's Tx queues
> >   net/ixgbe/base: change in the condition for response HI
> >   net/ixgbe/base: hange flow for "Apply Update" command
> >   net/ixgbe/base: x550em 10G NIC driver issue
> >   net/ixgbe/base: added API for NVM update
> >   net/ixgbe/base: resolve infinite recursion on PCIe link down
> >   net/ixgbe/base: added register definitions for NVM update
> >   net/ixgbe/base: cleanup spelling mistakes in comments
> >   net/ixgbe/base: remove whitespace in function comments
> >   net/ixgbe/base: move increments after evaluations
> >   net/ixgbe/base: modify loop accounting for retries
> >   net/ixgbe/base: create dedicated func to restart auto nego
> >   net/ixgbe/base: modify Klocwork hits for DDK 7.0
> >   net/ixgbe/base: add defines for min rollback revision fields
> >   net/ixgbe/base: remove unnecessary log message FC autonego
> >   net/ixgbe/base: initialize data field in struct buffer
> >   net/ixgbe/base: improve log about autonego being disabled
> >   net/ixgbe/base: ipv6 Mask for purpose FDIR VLAN Port Feature
> >   net/ixgbe/base: remove default advertising for 2.5G and 5G
> >   net/ixgbe/base: check Host Interface Return Status
> >   net/ixgbe/base: update version
> >
> >  drivers/net/ixgbe/base/README|2 +-
> >  drivers/net/ixgbe/base/ixgbe_82598.c |  238 ++---
> >  drivers/net/ixgbe/base/ixgbe_82599.c |  397 
> >  drivers/net/ixgbe/base/ixgbe_api.c   |  892 -
> >  drivers/net/ixgbe/base/ixgbe_api.h   |1 +
> >  drivers/net/ixgbe/base/ixgbe_common.c| 1102 --
> >  drivers/net/ixgbe/base/ixgbe_common.h|3 +-
> >  drivers/net/ixgbe/base/ixgbe_dcb.c   |6 +-
> >  drivers/net/ixgbe/base/ixgbe_dcb_82598.c |2 +-
> >  drivers/net/ixgbe/base/ixgbe_dcb_82599.c |2 +-
> >  drivers/net/ixgbe/base/ixgbe_hv_vf.c |   20 +-
> >  drivers/net/ixgbe/base/ixgbe_mbx.c   |  285 +++---
> >  drivers/net/ixgbe/base/ixgbe_mbx.h   |1 +
> >  drivers/net/ixgbe/base/ixgbe_phy.c   |  488 +-
> >  drivers/net/ixgbe/base/ixgbe_phy.h   |1 +
> >  drivers/net/ixgbe/base/ixgbe_type.h  |   67 ++
> >  drivers/net/ixgbe/base/ixgbe_vf.c|  166 ++--
> >  drivers/net/ixgbe/base/ixgbe_x540.c  |  190 ++--
> >  drivers/net/ixgbe/base/ixgbe_x550.c  |  505 +-
> >  19 files changed, 2282 insertions(+), 2086 deletions(-)
> >
> > --
> > 2.17.1
> 
> 
> Please give out tested-by for these patch later.

It will be given after the regression test is completed.
> 



Re: [dpdk-dev] [PATCH] net/i40e: enable port filter by switch filter

2020-06-14 Thread Sun, GuinanX
Hi zhaowei

> -Original Message-
> From: Zhao1, Wei
> Sent: Monday, June 15, 2020 2:13 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Xing, Beilei ; Guo, Jia ; Sun,
> GuinanX 
> Subject: RE: [dpdk-dev] [PATCH] net/i40e: enable port filter by switch filter
> 
> Hi, Guinan
> 
> > -Original Message-
> > From: dev  On Behalf Of Guinan Sun
> > Sent: Thursday, June 11, 2020 1:24 PM
> > To: dev@dpdk.org
> > Cc: Xing, Beilei ; Guo, Jia
> > ; Sun, GuinanX 
> > Subject: [dpdk-dev] [PATCH] net/i40e: enable port filter by switch
> > filter
> >
> > This patch enables the filter that supports to create following two
> > rules for the same packet type:
> > One is to select source port only as input set and the other is for
> > destination port only.
> >
> > Signed-off-by: Guinan Sun 
> > ---
> >  doc/guides/rel_notes/release_20_08.rst |   7 +
> >  drivers/net/i40e/i40e_ethdev.c | 195 -
> >  drivers/net/i40e/i40e_ethdev.h |  17 ++
> >  drivers/net/i40e/i40e_flow.c   | 223
> > +
> >  4 files changed, 441 insertions(+), 1 deletion(-)
> >
> > diff --git a/doc/guides/rel_notes/release_20_08.rst
> > b/doc/guides/rel_notes/release_20_08.rst
> > index 7a67c960c..16870100d 100644
> > --- a/doc/guides/rel_notes/release_20_08.rst
> > +++ b/doc/guides/rel_notes/release_20_08.rst
> > @@ -68,6 +68,13 @@ New Features
> >
> >* Added new PMD devarg ``reclaim_mem_mode``.
> >
> > +* **Updated Intel i40e driver.**
> > +
> > +  Updated i40e PMD with new features and improvements, including:
> > +
> > +  * Added a new type of cloud filter to support the coexistence of the
> > +following two rules. One selects L4 destination as input set and
> > +the other one selects L4 source port.
> >
> >  Removed Items
> >  -
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 970a31cb2..97e6e948a 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -7956,6 +7956,13 @@ i40e_dev_tunnel_filter_set(struct i40e_pf *pf,
> > #define I40E_TR_GRE_KEY_MASK0x400  #define
> > I40E_TR_GRE_KEY_WITH_XSUM_MASK0x800
> >  #define I40E_TR_GRE_NO_KEY_MASK0x8000
> 
> > +#define I40E_AQC_REPLACE_CLOUD_CMD_INPUT_PORT_TR_WORD0 0x49
> 
> A confused question, this 0x49 seems to be a field vector index? Filed vector
> table is 128 byte long, why does this can large than 64?
> 

This is not field vector. Examples of filed vector use are as follows
I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD1 or 
I40E_AQC_ADD_CLOUD_FV_FLU_0X10_WORD1.

> 
> > #define
> > +I40E_AQC_REPLACE_CLOUD_CMD_INPUT_DIRECTION_WORD0 0x41
> #define
> > +I40E_AQC_REPLACE_CLOUD_CMD_INPUT_INGRESS_WORD0 0x80 #define
> > +I40E_DIRECTION_INGRESS_KEY0x8000 #define I40E_TR_L4_TYPE_TCP0x2
> > +#define I40E_TR_L4_TYPE_UDP0x4 #define I40E_TR_L4_TYPE_SCTP0x8
> >
> >  static enum
> >  i40e_status_code i40e_replace_mpls_l1_filter(struct i40e_pf *pf) @@
> > -8254,6 +8261,131 @@ i40e_status_code
> > i40e_replace_gtp_cloud_filter(struct i40e_pf *pf)  return status;
> 



Re: [dpdk-dev] [PATCH] net/i40e: enable port filter by switch filter

2020-06-23 Thread Sun, GuinanX
Hi Guojia

> -Original Message-
> From: Guo, Jia
> Sent: Sunday, June 21, 2020 8:29 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Xing, Beilei 
> Subject: Re: [PATCH] net/i40e: enable port filter by switch filter
> 
> hi, guinan
> 
> On 6/11/2020 1:24 PM, Guinan Sun wrote:
> > This patch enables the filter that supports to create following two
> > rules for the same packet type:
> > One is to select source port only as input set and the other is for
> > destination port only.
> >
> > Signed-off-by: Guinan Sun 
> > ---
> >   doc/guides/rel_notes/release_20_08.rst |   7 +
> >   drivers/net/i40e/i40e_ethdev.c | 195 -
> >   drivers/net/i40e/i40e_ethdev.h |  17 ++
> >   drivers/net/i40e/i40e_flow.c   | 223 +
> >   4 files changed, 441 insertions(+), 1 deletion(-)
> >
> > diff --git a/doc/guides/rel_notes/release_20_08.rst
> > b/doc/guides/rel_notes/release_20_08.rst
> > index 7a67c960c..16870100d 100644
> > --- a/doc/guides/rel_notes/release_20_08.rst
> > +++ b/doc/guides/rel_notes/release_20_08.rst
> > @@ -68,6 +68,13 @@ New Features
> >
> > * Added new PMD devarg ``reclaim_mem_mode``.
> >
> > +* **Updated Intel i40e driver.**
> > +
> > +  Updated i40e PMD with new features and improvements, including:
> > +
> > +  * Added a new type of cloud filter to support the coexistence of the
> > +following two rules. One selects L4 destination as input set and
> > +the other one selects L4 source port.
> >
> >   Removed Items
> >   -
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 970a31cb2..97e6e948a 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -7956,6 +7956,13 @@ i40e_dev_tunnel_filter_set(struct i40e_pf *pf,
> >   #define I40E_TR_GRE_KEY_MASK  0x400
> >   #define I40E_TR_GRE_KEY_WITH_XSUM_MASK0x800
> >   #define I40E_TR_GRE_NO_KEY_MASK   0x8000
> > +#define I40E_AQC_REPLACE_CLOUD_CMD_INPUT_PORT_TR_WORD0 0x49
> #define
> > +I40E_AQC_REPLACE_CLOUD_CMD_INPUT_DIRECTION_WORD0 0x41
> #define
> > +I40E_AQC_REPLACE_CLOUD_CMD_INPUT_INGRESS_WORD0 0x80
> > +#define I40E_DIRECTION_INGRESS_KEY 0x8000
> > +#define I40E_TR_L4_TYPE_TCP0x2
> > +#define I40E_TR_L4_TYPE_UDP0x4
> > +#define I40E_TR_L4_TYPE_SCTP   0x8
> >
> >   static enum
> >   i40e_status_code i40e_replace_mpls_l1_filter(struct i40e_pf *pf) @@
> > -8254,6 +8261,131 @@ i40e_status_code
> i40e_replace_gtp_cloud_filter(struct i40e_pf *pf)
> > return status;
> >   }
> >
> > +static enum i40e_status_code
> > +i40e_replace_port_l1_filter(struct i40e_pf *pf, enum
> > +i40e_l4_port_type port_type) {
> > +   struct i40e_aqc_replace_cloud_filters_cmd_buf  filter_replace_buf;
> > +   struct i40e_aqc_replace_cloud_filters_cmd  filter_replace;
> > +   enum i40e_status_code status = I40E_SUCCESS;
> > +   struct i40e_hw *hw = I40E_PF_TO_HW(pf);
> > +   struct rte_eth_dev *dev = ((struct i40e_adapter
> > +*)hw->back)->eth_dev;
> > +
> 
> 
> The Christmas tree would be look good?
> 
The definition of dev has no way to follow the chrismas tree principle.
> 
> > +   if (pf->support_multi_driver) {
> > +   PMD_DRV_LOG(ERR, "Replace l1 filter is not supported.");
> > +   return I40E_NOT_SUPPORTED;
> > +   }
> > +
> > +   memset(&filter_replace, 0,
> > +  sizeof(struct i40e_aqc_replace_cloud_filters_cmd));
> > +   memset(&filter_replace_buf, 0,
> > +  sizeof(struct i40e_aqc_replace_cloud_filters_cmd_buf));
> > +
> > +   /* create L1 filter */
> > +   if (port_type == I40E_L4_PORT_TYPE_SRC) {
> > +   filter_replace.old_filter_type =
> > +
>   I40E_AQC_REPLACE_CLOUD_CMD_INPUT_FV_TUNNLE_KEY;
> > +   filter_replace.new_filter_type =
> I40E_AQC_ADD_CLOUD_FILTER_0X11;
> > +   filter_replace_buf.data[8] =
> > +
>   I40E_AQC_REPLACE_CLOUD_CMD_INPUT_FV_SRC_PORT;
> > +   } else {
> > +   filter_replace.old_filter_type =
> > +
>   I40E_AQC_REPLACE_CLOUD_CMD_INPUT_FV_STAG_IVLAN;
> > +   filter_replace.new_filter_type =
> I40E_AQC_ADD_CLOUD_FILTER_0X10;
> > +   filter_replace_buf.data[8] =
> > +
>   I40E_AQC_REPLACE_CLOUD_CMD_INPUT_FV_DST_PORT;
> > +   

Re: [dpdk-dev] [PATCH 01/21] net/ixgbe/base: clear VFMBMEM and toggle VF's Tx queues

2020-06-29 Thread Sun, GuinanX
Hi Ferruh

> -Original Message-
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 7:59 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Pietruszewski, Piotr 
> Subject: Re: [dpdk-dev] [PATCH 01/21] net/ixgbe/base: clear VFMBMEM and
> toggle VF's Tx queues
> 
> On 6/12/2020 4:23 AM, Guinan Sun wrote:
> > Add a method to clear VFMBMEM memory.
> 
> Can you please give some context, what is "VFMBMEM memory", why need to
> clear it, etc...?
> 
> > Add a method to toggle VF's TX queues as workaround for silicon
> > errata.
> 
> 
> 'toggle' here means enable and disable the Tx queues right?
> Will this 'ixgbe_toggle_txdctl()' function used by the driver, if so better 
> to have
> that change in the same patch to get the full context.
> 
> Are there two changes related to eachother, 'txdctl' & 'VFMBMEM ', if not can
> you please seperate them?
> 

Sorry, our commit message caused you confusion.
Later V2 patch will modify the commit information and explain'txdctl' 
&'VFMBMEM'.

> >
> > Signed-off-by: Piotr Pietruszewski 
> > Signed-off-by: Guinan Sun 
> 
> <...>
> 
> > +/**
> > + *  ixgbe_clear_mbx - Clear Mailbox Memory
> > + *  @hw: pointer to the HW structure
> > + *  @vf_number: id of mailbox to write
> > + *
> > + *  Set VFMBMEM of given VF to 0x0.
> > + **/
> > +s32 ixgbe_clear_mbx(struct ixgbe_hw *hw, u16 vf_number) {
> > +   struct ixgbe_mbx_info *mbx = &hw->mbx;
> > +   s32 ret_val = IXGBE_SUCCESS;
> > +
> > +   DEBUGFUNC("ixgbe_clear_mbx");
> > +
> > +   if (mbx->ops.clear)
> > +   ret_val = mbx->ops.clear(hw, vf_number);
> > +
> > +   return ret_val;
> > +}
> > +
> >  /**
> >   *  ixgbe_poll_for_msg - Wait for message notification
> >   *  @hw: pointer to the HW structure
> > @@ -486,6 +506,7 @@ void ixgbe_init_mbx_params_vf(struct ixgbe_hw *hw)
> > mbx->ops.check_for_msg = ixgbe_check_for_msg_vf;
> > mbx->ops.check_for_ack = ixgbe_check_for_ack_vf;
> > mbx->ops.check_for_rst = ixgbe_check_for_rst_vf;
> > +   mbx->ops.clear = NULL;
> 
> If it is not used why 'ixgbe_clear_mbx()' added?


Re: [dpdk-dev] [PATCH 02/21] net/ixgbe/base: change in the condition for response HI

2020-06-29 Thread Sun, GuinanX
Hi Ferruh

> -Original Message-
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 7:59 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Mateusz Kowalski 
> Subject: Re: [dpdk-dev] [PATCH 02/21] net/ixgbe/base: change in the condition
> for response HI
> 
> On 6/12/2020 4:23 AM, Guinan Sun wrote:
> > According to SGVL EAS Host interface Shadow RAM Read (0x31) command
> > response buffer length is stored in two bytes, instead of one byte.
> 
> For patch subject, better to say 'fix' instead of 'change' because that is 
> what
> done.
> Previously Shadow RAM Read (0x31) command response buffer length was
> taken wrong, now you are fixing it. Overall "Shadow RAM Read (0x31)"
> command response value was wrong, so I guess it can be fair to say:
> "net/ixgbe/base: fix host interface shadow RAM read"
> 

I agree with you.
V2 patch will fix the problem.

> >
> > Signed-off-by: Mateusz Kowalski 
> > Signed-off-by: Guinan Sun 
> > ---
> >  drivers/net/ixgbe/base/ixgbe_common.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ixgbe/base/ixgbe_common.c
> > b/drivers/net/ixgbe/base/ixgbe_common.c
> > index 36c003844..9e3b71e38 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_common.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_common.c
> > @@ -4656,7 +4656,7 @@ s32 ixgbe_host_interface_command(struct
> ixgbe_hw *hw, u32 *buffer,
> >  * Read Flash command requires reading buffer length from
> >  * two byes instead of one byte
> >  */
> > -   if (resp->cmd == 0x30) {
> > +   if (resp->cmd == 0x30 || resp->cmd == 0x31) {
> > for (; bi < dword_len + 2; bi++) {
> > buffer[bi] = IXGBE_READ_REG_ARRAY(hw,
> IXGBE_FLEX_MNG,
> >   bi);
> >



Re: [dpdk-dev] [PATCH 03/21] net/ixgbe/base: hange flow for "Apply Update" command

2020-06-29 Thread Sun, GuinanX
Hi Ferruh

> -Original Message-
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 7:59 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Mateusz Kowalski 
> Subject: Re: [dpdk-dev] [PATCH 03/21] net/ixgbe/base: hange flow for "Apply
> Update" command
> 
> On 6/12/2020 4:23 AM, Guinan Sun wrote:
> > For the "Apply Update" command the firmware does not given an
> > response. For this command, success should be return. Based on
> > information at EAS.
> >
> > Signed-off-by: Mateusz Kowalski 
> > Signed-off-by: Guinan Sun 
> 
> <...>
> 
> > diff --git a/drivers/net/ixgbe/base/ixgbe_type.h
> > b/drivers/net/ixgbe/base/ixgbe_type.h
> > index 0470b1dfc..33ca659cd 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_type.h
> > +++ b/drivers/net/ixgbe/base/ixgbe_type.h
> > @@ -4366,4 +4366,16 @@ struct ixgbe_hw {
> >  #define IXGBE_NW_MNG_IF_SEL_MDIO_PHY_ADD   \
> > (0x1F <<
> IXGBE_NW_MNG_IF_SEL_MDIO_PHY_ADD_SHIFT)
> >
> > +/* Code Command (Flash I/F Interface) */
> > +#define IXGBE_HOST_INTERFACE_FLASH_READ_CMD0x30
> > +#define IXGBE_HOST_INTERFACE_SHADOW_RAM_READ_CMD
>   0x31
> > +#define IXGBE_HOST_INTERFACE_FLASH_WRITE_CMD
>   0x32
> > +#define IXGBE_HOST_INTERFACE_SHADOW_RAM_WRITE_CMD
>   0x33
> > +#define IXGBE_HOST_INTERFACE_FLASH_MODULE_UPDATE_CMD
>   0x34
> > +#define IXGBE_HOST_INTERFACE_FLASH_BLOCK_EREASE_CMD
>   0x35
> > +#define IXGBE_HOST_INTERFACE_SHADOW_RAM_DUMP_CMD
>   0x36
> > +#define IXGBE_HOST_INTERFACE_FLASH_INFO_CMD0x37
> > +#define IXGBE_HOST_INTERFACE_APPLY_UPDATE_CMD
>   0x38
> > +#define IXGBE_HOST_INTERFACE_MASK_CMD
>   0x00FF
> > +
> >  #endif /* _IXGBE_TYPE_H_ */
> >
> 
> Previous patch uses these commands in a hardcoded way, since we are adding
> defines, why not them in the previous patch so we can use the macros in
> previous patch?

I agree with you.
V2 patch we will split this patch into two parts.
The part about macrow will be merged into the previous patch.


Re: [dpdk-dev] [PATCH 05/21] net/ixgbe/base: added API for NVM update

2020-06-29 Thread Sun, GuinanX
Hi Ferruh

> -Original Message-
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 7:59 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Skajewski, PiotrX 
> Subject: Re: [dpdk-dev] [PATCH 05/21] net/ixgbe/base: added API for NVM
> update
> 
> On 6/12/2020 4:23 AM, Guinan Sun wrote:
> > When Secure Boot is enabled access to the /dev/mem is forbidden for
> > user-space applications and clients are reporting inability to use
> > tools in Secure Boot Mode. The way to perform NVM update is to use
> > ixgbe driver. Currently 10G Linux Base Driver has API which allows
> > only EEPROM access. There is a need to extend IOCTL API to allow NVM
> > and registers access.
> 
> If I understand this correctly, this is to enable NVM update through 'ixgbe'
> kernel driver, is this needed for the DPDK?
> 
> >
> > Signed-off-by: Piotr Skajewski 
> > Signed-off-by: Guinan Sun 
> 
> <...>
> 
> > @@ -4200,6 +4236,10 @@ struct ixgbe_hw {
> > bool allow_unsupported_sfp;
> > bool wol_enabled;
> > bool need_crosstalk_fix;
> > +#ifdef IXGBE_NVMUPD_SUPPORT
> > +   /* NVM Update features */
> > +   struct ixgbe_nvm_features nvmupd_features; #endif
> 
> In DPDK is there anywhere that sets this 'IXGBE_NVMUPD_SUPPORT' define?

V2 patch will remove this code.


Re: [dpdk-dev] [PATCH 11/21] net/ixgbe/base: modify loop accounting for retries

2020-06-29 Thread Sun, GuinanX
Hi Ferruh

> -Original Message-
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 8:00 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Cramer, Jeb J 
> Subject: Re: [dpdk-dev] [PATCH 11/21] net/ixgbe/base: modify loop accounting
> for retries
> 
> On 6/12/2020 4:24 AM, Guinan Sun wrote:
> > The condition for comparing retry against max_retry was flawed in the
> > do-while loops.  For the case where retry was initialized to 0 and
> > max_retry was initialized to 1, we'd break out of the loop at the
> > condition when the intent is to retry the code at least once.
> > Otherwise, the loop is unnecessary.  The other places have a larger
> > max_retry so code would get run multiple times (if necessary), but not
> > to the intended extent.
> >
> > Signed-off-by: Jeb Cramer 
> > Signed-off-by: Guinan Sun 
> > ---
> >  drivers/net/ixgbe/base/ixgbe_phy.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c
> > b/drivers/net/ixgbe/base/ixgbe_phy.c
> > index 9bb24f1ef..823cf161e 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> > @@ -143,7 +143,7 @@ s32 ixgbe_read_i2c_combined_generic_int(struct
> ixgbe_hw *hw, u8 addr, u16 reg,
> > else
> > DEBUGOUT("I2C byte read combined error.\n");
> > retry++;
> > -   } while (retry < max_retry);
> > +   } while (retry <= max_retry);
> >
> > return IXGBE_ERR_I2C;
> 
> Ahh, previous patch becomes correct with this change, can you please combine
> them? No need to break first and fix later.

Patch V2 will fix it.


Re: [dpdk-dev] [PATCH 10/21] net/ixgbe/base: move increments after evaluations

2020-06-29 Thread Sun, GuinanX
Hi Ferruh

> -Original Message-
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 7:59 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Cramer, Jeb J 
> Subject: Re: [dpdk-dev] [PATCH 10/21] net/ixgbe/base: move increments after
> evaluations
> 
> On 6/12/2020 4:23 AM, Guinan Sun wrote:
> > The retry variable was being incremented before it was evaluated by
> > the subsequent conditional against the maximum retries to figure out
> > which message to print.  So we'll move the increment op to the end.
> >
> > Signed-off-by: Jeb Cramer 
> > Signed-off-by: Guinan Sun 
> > ---
> >  drivers/net/ixgbe/base/ixgbe_phy.c | 9 -
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c
> > b/drivers/net/ixgbe/base/ixgbe_phy.c
> > index 13f00ac67..9bb24f1ef 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> > @@ -138,11 +138,11 @@ s32 ixgbe_read_i2c_combined_generic_int(struct
> ixgbe_hw *hw, u8 addr, u16 reg,
> > ixgbe_i2c_bus_clear(hw);
> > if (lock)
> > hw->mac.ops.release_swfw_sync(hw, swfw_mask);
> > -   retry++;
> > if (retry < max_retry)
> > DEBUGOUT("I2C byte read combined error -
> Retrying.\n");
> > else
> > DEBUGOUT("I2C byte read combined error.\n");
> > +   retry++;
> > } while (retry < max_retry);
> >
> 
> Isn't this wrong?
> With this update "DEBUGOUT("I2C byte read combined error.\n");" log won't
> be printed at all.
> And instead why not keep "Retrying" log in the loop without check and move
> "error" log out of loop, that looks easier to me...

The next patch will be combined with the current patch later, which will fix 
the problem.



[dpdk-dev] [PATCH] net/ixgbe: fix macsec setting

2019-10-24 Thread Sun GuinanX
macsec setting is not valid when port is stopped.
In order to make it valid, the patch changes the setting
to where port is started.

Fixes: 597f9fafe13b ("app/testpmd: convert to new Tx offloads API")
Cc: sta...@dpdk.org

Signed-off-by: Sun GuinanX 
---
 drivers/net/ixgbe/ixgbe_ethdev.c  | 160 ++
 drivers/net/ixgbe/ixgbe_ethdev.h  |  15 +++
 drivers/net/ixgbe/rte_pmd_ixgbe.c |   9 ++
 3 files changed, 184 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index dbce7a80e..29455f7ee 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -379,6 +379,11 @@ static int ixgbe_dev_udp_tunnel_port_del(struct 
rte_eth_dev *dev,
 static int ixgbe_filter_restore(struct rte_eth_dev *dev);
 static void ixgbe_l2_tunnel_conf(struct rte_eth_dev *dev);
 
+static void ixgbe_dev_macsec_ctrl_register_enable(struct rte_eth_dev *dev,
+   struct ixgbe_macsec_ctrl *macsec_contrl);
+
+static void ixgbe_dev_macsec_ctrl_register_disable(struct rte_eth_dev *dev);
+
 /*
  * Define VF Stats MACRO for Non "cleared on read" register
  */
@@ -2542,6 +2547,8 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
uint32_t *link_speeds;
struct ixgbe_tm_conf *tm_conf =
IXGBE_DEV_PRIVATE_TO_TM_CONF(dev->data->dev_private);
+   struct ixgbe_macsec_ctrl *macsec_ctrl =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data->dev_private);
 
PMD_INIT_FUNC_TRACE();
 
@@ -2794,6 +2801,9 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 */
ixgbe_dev_link_update(dev, 0);
 
+   /* setup the macsec ctrl register */
+   ixgbe_dev_macsec_ctrl_register_enable(dev, macsec_ctrl);
+
return 0;
 
 error:
@@ -2825,6 +2835,9 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
 
PMD_INIT_FUNC_TRACE();
 
+   /* disable mecsec register */
+   ixgbe_dev_macsec_ctrl_register_disable(dev);
+
rte_eal_alarm_cancel(ixgbe_dev_setup_link_alarm_handler, dev);
 
/* disable interrupts */
@@ -8816,6 +8829,153 @@ ixgbe_clear_all_l2_tn_filter(struct rte_eth_dev *dev)
return 0;
 }
 
+/* macsec ctrl setup enable */
+void
+ixgbe_dev_macsec_ctrl_setup_enable(struct rte_eth_dev *dev,
+   struct ixgbe_macsec_ctrl *macsec_ctrl)
+{
+   struct ixgbe_macsec_ctrl *macsec =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data->dev_private);
+
+   macsec->encrypt_en = macsec_ctrl->encrypt_en;
+   macsec->replayprotect_en = macsec_ctrl->replayprotect_en;
+}
+
+/* macsec ctrl setup disable */
+void
+ixgbe_dev_macsec_ctrl_setup_disable(struct rte_eth_dev *dev)
+{
+   struct ixgbe_macsec_ctrl *macsec =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data->dev_private);
+
+   macsec->encrypt_en = 0;
+   macsec->replayprotect_en = 0;
+}
+
+/* macsec ctrl register set */
+static void
+ixgbe_dev_macsec_ctrl_register_enable(struct rte_eth_dev *dev,
+   struct ixgbe_macsec_ctrl *macsec_contrl)
+{
+   struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+   uint32_t ctrl;
+   uint8_t en = (uint8_t)macsec_contrl->encrypt_en;
+   uint8_t rp = (uint8_t)macsec_contrl->replayprotect_en;
+
+   /**
+* Workaround:
+* As no ixgbe_disable_sec_rx_path equivalent is
+* implemented for tx in the base code, and we are
+* not allowed to modify the base code in DPDK, so
+* just call the hand-written one directly for now.
+* The hardware support has been checked by
+* ixgbe_disable_sec_rx_path().
+*/
+   ixgbe_disable_sec_tx_path_generic(hw);
+
+   /* Enable Ethernet CRC (required by MACsec offload) */
+   ctrl = IXGBE_READ_REG(hw, IXGBE_HLREG0);
+   ctrl |= IXGBE_HLREG0_TXCRCEN | IXGBE_HLREG0_RXCRCSTRP;
+   IXGBE_WRITE_REG(hw, IXGBE_HLREG0, ctrl);
+
+   /* Enable the TX and RX crypto engines */
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
+   ctrl &= ~IXGBE_SECTXCTRL_SECTX_DIS;
+   IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, ctrl);
+
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
+   ctrl &= ~IXGBE_SECRXCTRL_SECRX_DIS;
+   IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, ctrl);
+
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
+   ctrl &= ~IXGBE_SECTX_MINSECIFG_MASK;
+   ctrl |= 0x3;
+   IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, ctrl);
+
+   /* Enable SA lookup */
+   ctrl = IXGBE_READ_REG(hw, IXGBE_LSECTXCTRL);
+   ctrl &= ~IXGBE_LSECTXCTRL_EN_MASK;
+   ctrl |= en ? IXGBE_LSECTXCTRL_AUTH_ENCRYPT :
+IXGBE_LSECTXCTRL_AUTH;
+   ctrl |= IXGBE_LSECTXCTRL_AISCI;
+   ctrl &= ~IXGBE_LSECTXCTRL_PNTHRSH_MASK;
+   ctrl |= IXGBE_MACSEC_PNTHRSH & IXGBE_LSECTXCTRL_PNTHRSH_MASK;
+   IXGBE_WRITE_REG(hw, IXGBE_LSECTXCTRL, ctrl)

[dpdk-dev] [PATCH v2] net/ixgbe: fix macsec setting

2019-10-29 Thread Sun GuinanX
macsec setting is not valid when port is stopped.
In order to make it valid, the patch changes the setting
to where port is started.

Fixes: 597f9fafe13b ("app/testpmd: convert to new Tx offloads API")
Cc: sta...@dpdk.org

Signed-off-by: Sun GuinanX 
---
v2:
* Modified function name
* Refactored the rte_pmd_ixgbe_macsec_enable/disable function
* Modified structure name
* Modified the data type of a structure variable
---
 drivers/net/ixgbe/ixgbe_ethdev.c  | 160 ++
 drivers/net/ixgbe/ixgbe_ethdev.h  |  15 +++
 drivers/net/ixgbe/rte_pmd_ixgbe.c | 127 ++--
 3 files changed, 182 insertions(+), 120 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index dbce7a80e..7ae7bc9ca 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -379,6 +379,11 @@ static int ixgbe_dev_udp_tunnel_port_del(struct 
rte_eth_dev *dev,
 static int ixgbe_filter_restore(struct rte_eth_dev *dev);
 static void ixgbe_l2_tunnel_conf(struct rte_eth_dev *dev);
 
+static void ixgbe_dev_macsec_ctrl_register_enable(struct rte_eth_dev *dev,
+   struct ixgbe_macsec_setting *macsec_contrl);
+
+static void ixgbe_dev_macsec_ctrl_register_disable(struct rte_eth_dev *dev);
+
 /*
  * Define VF Stats MACRO for Non "cleared on read" register
  */
@@ -2542,6 +2547,8 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
uint32_t *link_speeds;
struct ixgbe_tm_conf *tm_conf =
IXGBE_DEV_PRIVATE_TO_TM_CONF(dev->data->dev_private);
+   struct ixgbe_macsec_setting *macsec_ctrl =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data->dev_private);
 
PMD_INIT_FUNC_TRACE();
 
@@ -2794,6 +2801,9 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 */
ixgbe_dev_link_update(dev, 0);
 
+   /* setup the macsec ctrl register */
+   ixgbe_dev_macsec_ctrl_register_enable(dev, macsec_ctrl);
+
return 0;
 
 error:
@@ -2825,6 +2835,9 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
 
PMD_INIT_FUNC_TRACE();
 
+   /* disable mecsec register */
+   ixgbe_dev_macsec_ctrl_register_disable(dev);
+
rte_eal_alarm_cancel(ixgbe_dev_setup_link_alarm_handler, dev);
 
/* disable interrupts */
@@ -8816,6 +8829,153 @@ ixgbe_clear_all_l2_tn_filter(struct rte_eth_dev *dev)
return 0;
 }
 
+/* macsec ctrl setup enable */
+void
+ixgbe_dev_macsec_setting_save(struct rte_eth_dev *dev,
+   struct ixgbe_macsec_setting *macsec_ctrl)
+{
+   struct ixgbe_macsec_setting *macsec =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data->dev_private);
+
+   macsec->encrypt_en = macsec_ctrl->encrypt_en;
+   macsec->replayprotect_en = macsec_ctrl->replayprotect_en;
+}
+
+/* macsec ctrl setup disable */
+void
+ixgbe_dev_macsec_setting_reset(struct rte_eth_dev *dev)
+{
+   struct ixgbe_macsec_setting *macsec =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data->dev_private);
+
+   macsec->encrypt_en = 0;
+   macsec->replayprotect_en = 0;
+}
+
+/* macsec ctrl register set */
+static void
+ixgbe_dev_macsec_ctrl_register_enable(struct rte_eth_dev *dev,
+   struct ixgbe_macsec_setting *macsec_contrl)
+{
+   struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+   uint32_t ctrl;
+   uint8_t en = (uint8_t)macsec_contrl->encrypt_en;
+   uint8_t rp = (uint8_t)macsec_contrl->replayprotect_en;
+
+   /**
+* Workaround:
+* As no ixgbe_disable_sec_rx_path equivalent is
+* implemented for tx in the base code, and we are
+* not allowed to modify the base code in DPDK, so
+* just call the hand-written one directly for now.
+* The hardware support has been checked by
+* ixgbe_disable_sec_rx_path().
+*/
+   ixgbe_disable_sec_tx_path_generic(hw);
+
+   /* Enable Ethernet CRC (required by MACsec offload) */
+   ctrl = IXGBE_READ_REG(hw, IXGBE_HLREG0);
+   ctrl |= IXGBE_HLREG0_TXCRCEN | IXGBE_HLREG0_RXCRCSTRP;
+   IXGBE_WRITE_REG(hw, IXGBE_HLREG0, ctrl);
+
+   /* Enable the TX and RX crypto engines */
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
+   ctrl &= ~IXGBE_SECTXCTRL_SECTX_DIS;
+   IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, ctrl);
+
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
+   ctrl &= ~IXGBE_SECRXCTRL_SECRX_DIS;
+   IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, ctrl);
+
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
+   ctrl &= ~IXGBE_SECTX_MINSECIFG_MASK;
+   ctrl |= 0x3;
+   IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, ctrl);
+
+   /* Enable SA lookup */
+   ctrl = IXGBE_READ_REG(hw, IXGBE_LSECTXCTRL);
+   ctrl &= ~IXGBE_LSECTXCTRL_EN_MASK;
+   ctrl |= en ? IXGBE_LSECTXCTRL_AUTH_ENCRYPT :
+IXGBE_LSECTXCTRL_AUTH;
+   ct

[dpdk-dev] [PATCH v3] net/ixgbe: fix macsec setting

2019-10-29 Thread Sun GuinanX
macsec setting is not valid when port is stopped.
In order to make it valid, the patch changes the setting
to where port is started.

Fixes: 597f9fafe13b ("app/testpmd: convert to new Tx offloads API")
Cc: sta...@dpdk.org

Signed-off-by: Sun GuinanX 
---
v3:
* Deleted extra comments
* Modified the function name of the setting register
* Increased the logic used to set the register in the port start state
v2:
* Modified function name
* Refactored the rte_pmd_ixgbe_macsec_enable/disable function
* Modified structure name
* Modified the data type of a structure variable
---
 drivers/net/ixgbe/ixgbe_ethdev.c  | 160 ++
 drivers/net/ixgbe/ixgbe_ethdev.h  |  15 +++
 drivers/net/ixgbe/rte_pmd_ixgbe.c | 127 ++--
 3 files changed, 182 insertions(+), 120 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index dbce7a80e..7ae7bc9ca 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -379,6 +379,11 @@ static int ixgbe_dev_udp_tunnel_port_del(struct 
rte_eth_dev *dev,
 static int ixgbe_filter_restore(struct rte_eth_dev *dev);
 static void ixgbe_l2_tunnel_conf(struct rte_eth_dev *dev);
 
+static void ixgbe_dev_macsec_ctrl_register_enable(struct rte_eth_dev *dev,
+   struct ixgbe_macsec_setting *macsec_contrl);
+
+static void ixgbe_dev_macsec_ctrl_register_disable(struct rte_eth_dev *dev);
+
 /*
  * Define VF Stats MACRO for Non "cleared on read" register
  */
@@ -2542,6 +2547,8 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
uint32_t *link_speeds;
struct ixgbe_tm_conf *tm_conf =
IXGBE_DEV_PRIVATE_TO_TM_CONF(dev->data->dev_private);
+   struct ixgbe_macsec_setting *macsec_ctrl =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data->dev_private);
 
PMD_INIT_FUNC_TRACE();
 
@@ -2794,6 +2801,9 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 */
ixgbe_dev_link_update(dev, 0);
 
+   /* setup the macsec ctrl register */
+   ixgbe_dev_macsec_ctrl_register_enable(dev, macsec_ctrl);
+
return 0;
 
 error:
@@ -2825,6 +2835,9 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
 
PMD_INIT_FUNC_TRACE();
 
+   /* disable mecsec register */
+   ixgbe_dev_macsec_ctrl_register_disable(dev);
+
rte_eal_alarm_cancel(ixgbe_dev_setup_link_alarm_handler, dev);
 
/* disable interrupts */
@@ -8816,6 +8829,153 @@ ixgbe_clear_all_l2_tn_filter(struct rte_eth_dev *dev)
return 0;
 }
 
+/* macsec ctrl setup enable */
+void
+ixgbe_dev_macsec_setting_save(struct rte_eth_dev *dev,
+   struct ixgbe_macsec_setting *macsec_ctrl)
+{
+   struct ixgbe_macsec_setting *macsec =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data->dev_private);
+
+   macsec->encrypt_en = macsec_ctrl->encrypt_en;
+   macsec->replayprotect_en = macsec_ctrl->replayprotect_en;
+}
+
+/* macsec ctrl setup disable */
+void
+ixgbe_dev_macsec_setting_reset(struct rte_eth_dev *dev)
+{
+   struct ixgbe_macsec_setting *macsec =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data->dev_private);
+
+   macsec->encrypt_en = 0;
+   macsec->replayprotect_en = 0;
+}
+
+/* macsec ctrl register set */
+static void
+ixgbe_dev_macsec_ctrl_register_enable(struct rte_eth_dev *dev,
+   struct ixgbe_macsec_setting *macsec_contrl)
+{
+   struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+   uint32_t ctrl;
+   uint8_t en = (uint8_t)macsec_contrl->encrypt_en;
+   uint8_t rp = (uint8_t)macsec_contrl->replayprotect_en;
+
+   /**
+* Workaround:
+* As no ixgbe_disable_sec_rx_path equivalent is
+* implemented for tx in the base code, and we are
+* not allowed to modify the base code in DPDK, so
+* just call the hand-written one directly for now.
+* The hardware support has been checked by
+* ixgbe_disable_sec_rx_path().
+*/
+   ixgbe_disable_sec_tx_path_generic(hw);
+
+   /* Enable Ethernet CRC (required by MACsec offload) */
+   ctrl = IXGBE_READ_REG(hw, IXGBE_HLREG0);
+   ctrl |= IXGBE_HLREG0_TXCRCEN | IXGBE_HLREG0_RXCRCSTRP;
+   IXGBE_WRITE_REG(hw, IXGBE_HLREG0, ctrl);
+
+   /* Enable the TX and RX crypto engines */
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
+   ctrl &= ~IXGBE_SECTXCTRL_SECTX_DIS;
+   IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, ctrl);
+
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
+   ctrl &= ~IXGBE_SECRXCTRL_SECRX_DIS;
+   IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, ctrl);
+
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
+   ctrl &= ~IXGBE_SECTX_MINSECIFG_MASK;
+   ctrl |= 0x3;
+   IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, ctrl);
+
+   /* Enable SA lookup */
+   ctrl = IXGBE_READ_RE

Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix macsec setting

2019-10-29 Thread Sun, GuinanX
Hi, Xiaolong

On 10/29, Sun GuinanX wrote:
> >macsec setting is not valid when port is stopped.
> >In order to make it valid, the patch changes the setting to where port
> >is started.
> >
> >Fixes: 597f9fafe13b ("app/testpmd: convert to new Tx offloads API")
> >Cc: sta...@dpdk.org
> >
> >Signed-off-by: Sun GuinanX 
> >---
> >v2:
> >* Modified function name
> >* Refactored the rte_pmd_ixgbe_macsec_enable/disable function
> >* Modified structure name
> >* Modified the data type of a structure variable
> >---
> > drivers/net/ixgbe/ixgbe_ethdev.c  | 160 ++
> >drivers/net/ixgbe/ixgbe_ethdev.h  |  15 +++
> >drivers/net/ixgbe/rte_pmd_ixgbe.c | 127 ++--
> > 3 files changed, 182 insertions(+), 120 deletions(-)
> >
> >diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> >b/drivers/net/ixgbe/ixgbe_ethdev.c
> >index dbce7a80e..7ae7bc9ca 100644
> >--- a/drivers/net/ixgbe/ixgbe_ethdev.c
> >+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> >@@ -379,6 +379,11 @@ static int ixgbe_dev_udp_tunnel_port_del(struct
> >rte_eth_dev *dev,  static int ixgbe_filter_restore(struct rte_eth_dev
> >*dev);  static void ixgbe_l2_tunnel_conf(struct rte_eth_dev *dev);
> >
> >+static void ixgbe_dev_macsec_ctrl_register_enable(struct rte_eth_dev *dev,
> >+struct ixgbe_macsec_setting *macsec_contrl);
> >+
> >+static void ixgbe_dev_macsec_ctrl_register_disable(struct rte_eth_dev
> >+*dev);
> >+
> > /*
> >  * Define VF Stats MACRO for Non "cleared on read" register
> >  */
> >@@ -2542,6 +2547,8 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
> > uint32_t *link_speeds;
> > struct ixgbe_tm_conf *tm_conf =
> > IXGBE_DEV_PRIVATE_TO_TM_CONF(dev->data->dev_private);
> >+struct ixgbe_macsec_setting *macsec_ctrl =
> >+IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data-
> >dev_private);
> >
> > PMD_INIT_FUNC_TRACE();
> >
> >@@ -2794,6 +2801,9 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
> >  */
> > ixgbe_dev_link_update(dev, 0);
> >
> >+/* setup the macsec ctrl register */
> >+ixgbe_dev_macsec_ctrl_register_enable(dev, macsec_ctrl);
> >+
> > return 0;
> >
> > error:
> >@@ -2825,6 +2835,9 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
> >
> > PMD_INIT_FUNC_TRACE();
> >
> >+/* disable mecsec register */
> >+ixgbe_dev_macsec_ctrl_register_disable(dev);
> >+
> > rte_eal_alarm_cancel(ixgbe_dev_setup_link_alarm_handler, dev);
> >
> > /* disable interrupts */
> >@@ -8816,6 +8829,153 @@ ixgbe_clear_all_l2_tn_filter(struct rte_eth_dev
> *dev)
> > return 0;
> > }
> >
> >+/* macsec ctrl setup enable */
> 
> This comment is not aligned with the function name, actually since this 
> function
> is quite straightforward, the comment is unnecessary.
Similar useless comments had been removed
> 
> >+void
> >+ixgbe_dev_macsec_setting_save(struct rte_eth_dev *dev,
> >+struct ixgbe_macsec_setting *macsec_ctrl) {
> >+struct ixgbe_macsec_setting *macsec =
> >+IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data-
> >dev_private);
> >+
> >+macsec->encrypt_en = macsec_ctrl->encrypt_en;
> >+macsec->replayprotect_en = macsec_ctrl->replayprotect_en; }
> >+
> >+/* macsec ctrl setup disable */
> 
> ditto
Similar useless comments had been removed
> 
> >+void
> >+ixgbe_dev_macsec_setting_reset(struct rte_eth_dev *dev) {
> >+struct ixgbe_macsec_setting *macsec =
> >+IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data-
> >dev_private);
> >+
> >+macsec->encrypt_en = 0;
> >+macsec->replayprotect_en = 0;
> >+}
> >+
> >+/* macsec ctrl register set */
> >+static void
> >+ixgbe_dev_macsec_ctrl_register_enable(struct rte_eth_dev *dev,
> >+struct ixgbe_macsec_setting *macsec_contrl) {
> >+struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> >+uint32_t ctrl;
> >+uint8_t en = (uint8_t)macsec_contrl->encrypt_en;
> >+uint8_t rp = (uint8_t)macsec_contrl->replayprotect_en;
> >+
> >+/**
> >+ * Workaround:
> >+ * As no ixgbe_disable_sec_rx_path equivalent is
> >+ * implemented for tx in the base code, and we are
> >+ * not allowed to modify the base code in DPDK, so
> >+ * just call the hand-written one di

[dpdk-dev] [PATCH v4] net/ixgbe: fix macsec setting

2019-10-29 Thread Sun GuinanX
macsec setting is not valid when port is stopped.
In order to make it valid, the patch changes the setting
to where port is started.

Fixes: 597f9fafe13b ("app/testpmd: convert to new Tx offloads API")
Cc: sta...@dpdk.org

Signed-off-by: Sun GuinanX 
---
v4:
* Deleted extra comments
* Modified function name
* Increased the logic used to set the register in the port start state
v3:
* Deleted extra comments
* Modified the function name of the setting register
* Increased the logic used to set the register in the port start state
v2:
* Modified function name
* Refactored the rte_pmd_ixgbe_macsec_enable/disable function
* Modified structure name
* Modified the data type of a structure variable
---
 drivers/net/ixgbe/ixgbe_ethdev.c  | 149 ++
 drivers/net/ixgbe/ixgbe_ethdev.h  |  19 
 drivers/net/ixgbe/rte_pmd_ixgbe.c | 125 ++---
 3 files changed, 175 insertions(+), 118 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index dbce7a80e..d33b152b5 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2542,6 +2542,8 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
uint32_t *link_speeds;
struct ixgbe_tm_conf *tm_conf =
IXGBE_DEV_PRIVATE_TO_TM_CONF(dev->data->dev_private);
+   struct ixgbe_macsec_setting *macsec_ctrl =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data->dev_private);
 
PMD_INIT_FUNC_TRACE();
 
@@ -2794,6 +2796,9 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 */
ixgbe_dev_link_update(dev, 0);
 
+   /* setup the macsec ctrl register */
+   ixgbe_dev_macsec_register_set(dev, macsec_ctrl);
+
return 0;
 
 error:
@@ -2825,6 +2830,9 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
 
PMD_INIT_FUNC_TRACE();
 
+   /* disable mecsec register */
+   ixgbe_dev_macsec_register_reset(dev);
+
rte_eal_alarm_cancel(ixgbe_dev_setup_link_alarm_handler, dev);
 
/* disable interrupts */
@@ -8816,6 +8824,147 @@ ixgbe_clear_all_l2_tn_filter(struct rte_eth_dev *dev)
return 0;
 }
 
+void
+ixgbe_dev_macsec_setting_save(struct rte_eth_dev *dev,
+   struct ixgbe_macsec_setting *macsec_ctrl)
+{
+   struct ixgbe_macsec_setting *macsec =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data->dev_private);
+
+   macsec->encrypt_en = macsec_ctrl->encrypt_en;
+   macsec->replayprotect_en = macsec_ctrl->replayprotect_en;
+}
+
+void
+ixgbe_dev_macsec_setting_reset(struct rte_eth_dev *dev)
+{
+   struct ixgbe_macsec_setting *macsec =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data->dev_private);
+
+   macsec->encrypt_en = 0;
+   macsec->replayprotect_en = 0;
+}
+
+void
+ixgbe_dev_macsec_register_set(struct rte_eth_dev *dev,
+   struct ixgbe_macsec_setting *macsec_contrl)
+{
+   struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+   uint32_t ctrl;
+   uint8_t en = (uint8_t)macsec_contrl->encrypt_en;
+   uint8_t rp = (uint8_t)macsec_contrl->replayprotect_en;
+
+   /**
+* Workaround:
+* As no ixgbe_disable_sec_rx_path equivalent is
+* implemented for tx in the base code, and we are
+* not allowed to modify the base code in DPDK, so
+* just call the hand-written one directly for now.
+* The hardware support has been checked by
+* ixgbe_disable_sec_rx_path().
+*/
+   ixgbe_disable_sec_tx_path_generic(hw);
+
+   /* Enable Ethernet CRC (required by MACsec offload) */
+   ctrl = IXGBE_READ_REG(hw, IXGBE_HLREG0);
+   ctrl |= IXGBE_HLREG0_TXCRCEN | IXGBE_HLREG0_RXCRCSTRP;
+   IXGBE_WRITE_REG(hw, IXGBE_HLREG0, ctrl);
+
+   /* Enable the TX and RX crypto engines */
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
+   ctrl &= ~IXGBE_SECTXCTRL_SECTX_DIS;
+   IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, ctrl);
+
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
+   ctrl &= ~IXGBE_SECRXCTRL_SECRX_DIS;
+   IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, ctrl);
+
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
+   ctrl &= ~IXGBE_SECTX_MINSECIFG_MASK;
+   ctrl |= 0x3;
+   IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, ctrl);
+
+   /* Enable SA lookup */
+   ctrl = IXGBE_READ_REG(hw, IXGBE_LSECTXCTRL);
+   ctrl &= ~IXGBE_LSECTXCTRL_EN_MASK;
+   ctrl |= en ? IXGBE_LSECTXCTRL_AUTH_ENCRYPT :
+IXGBE_LSECTXCTRL_AUTH;
+   ctrl |= IXGBE_LSECTXCTRL_AISCI;
+   ctrl &= ~IXGBE_LSECTXCTRL_PNTHRSH_MASK;
+   ctrl |= IXGBE_MACSEC_PNTHRSH & IXGBE_LSECTXCTRL_PNTHRSH_MASK;
+   IXGBE_WRITE_REG(hw, IXGBE_LSECTXCTRL, ctrl);
+
+   ctrl = IXGBE_READ_REG(hw, IXGBE_LSECRXCTRL);
+   ctrl &= ~IXGBE_LSECRXCTRL_EN_MASK;
+   ctrl |= IXGBE_LSECRXCTRL

[dpdk-dev] [PATCH v5] net/ixgbe: fix macsec setting

2019-10-29 Thread Sun GuinanX
macsec setting is not valid when port is stopped.
In order to make it valid, the patch changes the setting
to where port is started.

Fixes: 597f9fafe13b ("app/testpmd: convert to new Tx offloads API")
Cc: sta...@dpdk.org

Signed-off-by: Sun GuinanX 
---
v5:
* modified function name
v4:
* Deleted extra comments
* Modified function name
* Increased the logic used to set the register in the port start state
v3:
* Deleted extra comments
* Modified the function name of the setting register
* Increased the logic used to set the register in the port start state
v2:
* Modified function name
* Refactored the rte_pmd_ixgbe_macsec_enable/disable function
* Modified structure name
* Modified the data type of a structure variable
---
 drivers/net/ixgbe/ixgbe_ethdev.c  | 149 ++
 drivers/net/ixgbe/ixgbe_ethdev.h  |  19 
 drivers/net/ixgbe/rte_pmd_ixgbe.c | 125 ++---
 3 files changed, 175 insertions(+), 118 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index dbce7a80e..16a904ed3 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2542,6 +2542,8 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
uint32_t *link_speeds;
struct ixgbe_tm_conf *tm_conf =
IXGBE_DEV_PRIVATE_TO_TM_CONF(dev->data->dev_private);
+   struct ixgbe_macsec_setting *macsec_ctrl =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data->dev_private);
 
PMD_INIT_FUNC_TRACE();
 
@@ -2794,6 +2796,9 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 */
ixgbe_dev_link_update(dev, 0);
 
+   /* setup the macsec ctrl register */
+   ixgbe_dev_macsec_register_enable(dev, macsec_ctrl);
+
return 0;
 
 error:
@@ -2825,6 +2830,9 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
 
PMD_INIT_FUNC_TRACE();
 
+   /* disable mecsec register */
+   ixgbe_dev_macsec_register_disable(dev);
+
rte_eal_alarm_cancel(ixgbe_dev_setup_link_alarm_handler, dev);
 
/* disable interrupts */
@@ -8816,6 +8824,147 @@ ixgbe_clear_all_l2_tn_filter(struct rte_eth_dev *dev)
return 0;
 }
 
+void
+ixgbe_dev_macsec_setting_save(struct rte_eth_dev *dev,
+   struct ixgbe_macsec_setting *macsec_ctrl)
+{
+   struct ixgbe_macsec_setting *macsec =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data->dev_private);
+
+   macsec->encrypt_en = macsec_ctrl->encrypt_en;
+   macsec->replayprotect_en = macsec_ctrl->replayprotect_en;
+}
+
+void
+ixgbe_dev_macsec_setting_reset(struct rte_eth_dev *dev)
+{
+   struct ixgbe_macsec_setting *macsec =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_CTRL(dev->data->dev_private);
+
+   macsec->encrypt_en = 0;
+   macsec->replayprotect_en = 0;
+}
+
+void
+ixgbe_dev_macsec_register_enable(struct rte_eth_dev *dev,
+   struct ixgbe_macsec_setting *macsec_contrl)
+{
+   struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+   uint32_t ctrl;
+   uint8_t en = (uint8_t)macsec_contrl->encrypt_en;
+   uint8_t rp = (uint8_t)macsec_contrl->replayprotect_en;
+
+   /**
+* Workaround:
+* As no ixgbe_disable_sec_rx_path equivalent is
+* implemented for tx in the base code, and we are
+* not allowed to modify the base code in DPDK, so
+* just call the hand-written one directly for now.
+* The hardware support has been checked by
+* ixgbe_disable_sec_rx_path().
+*/
+   ixgbe_disable_sec_tx_path_generic(hw);
+
+   /* Enable Ethernet CRC (required by MACsec offload) */
+   ctrl = IXGBE_READ_REG(hw, IXGBE_HLREG0);
+   ctrl |= IXGBE_HLREG0_TXCRCEN | IXGBE_HLREG0_RXCRCSTRP;
+   IXGBE_WRITE_REG(hw, IXGBE_HLREG0, ctrl);
+
+   /* Enable the TX and RX crypto engines */
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
+   ctrl &= ~IXGBE_SECTXCTRL_SECTX_DIS;
+   IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, ctrl);
+
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
+   ctrl &= ~IXGBE_SECRXCTRL_SECRX_DIS;
+   IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, ctrl);
+
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
+   ctrl &= ~IXGBE_SECTX_MINSECIFG_MASK;
+   ctrl |= 0x3;
+   IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, ctrl);
+
+   /* Enable SA lookup */
+   ctrl = IXGBE_READ_REG(hw, IXGBE_LSECTXCTRL);
+   ctrl &= ~IXGBE_LSECTXCTRL_EN_MASK;
+   ctrl |= en ? IXGBE_LSECTXCTRL_AUTH_ENCRYPT :
+IXGBE_LSECTXCTRL_AUTH;
+   ctrl |= IXGBE_LSECTXCTRL_AISCI;
+   ctrl &= ~IXGBE_LSECTXCTRL_PNTHRSH_MASK;
+   ctrl |= IXGBE_MACSEC_PNTHRSH & IXGBE_LSECTXCTRL_PNTHRSH_MASK;
+   IXGBE_WRITE_REG(hw, IXGBE_LSECTXCTRL, ctrl);
+
+   ctrl = IXGBE_READ_REG(hw, IXGBE_LSECRXCTRL);
+   ctrl &= ~IXGBE_LSECRXCTRL

[dpdk-dev] [PATCH v6] net/ixgbe: fix macsec setting

2019-10-30 Thread Sun GuinanX
macsec setting is not valid when port is stopped.
In order to make it valid, the patch changes the setting
to where port is started.

Fixes: 597f9fafe13b ("app/testpmd: convert to new Tx offloads API")
Cc: sta...@dpdk.org

Signed-off-by: Sun GuinanX 
---
v6:
* Modified inappropriate parameter naming
* Removed unnecessary type conversions
v5:
* modified function name
v4:
* Deleted extra comments
* Modified function name
* Increased the logic used to set the register in the port start state
v3:
* Deleted extra comments
* Modified the function name of the setting register
* Increased the logic used to set the register in the port start state
v2:
* Modified function name
* Refactored the rte_pmd_ixgbe_macsec_enable/disable function
* Modified structure name
* Modified the data type of a structure variable
---
 drivers/net/ixgbe/ixgbe_ethdev.c  | 149 ++
 drivers/net/ixgbe/ixgbe_ethdev.h  |  19 
 drivers/net/ixgbe/rte_pmd_ixgbe.c | 125 ++---
 3 files changed, 175 insertions(+), 118 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index dbce7a80e..d43e11bd4 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2542,6 +2542,8 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
uint32_t *link_speeds;
struct ixgbe_tm_conf *tm_conf =
IXGBE_DEV_PRIVATE_TO_TM_CONF(dev->data->dev_private);
+   struct ixgbe_macsec_setting *macsec_ctrl =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private);
 
PMD_INIT_FUNC_TRACE();
 
@@ -2794,6 +2796,9 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 */
ixgbe_dev_link_update(dev, 0);
 
+   /* setup the macsec ctrl register */
+   ixgbe_dev_macsec_register_enable(dev, macsec_ctrl);
+
return 0;
 
 error:
@@ -2825,6 +2830,9 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
 
PMD_INIT_FUNC_TRACE();
 
+   /* disable mecsec register */
+   ixgbe_dev_macsec_register_disable(dev);
+
rte_eal_alarm_cancel(ixgbe_dev_setup_link_alarm_handler, dev);
 
/* disable interrupts */
@@ -8816,6 +8824,147 @@ ixgbe_clear_all_l2_tn_filter(struct rte_eth_dev *dev)
return 0;
 }
 
+void
+ixgbe_dev_macsec_setting_save(struct rte_eth_dev *dev,
+   struct ixgbe_macsec_setting *macsec_setting)
+{
+   struct ixgbe_macsec_setting *macsec =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private);
+
+   macsec->encrypt_en = macsec_setting->encrypt_en;
+   macsec->replayprotect_en = macsec_setting->replayprotect_en;
+}
+
+void
+ixgbe_dev_macsec_setting_reset(struct rte_eth_dev *dev)
+{
+   struct ixgbe_macsec_setting *macsec =
+   IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private);
+
+   macsec->encrypt_en = 0;
+   macsec->replayprotect_en = 0;
+}
+
+void
+ixgbe_dev_macsec_register_enable(struct rte_eth_dev *dev,
+   struct ixgbe_macsec_setting *macsec_setting)
+{
+   struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+   uint32_t ctrl;
+   uint8_t en = macsec_setting->encrypt_en;
+   uint8_t rp = macsec_setting->replayprotect_en;
+
+   /**
+* Workaround:
+* As no ixgbe_disable_sec_rx_path equivalent is
+* implemented for tx in the base code, and we are
+* not allowed to modify the base code in DPDK, so
+* just call the hand-written one directly for now.
+* The hardware support has been checked by
+* ixgbe_disable_sec_rx_path().
+*/
+   ixgbe_disable_sec_tx_path_generic(hw);
+
+   /* Enable Ethernet CRC (required by MACsec offload) */
+   ctrl = IXGBE_READ_REG(hw, IXGBE_HLREG0);
+   ctrl |= IXGBE_HLREG0_TXCRCEN | IXGBE_HLREG0_RXCRCSTRP;
+   IXGBE_WRITE_REG(hw, IXGBE_HLREG0, ctrl);
+
+   /* Enable the TX and RX crypto engines */
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
+   ctrl &= ~IXGBE_SECTXCTRL_SECTX_DIS;
+   IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL, ctrl);
+
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
+   ctrl &= ~IXGBE_SECRXCTRL_SECRX_DIS;
+   IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, ctrl);
+
+   ctrl = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
+   ctrl &= ~IXGBE_SECTX_MINSECIFG_MASK;
+   ctrl |= 0x3;
+   IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, ctrl);
+
+   /* Enable SA lookup */
+   ctrl = IXGBE_READ_REG(hw, IXGBE_LSECTXCTRL);
+   ctrl &= ~IXGBE_LSECTXCTRL_EN_MASK;
+   ctrl |= en ? IXGBE_LSECTXCTRL_AUTH_ENCRYPT :
+IXGBE_LSECTXCTRL_AUTH;
+   ctrl |= IXGBE_LSECTXCTRL_AISCI;
+   ctrl &= ~IXGBE_LSECTXCTRL_PNTHRSH_MASK;
+   ctrl |= IXGBE_MACSEC_PNTHRSH & IXGBE_LSECTXCTRL_PNTHRSH_MASK;
+   IXGBE_WRITE_REG(hw, IXGBE_LSECTXCTRL, ctrl);
+
+   ctrl = IXGBE

Re: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address

2020-10-14 Thread Sun, GuinanX
Hi beilei

> -Original Message-
> From: Xing, Beilei 
> Sent: Thursday, October 15, 2020 1:37 PM
> To: Peng, Yuan ; Sun, GuinanX
> ; dev@dpdk.org
> Cc: Wu, Jingjing ; Zhang, Qi Z ;
> Sun, GuinanX 
> Subject: RE: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address
> 
> 
> 
> > -Original Message-
> > From: Peng, Yuan 
> > Sent: Thursday, October 15, 2020 1:14 PM
> > To: Sun, GuinanX ; dev@dpdk.org
> > Cc: Xing, Beilei ; Wu, Jingjing
> > ; Zhang, Qi Z ; Sun,
> > GuinanX 
> > Subject: RE: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC
> > address
> >
> > Test-by Peng, Yuan 
> >
> >
> > -Original Message-
> > From: dev  On Behalf Of Guinan Sun
> > Sent: Thursday, October 15, 2020 10:02 AM
> > To: dev@dpdk.org
> > Cc: Xing, Beilei ; Wu, Jingjing
> > ; Zhang, Qi Z ; Sun,
> > GuinanX 
> > Subject: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address
> >
> > When the multicast address is added, it will flush previous addresses
> > first, and then add new ones.
> > So when adding an address that exceeds the upper limit causes a
> > failure, you need to add the previous address list back. This patch fixes 
> > the
> issue.
> >
> > Fixes: 05e4c3aff35f ("net/iavf: support multicast configuration")
> >
> > Signed-off-by: Guinan Sun 
> > ---
> >  drivers/net/iavf/iavf_ethdev.c | 30 ++
> > drivers/net/iavf/iavf_vchnl.c  |  3 ---
> >  2 files changed, 22 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/iavf/iavf_ethdev.c
> > b/drivers/net/iavf/iavf_ethdev.c index e68e3bc71..042edadd9 100644
> > --- a/drivers/net/iavf/iavf_ethdev.c
> > +++ b/drivers/net/iavf/iavf_ethdev.c
> > @@ -164,7 +164,14 @@ iavf_set_mc_addr_list(struct rte_eth_dev *dev,
> > struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data-
> > >dev_private);
> >  struct iavf_adapter *adapter =
> >  IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> > -int err;
> > +int err, temp_err;
> > +
> > +if (mc_addrs_num > IAVF_NUM_MACADDR_MAX) { PMD_DRV_LOG(ERR,
> > +"can't add more than a limited number (%u) of
> > addresses.",
> > +(uint32_t)IAVF_NUM_MACADDR_MAX);
> > +return -EINVAL;
> > +}
> >
> >  /* flush previous addresses */
> >  err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs, vf-
> > >mc_addrs_num, @@ -172,17 +179,24 @@ iavf_set_mc_addr_list(struct
> > rte_eth_dev *dev,
> >  if (err)
> >  return err;
> >
> > -vf->mc_addrs_num = 0;
> > -
> >  /* add new ones */
> >  err = iavf_add_del_mc_addr_list(adapter, mc_addrs, mc_addrs_num,
> > true); -if (err) -return err;
> >
> > -vf->mc_addrs_num = mc_addrs_num;
> > -memcpy(vf->mc_addrs, mc_addrs, mc_addrs_num * sizeof(*mc_addrs));
> > +if (err) {
> > +/* When adding new addresses fail, need to add the
> > + * previous addresses back.
> > + */
> > +temp_err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs,
> > + vf->mc_addrs_num, true);
> 
> Can we reuse err here?

This err cannot be reused.

When performing mac address addition, the testpmd side will first add the mac 
address to mac_pool.
When the driver layer returns that it failed to add an address, the tespmd 
layer will delete the failed address from mac_pool.

If err is reused, the successful result of executing the add back address will 
overwrite the previous err, causing problems with the address stored in 
mac_pool on the testpmd side.

> 
> > +if (temp_err)
> > +return temp_err;
> > +} else {
> > +vf->mc_addrs_num = mc_addrs_num;
> > +memcpy(vf->mc_addrs,
> > +   mc_addrs, mc_addrs_num * sizeof(*mc_addrs)); }
> >
> > -return 0;
> > +return err;
> >  }
> >
> >  static int
> > diff --git a/drivers/net/iavf/iavf_vchnl.c
> > b/drivers/net/iavf/iavf_vchnl.c index
> > db0b76876..a2295f879 100644
> > --- a/drivers/net/iavf/iavf_vchnl.c
> > +++ b/drivers/net/iavf/iavf_vchnl.c
> > @@ -1107,9 +1107,6 @@ iavf_add_del_mc_addr_list(struct iavf_adapter
> > *adapter,  if (mc_addrs == NULL || mc_addrs_num == 0)  return 0;
> >
> > -if (mc_addrs_num > IAVF_NUM_MACADDR_MAX) -return -EINVAL;
> > -
> >  list = (struct virtchnl_ether_addr_list *)cmd_buffer;  list->vsi_id =
> > vf->vsi_res->vsi_id;  list->num_elements = mc_addrs_num;
> > --
> > 2.13.6
> 



Re: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address

2020-10-14 Thread Sun, GuinanX
Hi Beilei

> -Original Message-
> From: Xing, Beilei 
> Sent: Thursday, October 15, 2020 2:44 PM
> To: Sun, GuinanX ; Peng, Yuan
> ; dev@dpdk.org
> Cc: Wu, Jingjing ; Zhang, Qi Z 
> Subject: RE: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address
> 
> 
> 
> > -----Original Message-
> > From: Sun, GuinanX 
> > Sent: Thursday, October 15, 2020 1:57 PM
> > To: Xing, Beilei ; Peng, Yuan
> > ; dev@dpdk.org
> > Cc: Wu, Jingjing ; Zhang, Qi Z
> > 
> > Subject: RE: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC
> > address
> >
> > Hi beilei
> >
> > > -Original Message-----
> > > From: Xing, Beilei 
> > > Sent: Thursday, October 15, 2020 1:37 PM
> > > To: Peng, Yuan ; Sun, GuinanX
> > > ; dev@dpdk.org
> > > Cc: Wu, Jingjing ; Zhang, Qi Z
> > > ; Sun, GuinanX 
> > > Subject: RE: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC
> > > address
> > >
> > >
> > >
> > > > -Original Message-
> > > > From: Peng, Yuan 
> > > > Sent: Thursday, October 15, 2020 1:14 PM
> > > > To: Sun, GuinanX ; dev@dpdk.org
> > > > Cc: Xing, Beilei ; Wu, Jingjing
> > > > ; Zhang, Qi Z ; Sun,
> > > > GuinanX 
> > > > Subject: RE: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC
> > > > address
> > > >
> > > > Test-by Peng, Yuan 
> > > >
> > > >
> > > > -Original Message-
> > > > From: dev  On Behalf Of Guinan Sun
> > > > Sent: Thursday, October 15, 2020 10:02 AM
> > > > To: dev@dpdk.org
> > > > Cc: Xing, Beilei ; Wu, Jingjing
> > > > ; Zhang, Qi Z ; Sun,
> > > > GuinanX 
> > > > Subject: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC
> > > > address
> > > >
> > > > When the multicast address is added, it will flush previous
> > > > addresses first, and then add new ones.
> > > > So when adding an address that exceeds the upper limit causes a
> > > > failure, you need to add the previous address list back. This
> > > > patch fixes the
> > > issue.
> > > >
> > > > Fixes: 05e4c3aff35f ("net/iavf: support multicast configuration")
> > > >
> > > > Signed-off-by: Guinan Sun 
> > > > ---
> > > >  drivers/net/iavf/iavf_ethdev.c | 30
> > > > ++ drivers/net/iavf/iavf_vchnl.c  |  3
> > > > ---
> > > >  2 files changed, 22 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/drivers/net/iavf/iavf_ethdev.c
> > > > b/drivers/net/iavf/iavf_ethdev.c index e68e3bc71..042edadd9 100644
> > > > --- a/drivers/net/iavf/iavf_ethdev.c
> > > > +++ b/drivers/net/iavf/iavf_ethdev.c
> > > > @@ -164,7 +164,14 @@ iavf_set_mc_addr_list(struct rte_eth_dev
> > > > *dev, struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data-
> > > > >dev_private);
> > > >  struct iavf_adapter *adapter =
> > > >  IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> > > > -int err;
> > > > +int err, temp_err;
> > > > +
> > > > +if (mc_addrs_num > IAVF_NUM_MACADDR_MAX) { PMD_DRV_LOG(ERR,
> > > > +"can't add more than a limited number (%u) of
> > > > addresses.",
> > > > +(uint32_t)IAVF_NUM_MACADDR_MAX); return -EINVAL; }
> > > >
> > > >  /* flush previous addresses */
> > > >  err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs, vf-
> > > > >mc_addrs_num, @@ -172,17 +179,24 @@ iavf_set_mc_addr_list(struct
> > > > rte_eth_dev *dev,
> > > >  if (err)
> > > >  return err;
> > > >
> > > > -vf->mc_addrs_num = 0;
> > > > -
> > > >  /* add new ones */
> > > >  err = iavf_add_del_mc_addr_list(adapter, mc_addrs, mc_addrs_num,
> > > > true); -if (err) -return err;
> > > >
> > > > -vf->mc_addrs_num = mc_addrs_num;
> > > > -memcpy(vf->mc_addrs, mc_addrs, mc_addrs_num * sizeof(*mc_addrs));
> > > > +if (err) {
> > > > +/* When adding new addresses fail, need to add the
> > > > + * previous addresses back.
> > > > + */
> > > > +temp_err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs,
> > > > + vf->mc_addrs_num, true);
&

Re: [dpdk-dev] [PATCH 5/8] net/i40e/base: check return value of DNL admin command

2020-07-26 Thread Sun, GuinanX
Hi Jeff

> -Original Message-
> From: Guo, Jia
> Sent: Monday, July 27, 2020 12:08 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Xing, Beilei ; Ludkiewicz, Adam
> 
> Subject: Re: [PATCH 5/8] net/i40e/base: check return value of DNL admin
> command
> 
> hi, guinan
> 
> On 7/21/2020 3:39 PM, Guinan Sun wrote:
> > Check return value of running DNL admin command.
> 
> 
> What is "DNL", I am not sure i know the reason it appear here, could you
> double check it?
> 

Regarding what DNL stands for, I haven't found it in the code and datasheet.
It can be seen from the code change that the return value check operation of 
i40e_aq_run_phy_activity has been added.
The description of this function is RUN DNL admin command.
So tile and commit log have made such changes.

> 
> >
> > Signed-off-by: Adam Ludkiewicz 
> > Signed-off-by: Guinan Sun 
> > ---
> >   drivers/net/i40e/base/i40e_common.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/i40e/base/i40e_common.c
> b/drivers/net/i40e/base/i40e_common.c
> > index 46a0b7881..85c22849e 100644
> > --- a/drivers/net/i40e/base/i40e_common.c
> > +++ b/drivers/net/i40e/base/i40e_common.c
> > @@ -7097,7 +7097,7 @@ enum i40e_status_code
> i40e_get_lpi_counters(struct i40e_hw *hw,
> >
>   I40E_AQ_RUN_PHY_ACT_DNL_OPCODE_GET_EEE_STAT,
> > &cmd_status, tx_counter, rx_counter, NULL);
> >
> > -   if (cmd_status != I40E_AQ_RUN_PHY_ACT_CMD_STAT_SUCC)
> > +   if (!retval && cmd_status !=
> I40E_AQ_RUN_PHY_ACT_CMD_STAT_SUCC)
> > retval = I40E_ERR_ADMIN_QUEUE_ERROR;
> >
> > return retval;


Re: [dpdk-dev] [PATCH 3/8] net/i40e/base: enable new custom cloud filters

2020-07-26 Thread Sun, GuinanX
Hi Jeff

> -Original Message-
> From: Guo, Jia
> Sent: Monday, July 27, 2020 11:53 AM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Xing, Beilei ; Ramamurthy, Harshitha
> 
> Subject: Re: [PATCH 3/8] net/i40e/base: enable new custom cloud filters
> 
> hi, guinan
> 
> This patch is just define some new types but not enable a feature, could the
> title to be "add new custom cloud filters types",
> 
> is it better and could eliminate some confuse?

I agree with you.
Patch V2 will modify it.

> 
> On 7/21/2020 3:39 PM, Guinan Sun wrote:
> > This patch adds the new filter types needed for custom cloud filters.
> > These custom cloud filters will route traffic to VFs based on the dst
> > IP for both tunneled and non-tunneled packets.
> >
> > Signed-off-by: Harshitha Ramamurthy 
> > Signed-off-by: Guinan Sun 
> > ---
> >   drivers/net/i40e/base/i40e_adminq_cmd.h | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/i40e/base/i40e_adminq_cmd.h
> > b/drivers/net/i40e/base/i40e_adminq_cmd.h
> > index f790183be..21f544840 100644
> > --- a/drivers/net/i40e/base/i40e_adminq_cmd.h
> > +++ b/drivers/net/i40e/base/i40e_adminq_cmd.h
> > @@ -1406,6 +1406,8 @@ struct i40e_aqc_cloud_filters_element_data {
> >   #define I40E_AQC_ADD_CLOUD_FILTER_IMAC
>   0x000A
> >   #define I40E_AQC_ADD_CLOUD_FILTER_OMAC_TEN_ID_IMAC
>   0x000B
> >   #define I40E_AQC_ADD_CLOUD_FILTER_IIP 0x000C
> > +#define I40E_AQC_ADD_CLOUD_FILTER_OIP1
>   0x0010
> > +#define I40E_AQC_ADD_CLOUD_FILTER_OIP2
>   0x0012
> >   /* 0x000D reserved */
> >   /* 0x000E reserved */
> >   /* 0x000F reserved */


Re: [dpdk-dev] [PATCH 6/8] net/i40e/base: add disable unused ports capability

2020-07-26 Thread Sun, GuinanX
Hi Jeff

> -Original Message-
> From: Guo, Jia
> Sent: Monday, July 27, 2020 12:25 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Xing, Beilei ; Milosek, Damian
> 
> Subject: Re: [PATCH 6/8] net/i40e/base: add disable unused ports capability
> 
> hi, guinan
> 
> On 7/21/2020 3:39 PM, Guinan Sun wrote:
> > This patch adds support for "Disable Unused Ports" functionality.
> 
> 
> "This patch adds support for disabling unused ports" is enough for reader i 
> think.
> 
> And use 2 verb is not good in title, "Support unused ports disabling" or other
> you want.
> 

I think you are right.
Patch V2 will modify it.

> 
> >
> > Signed-off-by: Damian Milosek 
> > Signed-off-by: Guinan Sun 
> > ---
> >   drivers/net/i40e/base/i40e_adminq_cmd.h | 1 +
> >   drivers/net/i40e/base/i40e_common.c | 6 ++
> >   drivers/net/i40e/base/i40e_type.h   | 1 +
> >   3 files changed, 8 insertions(+)
> >
> > diff --git a/drivers/net/i40e/base/i40e_adminq_cmd.h
> b/drivers/net/i40e/base/i40e_adminq_cmd.h
> > index e1018ed49..c7686b0d3 100644
> > --- a/drivers/net/i40e/base/i40e_adminq_cmd.h
> > +++ b/drivers/net/i40e/base/i40e_adminq_cmd.h
> > @@ -440,6 +440,7 @@ struct i40e_aqc_list_capabilities_element_resp {
> >   #define I40E_AQ_CAP_ID_SDP0x0062
> >   #define I40E_AQ_CAP_ID_MDIO   0x0063
> >   #define I40E_AQ_CAP_ID_WSR_PROT   0x0064
> > +#define I40E_AQ_CAP_ID_DIS_UNUSED_PORTS0x0067
> 
> 
> Please double check if it is "0x0066" or "0x0067", if it is conform,
> please add my ACK for this patch at next version.
> 
> 
> >   #define I40E_AQ_CAP_ID_NVM_MGMT   0x0080
> >   #define I40E_AQ_CAP_ID_FLEX10 0x00F1
> >   #define I40E_AQ_CAP_ID_CEM0x00F2
> > diff --git a/drivers/net/i40e/base/i40e_common.c
> b/drivers/net/i40e/base/i40e_common.c
> > index 85c22849e..65317f6c6 100644
> > --- a/drivers/net/i40e/base/i40e_common.c
> > +++ b/drivers/net/i40e/base/i40e_common.c
> > @@ -4021,6 +4021,12 @@ STATIC void
> i40e_parse_discover_capabilities(struct i40e_hw *hw, void *buff,
> >"HW Capability: wr_csr_prot = 0x%llX\n\n",
> >(p->wr_csr_prot & 0x));
> > break;
> > +   case I40E_AQ_CAP_ID_DIS_UNUSED_PORTS:
> > +   p->dis_unused_ports = (bool)number;
> > +   i40e_debug(hw, I40E_DEBUG_INIT,
> > +  "HW Capability: dis_unused_ports = %d\n\n",
> > +  p->dis_unused_ports);
> > +   break;
> > case I40E_AQ_CAP_ID_NVM_MGMT:
> > if (number & I40E_NVM_MGMT_SEC_REV_DISABLED)
> > p->sec_rev_disabled = true;
> > diff --git a/drivers/net/i40e/base/i40e_type.h
> b/drivers/net/i40e/base/i40e_type.h
> > index 0eeb55975..cf4134583 100644
> > --- a/drivers/net/i40e/base/i40e_type.h
> > +++ b/drivers/net/i40e/base/i40e_type.h
> > @@ -425,6 +425,7 @@ struct i40e_hw_capabilities {
> > u32 enabled_tcmap;
> > u32 maxtc;
> > u64 wr_csr_prot;
> > +   bool dis_unused_ports;
> > bool apm_wol_support;
> > enum i40e_acpi_programming_method acpi_prog_method;
> > bool proxy_support;


Re: [dpdk-dev] [PATCH 5/8] net/i40e/base: check return value of DNL admin command

2020-07-26 Thread Sun, GuinanX
Hi, Jeff

> -Original Message-
> From: Sun, GuinanX
> Sent: Monday, July 27, 2020 12:54 PM
> To: Guo, Jia ; dev@dpdk.org
> Cc: Xing, Beilei ; Ludkiewicz, Adam
> 
> Subject: RE: [PATCH 5/8] net/i40e/base: check return value of DNL admin
> command
> 
> Hi Jeff
> 
> > -Original Message-
> > From: Guo, Jia
> > Sent: Monday, July 27, 2020 12:08 PM
> > To: Sun, GuinanX ; dev@dpdk.org
> > Cc: Xing, Beilei ; Ludkiewicz, Adam
> > 
> > Subject: Re: [PATCH 5/8] net/i40e/base: check return value of DNL
> > admin command
> >
> > hi, guinan
> >
> > On 7/21/2020 3:39 PM, Guinan Sun wrote:
> > > Check return value of running DNL admin command.
> >
> >
> > What is "DNL", I am not sure i know the reason it appear here, could
> > you double check it?
> >
> 
> Regarding what DNL stands for, I haven't found it in the code and datasheet.
> It can be seen from the code change that the return value check operation of
> i40e_aq_run_phy_activity has been added.
> The description of this function is RUN DNL admin command.
> So tile and commit log have made such changes.

Sorry, I might have misunderstood it just now.
I will double check for it and patch V2 will modify it.

> 
> >
> > >
> > > Signed-off-by: Adam Ludkiewicz 
> > > Signed-off-by: Guinan Sun 
> > > ---
> > >   drivers/net/i40e/base/i40e_common.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/i40e/base/i40e_common.c
> > b/drivers/net/i40e/base/i40e_common.c
> > > index 46a0b7881..85c22849e 100644
> > > --- a/drivers/net/i40e/base/i40e_common.c
> > > +++ b/drivers/net/i40e/base/i40e_common.c
> > > @@ -7097,7 +7097,7 @@ enum i40e_status_code
> > i40e_get_lpi_counters(struct i40e_hw *hw,
> > >
> > I40E_AQ_RUN_PHY_ACT_DNL_OPCODE_GET_EEE_STAT,
> > >   &cmd_status, tx_counter, rx_counter, 
> > > NULL);
> > >
> > > - if (cmd_status != I40E_AQ_RUN_PHY_ACT_CMD_STAT_SUCC)
> > > + if (!retval && cmd_status !=
> > I40E_AQ_RUN_PHY_ACT_CMD_STAT_SUCC)
> > >   retval = I40E_ERR_ADMIN_QUEUE_ERROR;
> > >
> > >   return retval;


Re: [dpdk-dev] [PATCH] net/i40e: fix link status

2020-07-30 Thread Sun, GuinanX
Hi Jeff

> -Original Message-
> From: Guo, Jia
> Sent: Thursday, July 30, 2020 5:12 PM
> To: Sun, GuinanX ; Xing, Beilei
> ; dev@dpdk.org
> Cc: sta...@dpdk.org
> Subject: Re: [PATCH] net/i40e: fix link status
> 
> hi, guinan
> 
> On 7/30/2020 4:25 PM, Guinan Sun wrote:
> > The PF driver supports to use link_event_adv or link_event to get the
> > link speed. However, when using link_event_adv to get link speed,
> > there is a lack of logic to convert between link speed type
> > (I40E_LINK_SPEED_XXX) and mbps type (SPEED_XXX).
> > As a result, the mbps type is not recognized, so the link status down
> > problem occurs. This patch is used for type replacement between speed
> > types.
> >
> > Fixes: 2a73125b7041 ("i40evf: fix link info update")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Guinan Sun 
> > ---
> >   drivers/net/i40e/base/virtchnl.h  |  8 +-
> >   drivers/net/i40e/i40e_ethdev_vf.c | 42 +-
> -
> >   drivers/net/i40e/i40e_pf.c|  4 +++
> >   3 files changed, 51 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/i40e/base/virtchnl.h
> > b/drivers/net/i40e/base/virtchnl.h
> > index 4f498ca45..90c404fb8 100644
> > --- a/drivers/net/i40e/base/virtchnl.h
> > +++ b/drivers/net/i40e/base/virtchnl.h
> > @@ -240,7 +240,8 @@ VIRTCHNL_CHECK_STRUCT_LEN(16,
> virtchnl_vsi_resource);
> >   #define VIRTCHNL_VF_OFFLOAD_ENCAP 0X0010
> >   #define VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM0X0020
> >   #define VIRTCHNL_VF_OFFLOAD_RX_ENCAP_CSUM 0X0040
> > -
> > +/* Define below the capability flags that are not offloads */
> > +#define VIRTCHNL_VF_CAP_ADV_LINK_SPEED 0x0080
> >   #define VF_BASE_MODE_OFFLOADS (VIRTCHNL_VF_OFFLOAD_L2 | \
> >VIRTCHNL_VF_OFFLOAD_VLAN | \
> >VIRTCHNL_VF_OFFLOAD_RSS_PF) @@ -540,6
> +541,11 @@ struct
> > virtchnl_pf_event {
> > enum virtchnl_link_speed link_speed;
> > bool link_status;
> > } link_event;
> > +   struct {
> > +   /* link_speed provided in Mbps */
> > +   u32 link_speed;
> > +   u8 link_status;
> > +   } link_event_adv;
> > } event_data;
> >
> > int severity;
> > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > b/drivers/net/i40e/i40e_ethdev_vf.c
> > index 69cab8e73..f8cf45fbe 100644
> > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > @@ -1386,8 +1386,46 @@ i40evf_handle_pf_event(struct rte_eth_dev *dev,
> uint8_t *msg,
> > break;
> > case VIRTCHNL_EVENT_LINK_CHANGE:
> > PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE
> event");
> > -   vf->link_up = pf_msg->event_data.link_event.link_status;
> > -   vf->link_speed = pf_msg->event_data.link_event.link_speed;
> > +
> > +   if (vf->vf_res->vf_cap_flags &
> VIRTCHNL_VF_CAP_ADV_LINK_SPEED) {
> > +   vf->link_up =
> > +   pf_msg-
> >event_data.link_event_adv.link_status;
> > +
> > +   switch (pf_msg-
> >event_data.link_event_adv.link_speed) {
> > +   case ETH_SPEED_NUM_100M:
> > +   vf->link_speed = I40E_LINK_SPEED_100MB;
> > +   break;
> > +   case ETH_SPEED_NUM_1G:
> > +   vf->link_speed = I40E_LINK_SPEED_1GB;
> > +   break;
> > +   case ETH_SPEED_NUM_2_5G:
> > +   vf->link_speed = I40E_LINK_SPEED_2_5GB;
> > +   break;
> > +   case ETH_SPEED_NUM_5G:
> > +   vf->link_speed = I40E_LINK_SPEED_5GB;
> > +   break;
> > +   case ETH_SPEED_NUM_10G:
> > +   vf->link_speed = I40E_LINK_SPEED_10GB;
> > +   break;
> > +   case ETH_SPEED_NUM_20G:
> > +   vf->link_speed = I40E_LINK_SPEED_20GB;
> > +   break;
> > +   case ETH_SPEED_NUM_25G:
> > +   vf->link_speed = I40E_LINK_SPEED_25GB;
> > +   break;
> > +   case ETH_SPEED_NUM_40G:
> >

Re: [dpdk-dev] [PATCH v2] net/i40e: fix link status

2020-07-30 Thread Sun, GuinanX
Hi Beilei

> -Original Message-
> From: Xing, Beilei
> Sent: Friday, July 31, 2020 11:50 AM
> To: Wang, ShougangX ; Sun, GuinanX
> ; Guo, Jia ; dev@dpdk.org
> Cc: Sun, GuinanX ; sta...@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix link status
> 
> 
> 
> > -Original Message-
> > From: Wang, ShougangX 
> > Sent: Friday, July 31, 2020 10:38 AM
> > To: Xing, Beilei ; Sun, GuinanX
> > ; Guo, Jia ; dev@dpdk.org
> > Cc: Sun, GuinanX ; sta...@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix link status
> >
> > Hi Beilei
> >
> > > -Original Message-----
> > > From: dev  On Behalf Of Xing, Beilei
> > > Sent: Friday, July 31, 2020 10:30 AM
> > > To: Sun, GuinanX ; Guo, Jia
> > > ; dev@dpdk.org
> > > Cc: Sun, GuinanX ; sta...@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: fix link status
> > >
> > >
> > >
> > > > -Original Message-
> > > > From: dev  On Behalf Of Guinan Sun
> > > > Sent: Thursday, July 30, 2020 6:25 PM
> > > > To: Xing, Beilei ; Guo, Jia
> > > > ; dev@dpdk.org
> > > > Cc: Sun, GuinanX ; sta...@dpdk.org
> > > > Subject: [dpdk-dev] [PATCH v2] net/i40e: fix link status
> > > >
> > > > If the PF driver supports the new speed reporting capabilities
> > > > then use link_event_adv instead of link_event to get the speed.
> > > >
> > > > Fixes: 2a73125b7041 ("i40evf: fix link info update")
> > > > Cc: sta...@dpdk.org
> > > >
> > > > Signed-off-by: Guinan Sun 
> > > > ---
> > > > v2:
> > > > * Modify commit log.
> > > > * Add code comments.
> > > > * Delete useless codes.
> > > > ---
> > > >  drivers/net/i40e/base/virtchnl.h  | 16 +++-
> > > > drivers/net/i40e/i40e_ethdev_vf.c | 42
> > > +--
> > > >  2 files changed, 55 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/net/i40e/base/virtchnl.h
> > > > b/drivers/net/i40e/base/virtchnl.h
> > > > index 4f498ca45..9c64fd469 100644
> > > > --- a/drivers/net/i40e/base/virtchnl.h
> > > > +++ b/drivers/net/i40e/base/virtchnl.h
> > > > @@ -240,7 +240,8 @@ VIRTCHNL_CHECK_STRUCT_LEN(16,
> > > > virtchnl_vsi_resource);  #define
> > > > VIRTCHNL_VF_OFFLOAD_ENCAP0X0010
> > > >  #define VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM0X0020
> > > >  #define VIRTCHNL_VF_OFFLOAD_RX_ENCAP_CSUM0X0040
> > > > -
> > > > +/* Define below the capability flags that are not offloads */
> > > > +#define VIRTCHNL_VF_CAP_ADV_LINK_SPEED0x0080
> > >
> > > You defined the new capability VIRTCHNL_VF_CAP_ADV_LINK_SPEED in the
> > > patch, but didn't request the capability for i40evf?
> >
> > This capability is given by i40e kernel pf if
> > VIRTCHNL_VF_CAP_ADV_LINK_SPEED is defined. So vf doesn't need to
> > request it.
> 
> All the capabilities for VF is required by VF, please check
> i40evf_get_vf_resource.

I will double check for it.
New Patch will fix it.
Thank you.

> 
> >
> > Thanks.
> > Shougang



Re: [dpdk-dev] [PATCH] net/ice: fix flow validation for unsupported patterns

2020-09-01 Thread Sun, GuinanX
Hi qi

> -Original Message-
> From: Zhang, Qi Z
> Sent: Monday, August 31, 2020 12:23 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Yang, Qiming ; sta...@dpdk.org
> Subject: RE: [PATCH] net/ice: fix flow validation for unsupported patterns
> 
> 
> 
> > -Original Message-
> > From: Sun, GuinanX 
> > Sent: Friday, August 28, 2020 10:33 AM
> > To: dev@dpdk.org
> > Cc: Zhang, Qi Z ; Yang, Qiming
> > ; Sun, GuinanX ;
> > sta...@dpdk.org
> > Subject: [PATCH] net/ice: fix flow validation for unsupported patterns
> >
> > When loading the OS default package and the pipeline mode is enabled
> > by the "pipeline-mode-support=1" operation. In this case, the wrong
> > parser is selected for processing and it will cause the unsupported
> > patterns(pppoes/pfcp/l2tpv3/esp/ah) to be validated successfully.
> > This patch corrects the parser selection issue.
> >
> > Fixes: 47d460d63233 ("net/ice: rework switch filter")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Guinan Sun 
> > ---
> >  drivers/net/ice/ice_switch_filter.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ice/ice_switch_filter.c
> > b/drivers/net/ice/ice_switch_filter.c
> > index c4b00b6a2..884fbaae2 100644
> > --- a/drivers/net/ice/ice_switch_filter.c
> > +++ b/drivers/net/ice/ice_switch_filter.c
> > @@ -1806,7 +1806,8 @@ ice_switch_init(struct ice_adapter *ad)
> > else
> > return -EINVAL;
> >
> > -   if (ad->devargs.pipe_mode_support)
> > +   if (ad->devargs.pipe_mode_support &&
> > +   ad->active_pkg_type != ICE_PKG_TYPE_OS_DEFAULT)
> > ret = ice_register_parser(perm_parser, ad);
> 
> This is not correct, package type should not related with pipe line mode.

Under the premise of pipe-mode support, the choice of parser for different 
package types will be added.
Patch v2 will fix it.

> 
> 
> > else
> > ret = ice_register_parser(dist_parser, ad); @@ -1824,7
> +1825,8 @@
> > ice_switch_uninit(struct ice_adapter *ad)
> > else
> > dist_parser = &ice_switch_dist_parser_os;
> >
> > -   if (ad->devargs.pipe_mode_support)
> > +   if (ad->devargs.pipe_mode_support &&
> > +   ad->active_pkg_type != ICE_PKG_TYPE_OS_DEFAULT)
> > ice_unregister_parser(perm_parser, ad);
> > else
> > ice_unregister_parser(dist_parser, ad);
> > --
> > 2.17.1



Re: [dpdk-dev] [dpdk-stable] [PATCH v3] net/i40e: fix link status

2020-09-01 Thread Sun, GuinanX
Hi Ferruh

> -Original Message-
> From: Ferruh Yigit [mailto:ferruh.yi...@intel.com]
> Sent: Monday, August 31, 2020 9:24 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Xing, Beilei ; Guo, Jia ;
> sta...@dpdk.org
> Subject: Re: [dpdk-stable] [PATCH v3] net/i40e: fix link status
> 
> On 8/6/2020 9:16 AM, Guinan Sun wrote:
> > If the PF driver supports the new speed reporting capabilities then
> > use link_event_adv instead of link_event to get the speed.
> >
> > Fixes: 2a73125b7041 ("i40evf: fix link info update")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Guinan Sun 
> > ---
> > v3:
> > * request the capability for i40evf
> > v2:
> > * modify commit log
> > * add code comments
> > * delete useless code
> > ---
> >  drivers/net/i40e/base/virtchnl.h  | 16 ++-
> > drivers/net/i40e/i40e_ethdev_vf.c | 45 ---
> >  2 files changed, 57 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/i40e/base/virtchnl.h
> > b/drivers/net/i40e/base/virtchnl.h
> > index 4f498ca45..9c64fd469 100644
> > --- a/drivers/net/i40e/base/virtchnl.h
> > +++ b/drivers/net/i40e/base/virtchnl.h
> > @@ -240,7 +240,8 @@ VIRTCHNL_CHECK_STRUCT_LEN(16,
> virtchnl_vsi_resource);
> >  #define VIRTCHNL_VF_OFFLOAD_ENCAP  0X0010
> >  #define VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM 0X0020
> >  #define VIRTCHNL_VF_OFFLOAD_RX_ENCAP_CSUM  0X0040
> > -
> > +/* Define below the capability flags that are not offloads */
> > +#define VIRTCHNL_VF_CAP_ADV_LINK_SPEED 0x0080
> >  #define VF_BASE_MODE_OFFLOADS (VIRTCHNL_VF_OFFLOAD_L2 | \
> >VIRTCHNL_VF_OFFLOAD_VLAN | \
> >VIRTCHNL_VF_OFFLOAD_RSS_PF) @@ -536,10
> +537,23 @@ enum
> > virtchnl_event_codes {  struct virtchnl_pf_event {
> > enum virtchnl_event_codes event;
> > union {
> > +   /* If the PF driver does not support the new speed reporting
> > +* capabilities then use link_event else use link_event_adv to
> > +* get the speed and link information. The ability to understand
> > +* new speeds is indicated by setting the capability flag
> > +* VIRTCHNL_VF_CAP_ADV_LINK_SPEED in vf_cap_flags
> parameter
> > +* in virtchnl_vf_resource struct and can be used to determine
> > +* which link event struct to use below.
> > +*/
> > struct {
> > enum virtchnl_link_speed link_speed;
> > bool link_status;
> > } link_event;
> > +   struct {
> > +   /* link_speed provided in Mbps */
> > +   u32 link_speed;
> > +   u8 link_status;
> > +   } link_event_adv;
> > } event_data;
> >
> > int severity;
> > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > b/drivers/net/i40e/i40e_ethdev_vf.c
> > index 69cab8e73..ccf5d8c57 100644
> > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > @@ -469,7 +469,8 @@ i40evf_get_vf_resource(struct rte_eth_dev *dev)
> >VIRTCHNL_VF_OFFLOAD_RSS_AQ |
> >VIRTCHNL_VF_OFFLOAD_RSS_REG |
> >VIRTCHNL_VF_OFFLOAD_VLAN |
> > -  VIRTCHNL_VF_OFFLOAD_RX_POLLING;
> > +  VIRTCHNL_VF_OFFLOAD_RX_POLLING |
> > +  VIRTCHNL_VF_CAP_ADV_LINK_SPEED;
> > args.in_args = (uint8_t *)∩︀
> > args.in_args_size = sizeof(caps);
> > } else {
> > @@ -1386,8 +1387,46 @@ i40evf_handle_pf_event(struct rte_eth_dev *dev,
> uint8_t *msg,
> > break;
> > case VIRTCHNL_EVENT_LINK_CHANGE:
> > PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE
> event");
> > -   vf->link_up = pf_msg->event_data.link_event.link_status;
> > -   vf->link_speed = pf_msg->event_data.link_event.link_speed;
> > +
> > +   if (vf->vf_res->vf_cap_flags &
> VIRTCHNL_VF_CAP_ADV_LINK_SPEED) {
> > +   vf->link_up =
> > +   pf_msg-
> >event_data.link_event_adv.link_status;
> > +
> > +   switch (pf_msg-
> >event_data.link_event_adv.link_speed) {
> > +   case ETH_SPEED_NUM_100M:
> > +   vf->link_speed = I40E_LINK_SPEED_100MB;
> > +   break;
&

Re: [dpdk-dev] [PATCH v3 00/15] update i40e base code

2020-09-03 Thread Sun, GuinanX
Hi Ferruh

> -Original Message-
> From: Ferruh Yigit [mailto:ferruh.yi...@intel.com]
> Sent: Thursday, September 3, 2020 10:31 PM
> To: Sun, GuinanX ; dev@dpdk.org
> Cc: Xing, Beilei ; Guo, Jia 
> Subject: Re: [dpdk-dev] [PATCH v3 00/15] update i40e base code
> 
> On 9/3/2020 5:48 AM, Guinan Sun wrote:
> > update i40e base code.
> >
> > source code of i40e driver:
> > cid-i40e.2020.08.27.tar.gz dropped by the team which develop basic
> > drivers for any i40e NIC.
> >
> > changelog in ND share repo:
> > From fc99a1143d3f ("i40e-shared: FEC on/off support flag for X722") To
> > 1a82d59f0797 ("i40e-shared: Fix PHY configuration parameters when
> > enabling EEE")
> 
> Hi Guinan,
> 
> The v2 of this patch has been merged to next-net-intel and pulled by next-
> net, it is now in the next-net repo.
> 
> I can see v3 is slightly newer version, I didn't check the details but does it
> can you make updates as incremental set on top of latest head (v2 of this
> set)?

I have synced with zhangqi.
The patch status has been changed to superseded.
The share code of the new drop will send a new version.

> 
> 
> 
> >
> > The following commits are ignored.
> > ea2cd04e5db1 ("i40e-shared: do AQ calls without sleeping by default")
> > 4eec86f12992 ("i40e-shared: Fix undeclared i40e_asq_send_command()
> > function")
> > 9d60cec99eb7 ("i40e-shared: Send rx ctl write command in atomic
> > context") a204afdc1cad ("i40e-shared: Send uc/mc commands in atomic
> > context")
> > d22f8cb2a111 ("i40e-shared: Send commands in atomic context")
> > 136a7d931a45 ("i40e-shared: Fix improper preprocessor conditional")
> > 5b7d5a698092 ("i40e-shared: use linux packing style")
> > f16fa495c503 ("i40e-shared: Fix compilation issue with __packed")
> >
> > Guinan Sun (15):
> >   net/i40e/base: enable FEC on/off flag setting for X722
> >   net/i40e/base: add PTYPE definition
> >   net/i40e/base: add new custom cloud filter types
> >   net/i40e/base: update FW API version to 1.12
> >   net/i40e/base: fix possible uninitialized variable
> >   net/i40e/base: support unused ports disabling
> >   net/i40e/base: replace AQ command for NVM update
> >   net/i40e/base: add VLAN field for input set
> >   net/i40e/base: enable pipe monitor thresholds
> >   net/i40e/base: fix missing function header arguments
> >   net/i40e/base: add support for Minimum Rollback Revision
> >   net/i40e/base: fix Rx only mode for unicast promisc on VLAN
> >   net/i40e/base: add EEE LPI status check for X722 adapters
> >   net/i40e/base: fix PHY config param when enabling EEE
> >   net/i40e/base: update version
> 
> <...>