net/mlx5:fix storing the synced MAC to internal table

Message ID 20201209151108.12352-2-sodey@rbbn.com (mailing list archive)
State Superseded, archived
Delegated to: Raslan Darawsheh
Headers
Series net/mlx5:fix storing the synced MAC to internal table |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed

Commit Message

Dey, Souvik Dec. 9, 2020, 3:11 p.m. UTC
  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(-)
  

Comments

Slava Ovsiienko Jan. 21, 2021, 4:56 p.m. UTC | #1
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;
+ }
}
}
}
  

Patch

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;
+				}
 			}
 		}
 	}