Re: [PATCH v2 10/10] net: cadence_gem: Fix RX address filtering

["Edgar E. Iglesias" <edgar.iglesias@gmail.com>]

On Mon, May 04, 2020 at 07:36:08PM +0530, Sai Pavan Boddu wrote:
> From: Tong Ho <tong.ho@xilinx.com>
> 
> Two defects are fixed:
> 
> 1/ Detection of multicast frames
> 2/ Treating drop of mis-addressed frames as non-error


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> 
> Signed-off-by: Tong Ho <tong.ho@xilinx.com>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> ---
>  hw/net/cadence_gem.c | 26 +++++++++++---------------
>  1 file changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index f0bf2e7..112794a 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -34,6 +34,7 @@
>  #include "qemu/module.h"
>  #include "sysemu/dma.h"
>  #include "net/checksum.h"
> +#include "net/eth.h"
>  
>  #define CADENCE_GEM_ERR_DEBUG 0
>  #define DB_PRINT(...) do {\
> @@ -669,7 +670,7 @@ static unsigned calc_mac_hash(const uint8_t *mac)
>  static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
>  {
>      uint8_t *gem_spaddr;
> -    int i;
> +    int i, is_mc;
>  
>      /* Promiscuous mode? */
>      if (s->regs[GEM_NWCFG] & GEM_NWCFG_PROMISC) {
> @@ -685,22 +686,17 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
>      }
>  
>      /* Accept packets -w- hash match? */
> -    if ((packet[0] == 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> -        (packet[0] != 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> +    is_mc = is_multicast_ether_addr(packet);
> +    if ((is_mc && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> +        (!is_mc && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> +        uint64_t buckets;
>          unsigned hash_index;
>  
>          hash_index = calc_mac_hash(packet);
> -        if (hash_index < 32) {
> -            if (s->regs[GEM_HASHLO] & (1<<hash_index)) {
> -                return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> -                                           GEM_RX_UNICAST_HASH_ACCEPT;
> -            }
> -        } else {
> -            hash_index -= 32;
> -            if (s->regs[GEM_HASHHI] & (1<<hash_index)) {
> -                return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> -                                           GEM_RX_UNICAST_HASH_ACCEPT;
> -            }
> +        buckets = ((uint64_t)s->regs[GEM_HASHHI] << 32) | s->regs[GEM_HASHLO];
> +        if ((buckets >> hash_index) & 1) {
> +            return is_mc ? GEM_RX_MULTICAST_HASH_ACCEPT
> +                         : GEM_RX_UNICAST_HASH_ACCEPT;
>          }
>      }
>  
> @@ -924,7 +920,7 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>      /* Is this destination MAC address "for us" ? */
>      maf = gem_mac_address_filter(s, buf);
>      if (maf == GEM_RX_REJECT) {
> -        return -1;
> +        return size;  /* no, drop siliently b/c it's not an error */
>      }
>  
>      /* Discard packets with receive length error enabled ? */
> -- 
> 2.7.4
>