Re: [PATCH edk2-platforms 1/1] Silicon/Broadcom/BcmGenetDxe: fix multicast handling

["Andrei Warkentin" <awarkentin@vmware.com>]

Hi Ard,

Could we upgrade broadcast to promisc? The spec allows using a “wider” filter.

Could we treat multicast the same way? So any non-unicast filter means promisc?

A

> 9 мая 2020 г., в 6:06 AM, Ard Biesheuvel <Ard.Biesheuvel@arm.com> написал(а):
> 
> Move the handling of the promiscuous receive control to the SNP
> ReceiveFilters() method where it belongs. Given that we do not
> support multicast filtering, only minimal handling is required to
> test the promiscuous bit and program the MAC accordingly. Any
> multicast handling will be done by the MNP layer above it.
> 
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Jared McNeill <jmcneill@invisible.ca>
> Cc: Andrei Warkentin <awarkentin@vmware.com>
> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c |  7 +++----
> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c | 17 ++++++-----------
> 2 files changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
> index e3d015dd0820..a6102421cc26 100644
> --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
> @@ -154,11 +154,10 @@ GenetDriverBindingStart (
>   Genet->SnpMode.NvRamSize              = 0;
>   Genet->SnpMode.NvRamAccessSize        = 0;
>   Genet->SnpMode.ReceiveFilterMask      = EFI_SIMPLE_NETWORK_RECEIVE_UNICAST |
> -                                          EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST |
>                                           EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST |
> -                                          EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS |
> -                                          EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST;
> -  Genet->SnpMode.ReceiveFilterSetting   = Genet->SnpMode.ReceiveFilterMask;
> +                                          EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS;
> +  Genet->SnpMode.ReceiveFilterSetting   = EFI_SIMPLE_NETWORK_RECEIVE_UNICAST |
> +                                          EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST;
>   Genet->SnpMode.MaxMCastFilterCount    = 0;
>   Genet->SnpMode.MCastFilterCount       = 0;
>   Genet->SnpMode.IfType                 = NET_IFTYPE_ETHERNET;
> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
> index bf28448445d1..9ea153780538 100644
> --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
> @@ -148,10 +148,6 @@ GenetSimpleNetworkInitialize (
>   }
> 
>   GenetSetMacAddress (Genet, &Genet->SnpMode.CurrentAddress);
> -  /*
> -   * TODO: this belongs in GenetSimpleNetworkReceiveFilters, not here.
> -   */
> -  GenetSetPromisc (Genet, TRUE);
> 
>   GenetDmaInitRings (Genet);
> 
> @@ -306,6 +302,10 @@ GenetSimpleNetworkReceiveFilters (
>   }
> 
>   Genet = GENET_PRIVATE_DATA_FROM_SNP_THIS (This);
> +  if (((Enable | Disable) & ~Genet->SnpMode.ReceiveFilterMask) != 0 ||
> +      (!ResetMCastFilter && MCastFilterCnt > Genet->SnpMode.MaxMCastFilterCount)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
>   if (Genet->SnpMode.State == EfiSimpleNetworkStopped) {
>     return EFI_NOT_STARTED;
>   }
> @@ -313,13 +313,8 @@ GenetSimpleNetworkReceiveFilters (
>     return EFI_DEVICE_ERROR;
>   }
> 
> -  //
> -  // Can't actually return EFI_UNSUPPORTED, so just ignore the filters
> -  // (we set promiscuous mode ON inside GenetSimpleNetworkInitialize).
> -  //
> -  // TODO: move promisc handling here.
> -  // TODO 2: support multicast/broadcast.
> -  //
> +  GenetSetPromisc (Genet,
> +    (Enable & ~Disable & EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS) != 0);
> 
>   return EFI_SUCCESS;
> }
> -- 
> 2.17.1
>