diff mbox series

[v3] net/i40e: disable source pruning

Message ID 20211020012831.8480-1-alvinx.zhang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers show
Series [v3] net/i40e: disable source pruning | expand

Checks

Context Check Description
ci/intel-Testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-spell-check-testing warning Testing issues
ci/checkpatch success coding style OK

Commit Message

Alvin Zhang Oct. 20, 2021, 1:28 a.m. UTC
VRRP advertisement packets are dropped on i40e PF devices because
when a MAC address is added to a device, packets originating from
that MAC address are dropped.

This patch adds a devarg to support disabling source pruning to work
around above issue.

Bugzilla ID: 648

Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
---

v3: Rebase to 69a3c6319140. Add bugzilla ID in commit log.
---
 doc/guides/nics/i40e.rst       |  9 ++++++
 drivers/net/i40e/i40e_ethdev.c | 65 ++++++++++++++++++++++++++++++++++++++++++
 drivers/net/i40e/i40e_ethdev.h |  1 +
 3 files changed, 75 insertions(+)

Comments

Jiang, YuX Feb. 21, 2022, 8:30 a.m. UTC | #1
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Alvin Zhang
> Sent: Wednesday, October 20, 2021 9:29 AM
> To: Xing, Beilei <beilei.xing@intel.com>; Guo, Junfeng
> <junfeng.guo@intel.com>
> Cc: dev@dpdk.org; Zhang, AlvinX <alvinx.zhang@intel.com>
> Subject: [dpdk-dev] [PATCH v3] net/i40e: disable source pruning
> 
> VRRP advertisement packets are dropped on i40e PF devices because when
> a MAC address is added to a device, packets originating from that MAC
> address are dropped.
> 
> This patch adds a devarg to support disabling source pruning to work around
> above issue.
> 
> Bugzilla ID: 648
> 
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> ---
Tested-by:  Yu Jiang <YuX.Jiang@intel.com>

Verified patchset http://patches.dpdk.org/project/dpdk/patch/20211020012831.8480-1-alvinx.zhang@intel.com/ on baseline dpdk22.03-rc1:ecc0dd455e "raw/cnxk_gpio: add option to select subset of GPIOs"
Tested pass on Ethernet Controller XL710 for 40GbE QSFP+ 1583, OS: Fedora Linux 35/5.14.10-300.fc35.x86_64
Test step as below:
 ./dpdk-testpmd -l 1,2 -n 1 -a 18:00.0,disable_source_pruning=1 -- -i
 pkt = Ether(src="00:00:5E:00:01:0A")/IP()/Raw("x"*60)
 test steps:
1). testpmd>set verbose 1
    testpmd>start
2). Send the pkt, the pkt can be received by testpmd
3). testpmd>mac_addr add 0 00:00:5E:00:01:0A 
4). Re-send the pkt, the pkt still can be received by testpmd.
Morten Brørup Feb. 21, 2022, 9:35 a.m. UTC | #2
> From: Jiang, YuX [mailto:yux.jiang@intel.com]
> Sent: Monday, 21 February 2022 09.31
> 
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Alvin Zhang
> > Sent: Wednesday, October 20, 2021 9:29 AM
> >
> > VRRP advertisement packets are dropped on i40e PF devices because
> when
> > a MAC address is added to a device, packets originating from that MAC
> > address are dropped.
> >
> > This patch adds a devarg to support disabling source pruning to work
> around
> > above issue.
> >
> > Bugzilla ID: 648
> >
> > Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> > ---
> Tested-by:  Yu Jiang <YuX.Jiang@intel.com>
> 
> Verified patchset
> http://patches.dpdk.org/project/dpdk/patch/20211020012831.8480-1-
> alvinx.zhang@intel.com/ on baseline dpdk22.03-rc1:ecc0dd455e
> "raw/cnxk_gpio: add option to select subset of GPIOs"
> Tested pass on Ethernet Controller XL710 for 40GbE QSFP+ 1583, OS:
> Fedora Linux 35/5.14.10-300.fc35.x86_64
> Test step as below:
>  ./dpdk-testpmd -l 1,2 -n 1 -a 18:00.0,disable_source_pruning=1 -- -i
>  pkt = Ether(src="00:00:5E:00:01:0A")/IP()/Raw("x"*60)
>  test steps:
> 1). testpmd>set verbose 1
>     testpmd>start
> 2). Send the pkt, the pkt can be received by testpmd
> 3). testpmd>mac_addr add 0 00:00:5E:00:01:0A
> 4). Re-send the pkt, the pkt still can be received by testpmd.

If source pruning is not the default behavior of all NICs, it should be disabled by default in the i40e NIC too.

A NIC shouldn't drop any packets unless it has explicitly been configured for it! And a NIC shouldn't treat any packets differently than other NICs do, unless the NIC has explicitly been configured so!

Furthermore, I would prefer that configurations for explicitly dropping certain types of packets is available through runtime APIs, e.g. RTE_FLOWS, or dedicated functions like rte_eth_promiscuous_enable/disable(). This patch doesn't support runtime detection of installed NICs performed by the application.

I am very surprised by this default behavior of a NIC. Please confirm that Source Pruning is at least disabled in Promiscuous mode?

-Morten
diff mbox series

Patch

diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst
index ef91b3a..1089a3a 100644
--- a/doc/guides/nics/i40e.rst
+++ b/doc/guides/nics/i40e.rst
@@ -236,6 +236,15 @@  Runtime Config Options
 
   -a 84:00.0,vf_msg_cfg=80@120:180
 
