| Message ID | 20201209151108.12352-2-sodey@rbbn.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Raslan Darawsheh |
| Headers | show |
| Series | net/mlx5:fix storing the synced MAC to internal table | expand |
| Context | Check | Description |
|---|---|---|
| ci/travis-robot | success | Travis build: passed |
| ci/iol-intel-Performance | success | Performance Testing PASS |
| ci/iol-testing | success | Testing PASS |
| ci/iol-intel-Functional | success | Functional Testing PASS |
| ci/iol-broadcom-Performance | success | Performance Testing PASS |
| ci/iol-broadcom-Functional | success | Functional Testing PASS |
| ci/Intel-compilation | success | Compilation OK |
| ci/checkpatch | success | coding style OK |
Hi, Souvik Thank you for the patch, please see my comments below. >From: Souvik Dey <sodey@rbbn.com> >Sent: Wednesday, December 9, 2020 17:11 >To: Raslan Darawsheh <rasland@nvidia.com>; Matan Azrad <matan@nvidia.com>; Shahaf Shuler ><shahafs@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com> >Cc: dev@dpdk.org; Souvik Dey <sodey@rbbn.com> >Subject: [PATCH] net/mlx5:fix storing the synced MAC to internal table We have the warnings: Wrong headline format: net/mlx5:fix storing the synced MAC to internal table Wrong headline prefix: net/mlx5:fix storing the synced MAC to internal table Missing 'Fixes' tag: net/mlx5:fix storing the synced MAC to internal table The patch headline should be: common/mlx5: fix storing the synched MAC to internal table - net/ -> common/ - space after mlx5: - synced -> synched The patch should contain the Fixes tag: Fixes: f22442cb5d42 (“net/mlx5: reduce Netlink commands dependencies”) Fixes: ccdcba53a3f4 (“net/mlx5: use Netlink to add/remove MAC addresses”) The patch should contain (to be sent to LTS ML): Cc: stable@dpdk.org >As the internal MAC table is divided into Unicast and Multicast address >section, we should check the type of synced MAC address before storing section -> sections synced -> synched >it to the internal table. Currently the check is not done, and the >synced MAC of 33:33:00:00:00:01 gets stored in the unicast section sync ->synched >(mostly index 1) which causes all subsequent mlx5_set_mc_addr_list() reword? : which causes -> causing >to fail with error -EADDRINUSE, as the mac_list contains the MAC >33:33:00:00:00:01. This denies adding of any new multicast address to >the internal list and also fails to add the MAC address to the device >in case of SR-IOV VF case. typo: case (to remove) With best regards, Slava diff --git a/drivers/common/mlx5/linux/mlx5_nl.c b/drivers/common/mlx5/linux/mlx5_nl.c index 40d8620..ef7a521 100644 --- a/drivers/common/mlx5/linux/mlx5_nl.c +++ b/drivers/common/mlx5/linux/mlx5_nl.c @@ -758,11 +758,21 @@ mlx5_nl_mac_addr_sync(int nlsk_fd, unsigned int iface_idx, break; if (j != n) continue; - /* Find the first entry available. */ - for (j = 0; j != n; ++j) { - if (rte_is_zero_ether_addr(&mac_addrs[j])) { - mac_addrs[j] = macs[i]; - break; + if (rte_is_multicast_ether_addr(&macs[i])) { + /* Find the first entry available. */ + for (j = MLX5_MAX_UC_MAC_ADDRESSES; j != n; ++j) { + if (rte_is_zero_ether_addr(&mac_addrs[j])) { + mac_addrs[j] = macs[i]; + break; + } + } + } else { + /* Find the first entry available. */ + for (j = 0; j != MLX5_MAX_UC_MAC_ADDRESSES; ++j) { + if (rte_is_zero_ether_addr(&mac_addrs[j])) { + mac_addrs[j] = macs[i]; + break; + } } } }
diff --git a/drivers/common/mlx5/linux/mlx5_nl.c b/drivers/common/mlx5/linux/mlx5_nl.c index 40d8620..ef7a521 100644 --- a/drivers/common/mlx5/linux/mlx5_nl.c +++ b/drivers/common/mlx5/linux/mlx5_nl.c @@ -758,11 +758,21 @@ mlx5_nl_mac_addr_sync(int nlsk_fd, unsigned int iface_idx, break; if (j != n) continue; - /* Find the first entry available. */ - for (j = 0; j != n; ++j) { - if (rte_is_zero_ether_addr(&mac_addrs[j])) { - mac_addrs[j] = macs[i]; - break; + if (rte_is_multicast_ether_addr(&macs[i])) { + /* Find the first entry available. */ + for (j = MLX5_MAX_UC_MAC_ADDRESSES; j != n; ++j) { + if (rte_is_zero_ether_addr(&mac_addrs[j])) { + mac_addrs[j] = macs[i]; + break; + } + } + } else { + /* Find the first entry available. */ + for (j = 0; j != MLX5_MAX_UC_MAC_ADDRESSES; ++j) { + if (rte_is_zero_ether_addr(&mac_addrs[j])) { + mac_addrs[j] = macs[i]; + break; + } } } }
As the internal MAC table is divided into Unicast and Multicast address section, we should check the type of synced MAC address before storing it to the internal table. Currently the check is not done, and the synced MAC of 33:33:00:00:00:01 gets stored in the unicast section (mostly index 1) which causes all subsequent mlx5_set_mc_addr_list() to fail with error -EADDRINUSE, as the mac_list contains the MAC 33:33:00:00:00:01. This denies adding of any new multicast address to the internal list and also fails to add the MAC address to the device in case of SR-IOV VF case. Signed-off-by: Souvik Dey <sodey@rbbn.com> --- drivers/common/mlx5/linux/mlx5_nl.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)