[dpdk-dev] [PATCH] i40e: fix i40e_validate_mac_addr to permit multicast addresses

[dpdk-dev] [PATCH] i40e: fix i40e_validate_mac_addr to permit multicast addresses

[David Harton]

The i40e maintains a single MAC filter table for both
unicast and multicast addresses.  The i40e_validate_mac_addr
function was preventing multicast addresses from being added
to the table via i40evf_add_mac_addr.  Fixed the issue by
removing the multicast address check in i40e_validate_mac_addr.

Signed-off-by: David Harton <dharton@cisco.com>
---
 drivers/net/i40e/base/i40e_common.c | 12 +++++-------
 drivers/net/i40e/i40e_ethdev.c      |  3 ++-
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/net/i40e/base/i40e_common.c b/drivers/net/i40e/base/i40e_common.c
index 900d379..9779854 100644
--- a/drivers/net/i40e/base/i40e_common.c
+++ b/drivers/net/i40e/base/i40e_common.c
@@ -969,10 +969,10 @@ struct i40e_rx_ptype_decoded i40e_ptype_lookup[] = {
 
 
 /**
- * i40e_validate_mac_addr - Validate unicast MAC address
+ * i40e_validate_mac_addr - Validate MAC address
  * @mac_addr: pointer to MAC address
  *
- * Tests a MAC address to ensure it is a valid Individual Address
+ * Tests a MAC address to ensure it is a valid Address
  **/
 enum i40e_status_code i40e_validate_mac_addr(u8 *mac_addr)
 {
@@ -980,13 +980,11 @@ enum i40e_status_code i40e_validate_mac_addr(u8 *mac_addr)
 
 	DEBUGFUNC("i40e_validate_mac_addr");
 
-	/* Broadcast addresses ARE multicast addresses
-	 * Make sure it is not a multicast address
+	/*
 	 * Reject the zero address
 	 */
-	if (I40E_IS_MULTICAST(mac_addr) ||
-	    (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
-	      mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0))
+	if (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
+	    mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0)
 		status = I40E_ERR_INVALID_MAC_ADDR;
 
 	return status;
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 5f26e24..00b6082 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1199,7 +1199,8 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
 
 	/* Get and check the mac address */
 	i40e_get_mac_addr(hw, hw->mac.addr);
-	if (i40e_validate_mac_addr(hw->mac.addr) != I40E_SUCCESS) {
+	if (i40e_validate_mac_addr(hw->mac.addr) != I40E_SUCCESS ||
+	    I40E_IS_MULTICAST(hw->mac.addr)) {
 		PMD_INIT_LOG(ERR, "mac address is not valid");
 		ret = -EIO;
 		goto err_get_mac_addr;
-- 
2.10.3.dirty

Re: [dpdk-dev] [PATCH] i40e: fix i40e_validate_mac_addr to permit multicast addresses

[Ferruh Yigit]

On 8/22/2017 11:21 PM, David Harton wrote:
> The i40e maintains a single MAC filter table for both
> unicast and multicast addresses.  The i40e_validate_mac_addr
> function was preventing multicast addresses from being added
> to the table via i40evf_add_mac_addr.  Fixed the issue by
> removing the multicast address check in i40e_validate_mac_addr.
> 
> Signed-off-by: David Harton <dharton@cisco.com>
> ---
>  drivers/net/i40e/base/i40e_common.c | 12 +++++-------
>  drivers/net/i40e/i40e_ethdev.c      |  3 ++-
>  2 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/i40e/base/i40e_common.c b/drivers/net/i40e/base/i40e_common.c
> index 900d379..9779854 100644
> --- a/drivers/net/i40e/base/i40e_common.c
> +++ b/drivers/net/i40e/base/i40e_common.c
> @@ -969,10 +969,10 @@ struct i40e_rx_ptype_decoded i40e_ptype_lookup[] = {
>  
>  
>  /**
> - * i40e_validate_mac_addr - Validate unicast MAC address
> + * i40e_validate_mac_addr - Validate MAC address
>   * @mac_addr: pointer to MAC address
>   *
> - * Tests a MAC address to ensure it is a valid Individual Address
> + * Tests a MAC address to ensure it is a valid Address
>   **/
>  enum i40e_status_code i40e_validate_mac_addr(u8 *mac_addr)
>  {
> @@ -980,13 +980,11 @@ enum i40e_status_code i40e_validate_mac_addr(u8 *mac_addr)
>  
>  	DEBUGFUNC("i40e_validate_mac_addr");
>  
> -	/* Broadcast addresses ARE multicast addresses
> -	 * Make sure it is not a multicast address
> +	/*
>  	 * Reject the zero address
>  	 */
> -	if (I40E_IS_MULTICAST(mac_addr) ||
> -	    (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
> -	      mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0))
> +	if (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
> +	    mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0)
>  		status = I40E_ERR_INVALID_MAC_ADDR;
>  
>  	return status;
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 5f26e24..00b6082 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -1199,7 +1199,8 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
>  
>  	/* Get and check the mac address */
>  	i40e_get_mac_addr(hw, hw->mac.addr);
> -	if (i40e_validate_mac_addr(hw->mac.addr) != I40E_SUCCESS) {
> +	if (i40e_validate_mac_addr(hw->mac.addr) != I40E_SUCCESS ||
> +	    I40E_IS_MULTICAST(hw->mac.addr)) {

As far as I can see this is to set PF permanent mac address during
init(), i40e_macaddr_add() can be used to set multicast address filters.
Why do you want to set multicast address as mac permanent address?

Any chance that you want to update i40evf_add_mac_addr() to let
multicast addresses?

>  		PMD_INIT_LOG(ERR, "mac address is not valid");
>  		ret = -EIO;
>  		goto err_get_mac_addr;
> 

Re: [dpdk-dev] [PATCH] i40e: fix i40e_validate_mac_addr to permit multicast addresses

[David Harton (dharton)]

> -----Original Message-----
> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Thursday, August 31, 2017 11:54 AM
> To: David Harton (dharton) <dharton@cisco.com>; jingjing.wu@intel.com;
> beilei.xing@intel.com
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] i40e: fix i40e_validate_mac_addr to permit
> multicast addresses
> 
> On 8/22/2017 11:21 PM, David Harton wrote:
> > The i40e maintains a single MAC filter table for both unicast and
> > multicast addresses.  The i40e_validate_mac_addr function was
> > preventing multicast addresses from being added to the table via
> > i40evf_add_mac_addr.  Fixed the issue by removing the multicast
> > address check in i40e_validate_mac_addr.
> >
> > Signed-off-by: David Harton <dharton@cisco.com>
> > ---
> >  drivers/net/i40e/base/i40e_common.c | 12 +++++-------
> >  drivers/net/i40e/i40e_ethdev.c      |  3 ++-
> >  2 files changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/i40e/base/i40e_common.c
> > b/drivers/net/i40e/base/i40e_common.c
> > index 900d379..9779854 100644
> > --- a/drivers/net/i40e/base/i40e_common.c
> > +++ b/drivers/net/i40e/base/i40e_common.c
> > @@ -969,10 +969,10 @@ struct i40e_rx_ptype_decoded i40e_ptype_lookup[]
> > = {
> >
> >
> >  /**
> > - * i40e_validate_mac_addr - Validate unicast MAC address
> > + * i40e_validate_mac_addr - Validate MAC address
> >   * @mac_addr: pointer to MAC address
> >   *
> > - * Tests a MAC address to ensure it is a valid Individual Address
> > + * Tests a MAC address to ensure it is a valid Address
> >   **/
> >  enum i40e_status_code i40e_validate_mac_addr(u8 *mac_addr)  { @@
> > -980,13 +980,11 @@ enum i40e_status_code i40e_validate_mac_addr(u8
> > *mac_addr)
> >
> >  	DEBUGFUNC("i40e_validate_mac_addr");
> >
> > -	/* Broadcast addresses ARE multicast addresses
> > -	 * Make sure it is not a multicast address
> > +	/*
> >  	 * Reject the zero address
> >  	 */
> > -	if (I40E_IS_MULTICAST(mac_addr) ||
> > -	    (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
> > -	      mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0))
> > +	if (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
> > +	    mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0)
> >  		status = I40E_ERR_INVALID_MAC_ADDR;
> >
> >  	return status;
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 5f26e24..00b6082 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -1199,7 +1199,8 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
> >
> >  	/* Get and check the mac address */
> >  	i40e_get_mac_addr(hw, hw->mac.addr);
> > -	if (i40e_validate_mac_addr(hw->mac.addr) != I40E_SUCCESS) {
> > +	if (i40e_validate_mac_addr(hw->mac.addr) != I40E_SUCCESS ||
> > +	    I40E_IS_MULTICAST(hw->mac.addr)) {
> 
> As far as I can see this is to set PF permanent mac address during init(),
> i40e_macaddr_add() can be used to set multicast address filters.
> Why do you want to set multicast address as mac permanent address?
> 
> Any chance that you want to update i40evf_add_mac_addr() to let multicast
> addresses?

No.  I'm preserving the existing behavior here.  The previous call used to ensure that the permanent mac address here was both non-null and not a multi-cast address.

Since I removed the multi cast check from i40e_validate_mac_addr() I added the multicast check here.  If "not valid" or a multicast address fail.

Regards,
Dave

> 
> >  		PMD_INIT_LOG(ERR, "mac address is not valid");
> >  		ret = -EIO;
> >  		goto err_get_mac_addr;
> >

Re: [dpdk-dev] [PATCH] i40e: fix i40e_validate_mac_addr to permit multicast addresses

[David Harton (dharton)]

Hi Jingjing/Beilei,

A kind reminder to review the patch and the discussion between 
Ferruh and myself.

Thanks,
Dave

> -----Original Message-----
> From: David Harton (dharton)
> > -----Original Message-----
> > From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> >
> > On 8/22/2017 11:21 PM, David Harton wrote:
> > > The i40e maintains a single MAC filter table for both unicast and
> > > multicast addresses.  The i40e_validate_mac_addr function was
> > > preventing multicast addresses from being added to the table via
> > > i40evf_add_mac_addr.  Fixed the issue by removing the multicast
> > > address check in i40e_validate_mac_addr.
> > >
> > > Signed-off-by: David Harton <dharton@cisco.com>
> > > ---
> > >  drivers/net/i40e/base/i40e_common.c | 12 +++++-------
> > >  drivers/net/i40e/i40e_ethdev.c      |  3 ++-
> > >  2 files changed, 7 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/net/i40e/base/i40e_common.c
> > > b/drivers/net/i40e/base/i40e_common.c
> > > index 900d379..9779854 100644
> > > --- a/drivers/net/i40e/base/i40e_common.c
> > > +++ b/drivers/net/i40e/base/i40e_common.c
> > > @@ -969,10 +969,10 @@ struct i40e_rx_ptype_decoded
> > > i40e_ptype_lookup[] = {
> > >
> > >
> > >  /**
> > > - * i40e_validate_mac_addr - Validate unicast MAC address
> > > + * i40e_validate_mac_addr - Validate MAC address
> > >   * @mac_addr: pointer to MAC address
> > >   *
> > > - * Tests a MAC address to ensure it is a valid Individual Address
> > > + * Tests a MAC address to ensure it is a valid Address
> > >   **/
> > >  enum i40e_status_code i40e_validate_mac_addr(u8 *mac_addr)  { @@
> > > -980,13 +980,11 @@ enum i40e_status_code i40e_validate_mac_addr(u8
> > > *mac_addr)
> > >
> > >  	DEBUGFUNC("i40e_validate_mac_addr");
> > >
> > > -	/* Broadcast addresses ARE multicast addresses
> > > -	 * Make sure it is not a multicast address
> > > +	/*
> > >  	 * Reject the zero address
> > >  	 */
> > > -	if (I40E_IS_MULTICAST(mac_addr) ||
> > > -	    (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
> > > -	      mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0))
> > > +	if (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
> > > +	    mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0)
> > >  		status = I40E_ERR_INVALID_MAC_ADDR;
> > >
> > >  	return status;
> > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > b/drivers/net/i40e/i40e_ethdev.c index 5f26e24..00b6082 100644
> > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > @@ -1199,7 +1199,8 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
> > >
> > >  	/* Get and check the mac address */
> > >  	i40e_get_mac_addr(hw, hw->mac.addr);
> > > -	if (i40e_validate_mac_addr(hw->mac.addr) != I40E_SUCCESS) {
> > > +	if (i40e_validate_mac_addr(hw->mac.addr) != I40E_SUCCESS ||
> > > +	    I40E_IS_MULTICAST(hw->mac.addr)) {
> >
> > As far as I can see this is to set PF permanent mac address during
> > init(),
> > i40e_macaddr_add() can be used to set multicast address filters.
> > Why do you want to set multicast address as mac permanent address?
> >
> > Any chance that you want to update i40evf_add_mac_addr() to let
> > multicast addresses?
> 
> No.  I'm preserving the existing behavior here.  The previous call used to
> ensure that the permanent mac address here was both non-null and not a
> multi-cast address.
> 
> Since I removed the multi cast check from i40e_validate_mac_addr() I added
> the multicast check here.  If "not valid" or a multicast address fail.
> 
> Regards,
> Dave
> 
> >
> > >  		PMD_INIT_LOG(ERR, "mac address is not valid");
> > >  		ret = -EIO;
> > >  		goto err_get_mac_addr;
> > >

Re: [dpdk-dev] [PATCH] i40e: fix i40e_validate_mac_addr to permit multicast addresses

[Ferruh Yigit]

On 9/8/2017 1:51 PM, David Harton (dharton) wrote:
> Hi Jingjing/Beilei,
> 
> A kind reminder to review the patch and the discussion between 
> Ferruh and myself.

Hi David,

To clarify, your reply answered my concern, so I am OK, still I believe
this should be reviewed by driver maintainer.

Thanks,
ferruh

> 
> Thanks,
> Dave
> 
>> -----Original Message-----
>> From: David Harton (dharton)
>>> -----Original Message-----
>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>
>>> On 8/22/2017 11:21 PM, David Harton wrote:
>>>> The i40e maintains a single MAC filter table for both unicast and
>>>> multicast addresses.  The i40e_validate_mac_addr function was
>>>> preventing multicast addresses from being added to the table via
>>>> i40evf_add_mac_addr.  Fixed the issue by removing the multicast
>>>> address check in i40e_validate_mac_addr.
>>>>
>>>> Signed-off-by: David Harton <dharton@cisco.com>
>>>> ---
>>>>  drivers/net/i40e/base/i40e_common.c | 12 +++++-------
>>>>  drivers/net/i40e/i40e_ethdev.c      |  3 ++-
>>>>  2 files changed, 7 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/net/i40e/base/i40e_common.c
>>>> b/drivers/net/i40e/base/i40e_common.c
>>>> index 900d379..9779854 100644
>>>> --- a/drivers/net/i40e/base/i40e_common.c
>>>> +++ b/drivers/net/i40e/base/i40e_common.c
>>>> @@ -969,10 +969,10 @@ struct i40e_rx_ptype_decoded
>>>> i40e_ptype_lookup[] = {
>>>>
>>>>
>>>>  /**
>>>> - * i40e_validate_mac_addr - Validate unicast MAC address
>>>> + * i40e_validate_mac_addr - Validate MAC address
>>>>   * @mac_addr: pointer to MAC address
>>>>   *
>>>> - * Tests a MAC address to ensure it is a valid Individual Address
>>>> + * Tests a MAC address to ensure it is a valid Address
>>>>   **/
>>>>  enum i40e_status_code i40e_validate_mac_addr(u8 *mac_addr)  { @@
>>>> -980,13 +980,11 @@ enum i40e_status_code i40e_validate_mac_addr(u8
>>>> *mac_addr)
>>>>
>>>>  	DEBUGFUNC("i40e_validate_mac_addr");
>>>>
>>>> -	/* Broadcast addresses ARE multicast addresses
>>>> -	 * Make sure it is not a multicast address
>>>> +	/*
>>>>  	 * Reject the zero address
>>>>  	 */
>>>> -	if (I40E_IS_MULTICAST(mac_addr) ||
>>>> -	    (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
>>>> -	      mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0))
>>>> +	if (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
>>>> +	    mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0)
>>>>  		status = I40E_ERR_INVALID_MAC_ADDR;
>>>>
>>>>  	return status;
>>>> diff --git a/drivers/net/i40e/i40e_ethdev.c
>>>> b/drivers/net/i40e/i40e_ethdev.c index 5f26e24..00b6082 100644
>>>> --- a/drivers/net/i40e/i40e_ethdev.c
>>>> +++ b/drivers/net/i40e/i40e_ethdev.c
>>>> @@ -1199,7 +1199,8 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
>>>>
>>>>  	/* Get and check the mac address */
>>>>  	i40e_get_mac_addr(hw, hw->mac.addr);
>>>> -	if (i40e_validate_mac_addr(hw->mac.addr) != I40E_SUCCESS) {
>>>> +	if (i40e_validate_mac_addr(hw->mac.addr) != I40E_SUCCESS ||
>>>> +	    I40E_IS_MULTICAST(hw->mac.addr)) {
>>>
>>> As far as I can see this is to set PF permanent mac address during
>>> init(),
>>> i40e_macaddr_add() can be used to set multicast address filters.
>>> Why do you want to set multicast address as mac permanent address?
>>>
>>> Any chance that you want to update i40evf_add_mac_addr() to let
>>> multicast addresses?
>>
>> No.  I'm preserving the existing behavior here.  The previous call used to
>> ensure that the permanent mac address here was both non-null and not a
>> multi-cast address.
>>
>> Since I removed the multi cast check from i40e_validate_mac_addr() I added
>> the multicast check here.  If "not valid" or a multicast address fail.
>>
>> Regards,
>> Dave
>>
>>>
>>>>  		PMD_INIT_LOG(ERR, "mac address is not valid");
>>>>  		ret = -EIO;
>>>>  		goto err_get_mac_addr;
>>>>
> 

Re: [dpdk-dev] [PATCH] i40e: fix i40e_validate_mac_addr to permit multicast addresses

[Xing, Beilei]

Hi,

> -----Original Message-----
> From: David Harton [mailto:dharton@cisco.com]
> Sent: Wednesday, August 23, 2017 6:22 AM
> To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; David Harton <dharton@cisco.com>
> Subject: [PATCH] i40e: fix i40e_validate_mac_addr to permit multicast
> addresses
> 
> The i40e maintains a single MAC filter table for both unicast and multicast
> addresses.  The i40e_validate_mac_addr function was preventing multicast
> addresses from being added to the table via i40evf_add_mac_addr.  Fixed
> the issue by removing the multicast address check in
> i40e_validate_mac_addr.
> 

Maybe fix line is needed here.

> Signed-off-by: David Harton <dharton@cisco.com>
> ---
>  drivers/net/i40e/base/i40e_common.c | 12 +++++-------
>  drivers/net/i40e/i40e_ethdev.c      |  3 ++-
>  2 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/i40e/base/i40e_common.c
> b/drivers/net/i40e/base/i40e_common.c
> index 900d379..9779854 100644
> --- a/drivers/net/i40e/base/i40e_common.c
> +++ b/drivers/net/i40e/base/i40e_common.c
> @@ -969,10 +969,10 @@ struct i40e_rx_ptype_decoded
> i40e_ptype_lookup[] = {
> 
> 
>  /**
> - * i40e_validate_mac_addr - Validate unicast MAC address
> + * i40e_validate_mac_addr - Validate MAC address
>   * @mac_addr: pointer to MAC address
>   *
> - * Tests a MAC address to ensure it is a valid Individual Address
> + * Tests a MAC address to ensure it is a valid Address
>   **/
>  enum i40e_status_code i40e_validate_mac_addr(u8 *mac_addr)  { @@ -
> 980,13 +980,11 @@ enum i40e_status_code i40e_validate_mac_addr(u8
> *mac_addr)
> 
>  	DEBUGFUNC("i40e_validate_mac_addr");
> 
> -	/* Broadcast addresses ARE multicast addresses
> -	 * Make sure it is not a multicast address
> +	/*
>  	 * Reject the zero address
>  	 */
> -	if (I40E_IS_MULTICAST(mac_addr) ||
> -	    (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
> -	      mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0))
> +	if (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
> +	    mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0)
>  		status = I40E_ERR_INVALID_MAC_ADDR;
> 

We'd better not to change the base code which is from kernel driver. How about changing the i40evf_add_mac_addr function as following:
-	if (i40e_validate_mac_addr(addr->addr_bytes) != I40E_SUCCESS) {
+	if (is_zero_ether_addr (addr->addr_bytes) != I40E_SUCCESS) {

Then following modification in eth_i40e_dev_init function can be removed. But we should also pay attention to other functions which call
i40evf_add_mac_addr.

Beilei

>  	return status;
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 5f26e24..00b6082 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -1199,7 +1199,8 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
> 
>  	/* Get and check the mac address */
>  	i40e_get_mac_addr(hw, hw->mac.addr);
> -	if (i40e_validate_mac_addr(hw->mac.addr) != I40E_SUCCESS) {
> +	if (i40e_validate_mac_addr(hw->mac.addr) != I40E_SUCCESS ||
> +	    I40E_IS_MULTICAST(hw->mac.addr)) {
>  		PMD_INIT_LOG(ERR, "mac address is not valid");
>  		ret = -EIO;
>  		goto err_get_mac_addr;
> --
> 2.10.3.dirty

Re: [dpdk-dev] [PATCH] i40e: fix i40e_validate_mac_addr to permit multicast addresses

[David Harton (dharton)]

Hi Beilei,

> -----Original Message-----
> From: Xing, Beilei [mailto:beilei.xing@intel.com]
> 
> Hi,
> 
> > -----Original Message-----
> > From: David Harton [mailto:dharton@cisco.com]
> >
> > The i40e maintains a single MAC filter table for both unicast and
> > multicast addresses.  The i40e_validate_mac_addr function was
> > preventing multicast addresses from being added to the table via
> > i40evf_add_mac_addr.  Fixed the issue by removing the multicast
> > address check in i40e_validate_mac_addr.
> >
> 
> Maybe fix line is needed here.

Please elaborate.  If feel this is a regression and you want 
me to add a fixline can you offer the offending commit?

> 
> > Signed-off-by: David Harton <dharton@cisco.com>
> > ---
> >  drivers/net/i40e/base/i40e_common.c | 12 +++++-------
> >  drivers/net/i40e/i40e_ethdev.c      |  3 ++-
> >  2 files changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/i40e/base/i40e_common.c
> > b/drivers/net/i40e/base/i40e_common.c
> > index 900d379..9779854 100644
> > --- a/drivers/net/i40e/base/i40e_common.c
> > +++ b/drivers/net/i40e/base/i40e_common.c
> > @@ -969,10 +969,10 @@ struct i40e_rx_ptype_decoded i40e_ptype_lookup[]
> > = {
> >
> >
> >  /**
> > - * i40e_validate_mac_addr - Validate unicast MAC address
> > + * i40e_validate_mac_addr - Validate MAC address
> >   * @mac_addr: pointer to MAC address
> >   *
> > - * Tests a MAC address to ensure it is a valid Individual Address
> > + * Tests a MAC address to ensure it is a valid Address
> >   **/
> >  enum i40e_status_code i40e_validate_mac_addr(u8 *mac_addr)  { @@ -
> > 980,13 +980,11 @@ enum i40e_status_code i40e_validate_mac_addr(u8
> > *mac_addr)
> >
> >  	DEBUGFUNC("i40e_validate_mac_addr");
> >
> > -	/* Broadcast addresses ARE multicast addresses
> > -	 * Make sure it is not a multicast address
> > +	/*
> >  	 * Reject the zero address
> >  	 */
> > -	if (I40E_IS_MULTICAST(mac_addr) ||
> > -	    (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
> > -	      mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0))
> > +	if (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
> > +	    mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0)
> >  		status = I40E_ERR_INVALID_MAC_ADDR;
> >
> 
> We'd better not to change the base code which is from kernel driver. How
> about changing the i40evf_add_mac_addr function as following:
> -	if (i40e_validate_mac_addr(addr->addr_bytes) != I40E_SUCCESS) {
> +	if (is_zero_ether_addr (addr->addr_bytes) != I40E_SUCCESS) {
> 
> Then following modification in eth_i40e_dev_init function can be removed.
> But we should also pay attention to other functions which call
> i40evf_add_mac_addr.

Sure.  I'll post a new patch as suggested once the fixline question is answered.  

I was trying to fix the issue the way I did in case i40e passthru 
wanted to benefit.  Your suggestion will work for the VF as the only 
other caller for i40evf_add_mac_addr() performs its own macaddr validation.

Thanks,
Dave

> 
> Beilei
> 
> >  	return status;
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 5f26e24..00b6082 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -1199,7 +1199,8 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
> >
> >  	/* Get and check the mac address */
> >  	i40e_get_mac_addr(hw, hw->mac.addr);
> > -	if (i40e_validate_mac_addr(hw->mac.addr) != I40E_SUCCESS) {
> > +	if (i40e_validate_mac_addr(hw->mac.addr) != I40E_SUCCESS ||
> > +	    I40E_IS_MULTICAST(hw->mac.addr)) {
> >  		PMD_INIT_LOG(ERR, "mac address is not valid");
> >  		ret = -EIO;
> >  		goto err_get_mac_addr;
> > --
> > 2.10.3.dirty

Re: [dpdk-dev] [PATCH] i40e: fix i40e_validate_mac_addr to permit multicast addresses

[Xing, Beilei]

> -----Original Message-----
> From: David Harton (dharton) [mailto:dharton@cisco.com]
> Sent: Tuesday, September 12, 2017 1:23 AM
> To: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] i40e: fix i40e_validate_mac_addr to permit multicast
> addresses
> 
> Hi Beilei,
> 
> > -----Original Message-----
> > From: Xing, Beilei [mailto:beilei.xing@intel.com]
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: David Harton [mailto:dharton@cisco.com]
> > >
> > > The i40e maintains a single MAC filter table for both unicast and
> > > multicast addresses.  The i40e_validate_mac_addr function was
> > > preventing multicast addresses from being added to the table via
> > > i40evf_add_mac_addr.  Fixed the issue by removing the multicast
> > > address check in i40e_validate_mac_addr.
> > >
> >
> > Maybe fix line is needed here.
> 
> Please elaborate.  If feel this is a regression and you want me to add a fixline
> can you offer the offending commit?

I think these two commit are related:
commit 4861cde4611601ccc9d467675f9d7a10c3095b54
commit 97ac72aa71a93d5c9bc5dc3ef1d1a324f38e61c8

> 
> >
> > > Signed-off-by: David Harton <dharton@cisco.com>
> > > ---
> > >  drivers/net/i40e/base/i40e_common.c | 12 +++++-------
> > >  drivers/net/i40e/i40e_ethdev.c      |  3 ++-
> > >  2 files changed, 7 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/net/i40e/base/i40e_common.c
> > > b/drivers/net/i40e/base/i40e_common.c
> > > index 900d379..9779854 100644
> > > --- a/drivers/net/i40e/base/i40e_common.c
> > > +++ b/drivers/net/i40e/base/i40e_common.c
> > > @@ -969,10 +969,10 @@ struct i40e_rx_ptype_decoded
> > > i40e_ptype_lookup[] = {
> > >
> > >
> > >  /**
> > > - * i40e_validate_mac_addr - Validate unicast MAC address
> > > + * i40e_validate_mac_addr - Validate MAC address
> > >   * @mac_addr: pointer to MAC address
> > >   *
> > > - * Tests a MAC address to ensure it is a valid Individual Address
> > > + * Tests a MAC address to ensure it is a valid Address
> > >   **/
> > >  enum i40e_status_code i40e_validate_mac_addr(u8 *mac_addr)  { @@ -
> > > 980,13 +980,11 @@ enum i40e_status_code i40e_validate_mac_addr(u8
> > > *mac_addr)
> > >
> > >  	DEBUGFUNC("i40e_validate_mac_addr");
> > >
> > > -	/* Broadcast addresses ARE multicast addresses
> > > -	 * Make sure it is not a multicast address
> > > +	/*
> > >  	 * Reject the zero address
> > >  	 */
> > > -	if (I40E_IS_MULTICAST(mac_addr) ||
> > > -	    (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
> > > -	      mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0))
> > > +	if (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
> > > +	    mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0)
> > >  		status = I40E_ERR_INVALID_MAC_ADDR;
> > >
> >
> > We'd better not to change the base code which is from kernel driver.
> > How about changing the i40evf_add_mac_addr function as following:
> > -	if (i40e_validate_mac_addr(addr->addr_bytes) != I40E_SUCCESS) {
> > +	if (is_zero_ether_addr (addr->addr_bytes) != I40E_SUCCESS) {
> >
> > Then following modification in eth_i40e_dev_init function can be removed.
> > But we should also pay attention to other functions which call
> > i40evf_add_mac_addr.
> 
> Sure.  I'll post a new patch as suggested once the fixline question is answered.
> 
> I was trying to fix the issue the way I did in case i40e passthru wanted to
> benefit.  Your suggestion will work for the VF as the only other caller for
> i40evf_add_mac_addr() performs its own macaddr validation.
> 
> Thanks,
> Dave
> 
> >
> > Beilei
> >
> > >  	return status;
> > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > b/drivers/net/i40e/i40e_ethdev.c index 5f26e24..00b6082 100644
> > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > @@ -1199,7 +1199,8 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
> > >
> > >  	/* Get and check the mac address */
> > >  	i40e_get_mac_addr(hw, hw->mac.addr);
> > > -	if (i40e_validate_mac_addr(hw->mac.addr) != I40E_SUCCESS) {
> > > +	if (i40e_validate_mac_addr(hw->mac.addr) != I40E_SUCCESS ||
> > > +	    I40E_IS_MULTICAST(hw->mac.addr)) {
> > >  		PMD_INIT_LOG(ERR, "mac address is not valid");
> > >  		ret = -EIO;
> > >  		goto err_get_mac_addr;
> > > --
> > > 2.10.3.dirty

[dpdk-dev] [PATCH v2] i40e: fix i40evf_add_mac_addr to permit multicast addresses

[David Harton]

From: David Harton <dharton@cisco.com>

The i40e maintains a single MAC filter table for both
unicast and multicast addresses.  The i40e_validate_mac_addr
function was preventing multicast addresses from being added
to the table via i40evf_add_mac_addr.  Fixed the issue by
adjusting the check in i40evf_add_mac_addr.

Fixes: 4861cde46116 ("i40e: new poll mode driver")
Fixes: 97ac72aa71a9 ("i40e: support setting VF MAC address")

Signed-off-by: David Harton <dharton@cisco.com>
---

v2
* Removed multicast check in i40evf_add_mac_addr.

v1
* Removed multicast check in i40e_validate_mac_addr.

 drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index f6d8293..5916d11 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -888,7 +888,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
 	int err;
 	struct vf_cmd_info args;
 
-	if (i40e_validate_mac_addr(addr->addr_bytes) != I40E_SUCCESS) {
+	if (is_zero_ether_addr(addr) != I40E_SUCCESS) {
 		PMD_DRV_LOG(ERR, "Invalid mac:%x:%x:%x:%x:%x:%x",
 			    addr->addr_bytes[0], addr->addr_bytes[1],
 			    addr->addr_bytes[2], addr->addr_bytes[3],
-- 
2.10.3.dirty

Re: [dpdk-dev] [PATCH v2] i40e: fix i40evf_add_mac_addr to permit multicast addresses

[Xing, Beilei]

Hi Harton,

> -----Original Message-----
> From: David Harton [mailto:dharton@cpp-rtpbld-31.cpprtplab]
> Sent: Tuesday, September 12, 2017 9:02 PM
> To: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; David Harton <dharton@cisco.com>
> Subject: [PATCH v2] i40e: fix i40evf_add_mac_addr to permit multicast
> addresses
> 
> From: David Harton <dharton@cisco.com>
> 
> The i40e maintains a single MAC filter table for both unicast and multicast
> addresses.  The i40e_validate_mac_addr function was preventing multicast
> addresses from being added to the table via i40evf_add_mac_addr.  Fixed
> the issue by adjusting the check in i40evf_add_mac_addr.
> 
> Fixes: 4861cde46116 ("i40e: new poll mode driver")
> Fixes: 97ac72aa71a9 ("i40e: support setting VF MAC address")
> 
> Signed-off-by: David Harton <dharton@cisco.com>
> ---
> 
> v2
> * Removed multicast check in i40evf_add_mac_addr.
> 
> v1
> * Removed multicast check in i40e_validate_mac_addr.
> 
>  drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> b/drivers/net/i40e/i40e_ethdev_vf.c
> index f6d8293..5916d11 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -888,7 +888,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
>  	int err;
>  	struct vf_cmd_info args;
> 
> -	if (i40e_validate_mac_addr(addr->addr_bytes) != I40E_SUCCESS) {
> +	if (is_zero_ether_addr(addr) != I40E_SUCCESS) {

Thanks for the patch, there's some mistake with my last comment.
* @return
 *   True  (1) if the given ethernet address is filled with zeros;
 *   false (0) otherwise.
 */
According to the comment of is_zero_ether_addr function, return value should be 0 or 1.
So you should use if (!is_zero_ether_addr(addr))  here.

>  		PMD_DRV_LOG(ERR, "Invalid mac:%x:%x:%x:%x:%x:%x",
>  			    addr->addr_bytes[0], addr->addr_bytes[1],
>  			    addr->addr_bytes[2], addr->addr_bytes[3],
> --
> 2.10.3.dirty

Re: [dpdk-dev] [PATCH v2] i40e: fix i40evf_add_mac_addr to permit multicast addresses

[David Harton (dharton)]

Hi Beilei,

> -----Original Message-----
> From: Xing, Beilei [mailto:beilei.xing@intel.com]
> 
> Hi Harton,
> 
> > -----Original Message-----
> > From: David Harton [mailto:dharton@cpp-rtpbld-31.cpprtplab]
> >
> > From: David Harton <dharton@cisco.com>
> >
> > The i40e maintains a single MAC filter table for both unicast and
> > multicast addresses.  The i40e_validate_mac_addr function was
> > preventing multicast addresses from being added to the table via
> > i40evf_add_mac_addr.  Fixed the issue by adjusting the check in
> i40evf_add_mac_addr.
> >
> > Fixes: 4861cde46116 ("i40e: new poll mode driver")
> > Fixes: 97ac72aa71a9 ("i40e: support setting VF MAC address")
> >
> > Signed-off-by: David Harton <dharton@cisco.com>
> > ---
> >
> > v2
> > * Removed multicast check in i40evf_add_mac_addr.
> >
> > v1
> > * Removed multicast check in i40e_validate_mac_addr.
> >
> >  drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > b/drivers/net/i40e/i40e_ethdev_vf.c
> > index f6d8293..5916d11 100644
> > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > @@ -888,7 +888,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
> >  	int err;
> >  	struct vf_cmd_info args;
> >
> > -	if (i40e_validate_mac_addr(addr->addr_bytes) != I40E_SUCCESS) {
> > +	if (is_zero_ether_addr(addr) != I40E_SUCCESS) {
> 
> Thanks for the patch, there's some mistake with my last comment.

Actually, I think the logic above works but I do agree it is confusing
but I wanted to use your suggestion to speed up the process. :)

> * @return
>  *   True  (1) if the given ethernet address is filled with zeros;
>  *   false (0) otherwise.
>  */
> According to the comment of is_zero_ether_addr function, return value
> should be 0 or 1.
> So you should use if (!is_zero_ether_addr(addr))  here.

I don't think so.  I think the logic that should be used is
if (is_zero_ether_addr(addr)) {
because we want to report an error if the addr is all 0's.

Please let me know if you agree and I'll post this patch instead.

Thanks,
Dave

> 
> >  		PMD_DRV_LOG(ERR, "Invalid mac:%x:%x:%x:%x:%x:%x",
> >  			    addr->addr_bytes[0], addr->addr_bytes[1],
> >  			    addr->addr_bytes[2], addr->addr_bytes[3],
> > --
> > 2.10.3.dirty

Re: [dpdk-dev] [PATCH v2] i40e: fix i40evf_add_mac_addr to permit multicast addresses

[Xing, Beilei]

> -----Original Message-----
> From: David Harton (dharton) [mailto:dharton@cisco.com]
> Sent: Wednesday, September 13, 2017 10:38 AM
> To: Xing, Beilei <beilei.xing@intel.com>; David Harton <dharton@cpp-rtpbld-
> 31.cpprtplab>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v2] i40e: fix i40evf_add_mac_addr to permit multicast
> addresses
> 
> Hi Beilei,
> 
> > -----Original Message-----
> > From: Xing, Beilei [mailto:beilei.xing@intel.com]
> >
> > Hi Harton,
> >
> > > -----Original Message-----
> > > From: David Harton [mailto:dharton@cpp-rtpbld-31.cpprtplab]
> > >
> > > From: David Harton <dharton@cisco.com>
> > >
> > > The i40e maintains a single MAC filter table for both unicast and
> > > multicast addresses.  The i40e_validate_mac_addr function was
> > > preventing multicast addresses from being added to the table via
> > > i40evf_add_mac_addr.  Fixed the issue by adjusting the check in
> > i40evf_add_mac_addr.
> > >
> > > Fixes: 4861cde46116 ("i40e: new poll mode driver")
> > > Fixes: 97ac72aa71a9 ("i40e: support setting VF MAC address")
> > >
> > > Signed-off-by: David Harton <dharton@cisco.com>
> > > ---
> > >
> > > v2
> > > * Removed multicast check in i40evf_add_mac_addr.
> > >
> > > v1
> > > * Removed multicast check in i40e_validate_mac_addr.
> > >
> > >  drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > > b/drivers/net/i40e/i40e_ethdev_vf.c
> > > index f6d8293..5916d11 100644
> > > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > > @@ -888,7 +888,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
> > >  	int err;
> > >  	struct vf_cmd_info args;
> > >
> > > -	if (i40e_validate_mac_addr(addr->addr_bytes) != I40E_SUCCESS) {
> > > +	if (is_zero_ether_addr(addr) != I40E_SUCCESS) {
> >
> > Thanks for the patch, there's some mistake with my last comment.
> 
> Actually, I think the logic above works but I do agree it is confusing but I
> wanted to use your suggestion to speed up the process. :)
> 
> > * @return
> >  *   True  (1) if the given ethernet address is filled with zeros;
> >  *   false (0) otherwise.
> >  */
> > According to the comment of is_zero_ether_addr function, return value
> > should be 0 or 1.
> > So you should use if (!is_zero_ether_addr(addr))  here.
> 
> I don't think so.  I think the logic that should be used is if
> (is_zero_ether_addr(addr)) { because we want to report an error if the addr
> is all 0's.
> 

Yes, correct:)

> Please let me know if you agree and I'll post this patch instead.
> 
> Thanks,
> Dave
> 
> >
> > >  		PMD_DRV_LOG(ERR, "Invalid mac:%x:%x:%x:%x:%x:%x",
> > >  			    addr->addr_bytes[0], addr->addr_bytes[1],
> > >  			    addr->addr_bytes[2], addr->addr_bytes[3],
> > > --
> > > 2.10.3.dirty

[dpdk-dev] [PATCH v3] i40e: fix i40evf_add_mac_addr to permit multicast addresses

[David Harton]

From: David Harton <dharton@cisco.com>

The i40e maintains a single MAC filter table for both
unicast and multicast addresses.  The i40e_validate_mac_addr
function was preventing multicast addresses from being added
to the table via i40evf_add_mac_addr.  Fixed the issue by
adjusting the check in i40evf_add_mac_addr.

Fixes: 4861cde46116 ("i40e: new poll mode driver")
Fixes: 97ac72aa71a9 ("i40e: support setting VF MAC address")

Signed-off-by: David Harton <dharton@cisco.com>
---

v3
* Modified diff to use more conventional logic.

v2
* Removed multicast check in i40evf_add_mac_addr.

v1
* Removed multicast check in i40e_validate_mac_addr.

 drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index f6d8293..08afd9e 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -888,7 +888,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
 	int err;
 	struct vf_cmd_info args;
 
-	if (i40e_validate_mac_addr(addr->addr_bytes) != I40E_SUCCESS) {
+	if (is_zero_ether_addr(addr)) {
 		PMD_DRV_LOG(ERR, "Invalid mac:%x:%x:%x:%x:%x:%x",
 			    addr->addr_bytes[0], addr->addr_bytes[1],
 			    addr->addr_bytes[2], addr->addr_bytes[3],
-- 
2.10.3.dirty

Re: [dpdk-dev] [PATCH v3] i40e: fix i40evf_add_mac_addr to permit multicast addresses

[Xing, Beilei]

> -----Original Message-----
> From: David Harton [mailto:dharton@cpp-rtpbld-31.cpprtplab]
> Sent: Wednesday, September 13, 2017 11:21 AM
> To: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; David Harton <dharton@cisco.com>
> Subject: [PATCH v3] i40e: fix i40evf_add_mac_addr to permit multicast
> addresses
> 
> From: David Harton <dharton@cisco.com>
> 
> The i40e maintains a single MAC filter table for both unicast and multicast
> addresses.  The i40e_validate_mac_addr function was preventing multicast
> addresses from being added to the table via i40evf_add_mac_addr.  Fixed
> the issue by adjusting the check in i40evf_add_mac_addr.
> 
> Fixes: 4861cde46116 ("i40e: new poll mode driver")
> Fixes: 97ac72aa71a9 ("i40e: support setting VF MAC address")
> 
> Signed-off-by: David Harton <dharton@cisco.com>

Acked-by: Beilei Xing <beilei.xing@intel.com>

Re: [dpdk-dev] [PATCH v3] i40e: fix i40evf_add_mac_addr to permit multicast addresses

[Ferruh Yigit]

On 9/13/2017 4:42 AM, Xing, Beilei wrote:
> 
> 
>> -----Original Message-----
>> From: David Harton [mailto:dharton@cpp-rtpbld-31.cpprtplab]
>> Sent: Wednesday, September 13, 2017 11:21 AM
>> To: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
>> Cc: dev@dpdk.org; David Harton <dharton@cisco.com>
>> Subject: [PATCH v3] i40e: fix i40evf_add_mac_addr to permit multicast
>> addresses
>>
>> From: David Harton <dharton@cisco.com>
>>
>> The i40e maintains a single MAC filter table for both unicast and multicast
>> addresses.  The i40e_validate_mac_addr function was preventing multicast
>> addresses from being added to the table via i40evf_add_mac_addr.  Fixed
>> the issue by adjusting the check in i40evf_add_mac_addr.
>>
>> Fixes: 4861cde46116 ("i40e: new poll mode driver")
>> Fixes: 97ac72aa71a9 ("i40e: support setting VF MAC address")
>>
>> Signed-off-by: David Harton <dharton@cisco.com>
> 
> Acked-by: Beilei Xing <beilei.xing@intel.com>

Applied to dpdk-next-net/master, thanks.