[v2,3/3] net/enic: allow multicast in MAC address add callback

enic_set_mac_address() (mac_addr_add callback) currently allows only
non-zero, unicast address to be added. It is overly restrictive.
rte_eth_dev_mac_addr_add() itself allows multicast addresses. And,
some applications do use rte_eth_dev_mac_addr_add() to accept
multicast addresses. So, remove the unicast check in
enic_set_mac_address().

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
Reviewed-by: John Daley <johndale@cisco.com>
---
 drivers/net/enic/enic_main.c | 20 --------------------
 1 file changed, 20 deletions(-)
  

On 8/8/2024 7:14 AM, Hyong Youb Kim wrote:
> enic_set_mac_address() (mac_addr_add callback) currently allows only
> non-zero, unicast address to be added. It is overly restrictive.
> rte_eth_dev_mac_addr_add() itself allows multicast addresses. And,
> some applications do use rte_eth_dev_mac_addr_add() to accept
> multicast addresses. So, remove the unicast check in
> enic_set_mac_address().
> 

What is the usecase to set Multicast MAC address as device default MAC
address?

Also, just a reminder that we have 'rte_eth_dev_set_mc_addr_list()' API
to set multicast MAC addresses.
  

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Thursday, August 8, 2024 5:51 PM
> To: Hyong Youb Kim (hyonkim) <hyonkim@cisco.com>
> Cc: dev@dpdk.org; John Daley (johndale) <johndale@cisco.com>
> Subject: Re: [PATCH v2 3/3] net/enic: allow multicast in MAC address add
> callback
> 
> On 8/8/2024 7:14 AM, Hyong Youb Kim wrote:
> > enic_set_mac_address() (mac_addr_add callback) currently allows only
> > non-zero, unicast address to be added. It is overly restrictive.
> > rte_eth_dev_mac_addr_add() itself allows multicast addresses. And,
> > some applications do use rte_eth_dev_mac_addr_add() to accept
> > multicast addresses. So, remove the unicast check in
> > enic_set_mac_address().
> >
> 
> What is the usecase to set Multicast MAC address as device default MAC
> address?
> 
> Also, just a reminder that we have 'rte_eth_dev_set_mc_addr_list()' API
> to set multicast MAC addresses.

I am aware of mc_addr_list().. Some people seem to use mac_addr_add()
as 'add a MAC filter'.

For the VIC adapter, there is no difference between the default MAC and
other MAC addresses added to allow/accept. They are all MAC filters in HW. 
I believe some other NICs also do not distinguish 'default' vs. additional
allowed MAC addresses (can be unicast or multicast).

The app in question is VPP. There might be others. It uses
rte_eth_dev_mac_addr_add() to add multicast MAC filters like all-hosts
address. I am not defending such usage. But given that (1)
rte_eth_dev_mac_addr_add() accepts multicast, and (2) adding
multicast works fine on multiple NICs, such usage is probably acceptable.

I am guessing some app developers find it easier to use mac_addr_add()
than mc_addr_list()..

We could tighten the semantics of rte_eth_dev_mac_addr_add() and
check if the given MAC is a valid unicast address in that API function.

Thanks.
-Hyong