+- ``Disable source pruning on PF`` (default ``not disabled``)
+
+  When a MAC address is added to a device, packets originating from that MAC
+  address will be dropped for source pruning. To disable source pruning on PF
+  we can add ``devargs`` parameter ``disable_source_pruning=[0,1]`` and set its
+  value to 1, for example::
+
+  -a 84:00.0,disable_source_pruning=1
+
 Vector RX Pre-conditions
 ~~~~~~~~~~~~~~~~~~~~~~~~
 For Vector RX it is assumed that the number of descriptor rings will be a power
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 1fc3d89..3722e87 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -47,6 +47,7 @@ 
 #define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
 #define ETH_I40E_QUEUE_NUM_PER_VF_ARG	"queue-num-per-vf"
 #define ETH_I40E_VF_MSG_CFG		"vf_msg_cfg"
+#define ETH_I40E_DISABLE_SOURCE_PRUNING_ARG	"disable_source_pruning"
 
 #define I40E_CLEAR_PXE_WAIT_MS     200
 #define I40E_VSI_TSR_QINQ_STRIP		0x4010
@@ -411,6 +412,7 @@  static int i40e_sw_tunnel_filter_insert(struct i40e_pf *pf,
 	ETH_I40E_SUPPORT_MULTI_DRIVER,
 	ETH_I40E_QUEUE_NUM_PER_VF_ARG,
 	ETH_I40E_VF_MSG_CFG,
+	ETH_I40E_DISABLE_SOURCE_PRUNING_ARG,
 	NULL};
 
 static const struct rte_pci_id pci_id_i40e_map[] = {
@@ -1368,6 +1370,23 @@  static inline void i40e_clear_automask(struct i40e_pf *pf)
 }
 
 static int
+i40e_parse_bool(__rte_unused const char *key, const char *value, void *opaque)
+{
+	char *end;
+	unsigned long i;
+
+	errno = 0;
+	i = strtoul(value, &end, 10);
+	if (errno != 0 || end == value || *end != 0 || i > 1) {
+		PMD_DRV_LOG(ERR, "Wrong bool value: %s", value);
+		return -EINVAL;
+	}
+
+	*(bool *)opaque = (bool)i;
+	return 0;
+}
+
+static int
 i40e_parse_vf_msg_config(struct rte_eth_dev *dev,
 		struct i40e_vf_msg_cfg *msg_cfg)
 {
@@ -1404,6 +1423,42 @@  static inline void i40e_clear_automask(struct i40e_pf *pf)
 	return ret;
 }
 
+static int
+i40e_parse_source_pruning_config(struct rte_eth_dev *dev)
+{
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+	struct rte_kvargs *kvlist;
+	int kvargs_count;
+	int ret = -EINVAL;
+
+	if (!dev->device->devargs)
+		return 0;
+
+	kvlist = rte_kvargs_parse(dev->device->devargs->args, valid_keys);
+	if (!kvlist)
+		return 0;
+
+	kvargs_count = rte_kvargs_count(kvlist,
+					ETH_I40E_DISABLE_SOURCE_PRUNING_ARG);
+	if (kvargs_count > 1) {
+		PMD_DRV_LOG(ERR, "More than one argument \"%s\"!",
+			    ETH_I40E_DISABLE_SOURCE_PRUNING_ARG);
+		goto free_end;
+	}
+
+	if (kvargs_count &&
+	    rte_kvargs_process(kvlist, ETH_I40E_DISABLE_SOURCE_PRUNING_ARG,
+			       i40e_parse_bool,
+			       &pf->disable_source_pruning) < 0)
+		goto free_end;
+
+	ret = 0;
+
+free_end:
+	rte_kvargs_free(kvlist);
+	return ret;
+}
+
 #define I40E_ALARM_INTERVAL 50000 /* us */
 
 static int
@@ -1487,6 +1542,10 @@  static inline void i40e_clear_automask(struct i40e_pf *pf)
 	/* Check if need to support multi-driver */
 	i40e_support_multi_driver(dev);
 
+	ret = i40e_parse_source_pruning_config(dev);
+	if (ret)
+		return ret;
+
 	/* Make sure all is clean before doing PF reset */
 	i40e_clear_hw(hw);
 
@@ -5812,6 +5871,12 @@  struct i40e_vsi *
 		ctxt.pf_num = hw->pf_id;
 		ctxt.uplink_seid = vsi->uplink_seid;
 		ctxt.vf_num = 0;
+		if (pf->disable_source_pruning) {
+			ctxt.info.valid_sections |=
+				cpu_to_le16(I40E_AQ_VSI_PROP_SWITCH_VALID);
+			ctxt.info.switch_id |=
+				cpu_to_le16(I40E_AQ_VSI_SW_ID_FLAG_LOCAL_LB);
+		}
 
 		/* Update VSI parameters */
 		ret = i40e_aq_update_vsi_params(hw, &ctxt, NULL);
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 8a68b36..b441153 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -1171,6 +1171,7 @@  struct i40e_pf {
 	bool dport_replace_flag;   /* Destination port replace is done */
 	struct i40e_tm_conf tm_conf;
 	bool support_multi_driver; /* 1 - support multiple driver */
+	bool disable_source_pruning; /* 1 - disable source pruning */
 
 	/* Dynamic Device Personalization */
 	bool gtp_support; /* 1 - support GTP-C and GTP-U */