Re: [dpdk-dev] [PATCH] net/mlx5:fix storing the synced MAC to internal table

[Slava Ovsiienko <viacheslavo@nvidia.com>]

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;
+ }
}
}
}
-- 
2.9.3.windows.1

________________________________________
Notice: This e-mail together with any attachments may contain information of Ribbon Communications Inc. that is confidential and/or proprietary for the sole use of the intended recipient. Any review, disclosure, reliance or distribution by others or forwarding without express permission is strictly prohibited. If you are not the intended recipient, please notify the sender immediately and then delete all copies, including any attachments.
________________________________________