[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] efinet: filter multicast traffic based on addresses
From: |
Andrei Borzenkov |
Subject: |
Re: [PATCH] efinet: filter multicast traffic based on addresses |
Date: |
Sun, 29 Nov 2015 10:02:05 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
17.11.2015 21:35, Josef Bacik пишет:
> We have some hardware that claims to support PROMISCUOUS_MULTICAST but doesn't
> actually work. Instead utilize the multicast filters and specifically enable
> the multicast traffic we care about. In reality we only care about ipv6
> multicast traffic but enable ipv4 multicast as well just in case. Whenever we
> add a new address to the card we calculate the solicited node multicast
> address
> to the multicast filter. With this patch my broken hardware is still broken
> but
> functional. Thanks,
>
> Signed-off-by: Josef Bacik <address@hidden>
> ---
> grub-core/net/drivers/efi/efinet.c | 84
> ++++++++++++++++++++++++++++++++++----
> grub-core/net/net.c | 2 +
> include/grub/net.h | 54 ++++++++++++------------
> 3 files changed, 105 insertions(+), 35 deletions(-)
>
> diff --git a/grub-core/net/drivers/efi/efinet.c
> b/grub-core/net/drivers/efi/efinet.c
> index c8f80a1..bbbadd2 100644
> --- a/grub-core/net/drivers/efi/efinet.c
> +++ b/grub-core/net/drivers/efi/efinet.c
> @@ -23,6 +23,7 @@
> #include <grub/efi/api.h>
> #include <grub/efi/efi.h>
> #include <grub/i18n.h>
> +#include <grub/net/ip.h>
>
> GRUB_MOD_LICENSE ("GPLv3+");
>
> @@ -183,8 +184,9 @@ open_card (struct grub_net_card *dev)
> We need unicast and broadcast and additionaly all nodes and
> solicited multicast for IPv6. Solicited multicast is per-IPv6
> address and we currently do not have API to do it so simply
This comment becomes wrong now after your patch
> - try to enable receive of all multicast packets or evertyhing in
> - the worst case (i386 PXE driver always enables promiscuous too).
> + enable the all node addresses and the link local address. We do this
> + because some firmware has been found to not do promiscuous multicast
> + mode properly.
>
> This does trust firmware to do what it claims to do.
> */
> @@ -192,14 +194,25 @@ open_card (struct grub_net_card *dev)
> {
> grub_uint32_t filters = GRUB_EFI_SIMPLE_NETWORK_RECEIVE_UNICAST |
> GRUB_EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST |
> -
> GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST;
> + GRUB_EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST;
> + grub_efi_status_t st;
> + grub_efi_mac_address_t mac_filter[2] = {
> + { 0x1, 0, 0x5e, 0, 0, 1, },
Why this one? Where is it defined?
> + { 0x33, 0x33, 0, 0, 0, 1, },};
>
> filters &= net->mode->receive_filter_mask;
> - if (!(filters &
> GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST))
> - filters |= (net->mode->receive_filter_mask &
> - GRUB_EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS);
> -
> - efi_call_6 (net->receive_filters, net, filters, 0, 0, 0, NULL);
> + if (net->mode->max_mcast_filter_count < 2)
> + filters &= ~GRUB_EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST;
> +
> + if (filters & GRUB_EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST)
> + st = efi_call_6 (net->receive_filters, net, filters, 0, 0, 2,
> + mac_filter);
> + else
> + st = efi_call_6 (net->receive_filters, net, filters, 0, 0, 0,
> + NULL);
I think we still should attempt to fallback to promiscuous in this case.
It is better than giving up completely. Also grub_dprintf with current
filter mask would be good.
> + if (st != GRUB_EFI_SUCCESS)
> + grub_dprintf("efinet", "failed to set receive filters %u, %u\n",
> + (unsigned)filters, (unsigned)st);
> }
>
> efi_call_4 (grub_efi_system_table->boot_services->close_protocol,
> @@ -212,6 +225,58 @@ open_card (struct grub_net_card *dev)
> return GRUB_ERR_NONE;
> }
>
> +/* We only need the lower 24 bits of the address, so just take the bottom
> part
> + of the address and convert it over.
> + */
> +static void
> +solicited_node_mcast_addr_to_mac (grub_uint64_t addr,
> + grub_efi_mac_address_t mac)
> +{
> + grub_uint64_t cpu_addr = grub_be_to_cpu64(addr);
Why cannot you simply copy last 3 bytes?
> + int i, c = 0;
> +
> + /* The format is 33:33:xx:xx:xx:xx, where xx is the last 32 bits of the
> + multicast address.
> +
> + The solicited node mcast addr is in the format ff02:0:0:0:0:1:ffxx:xxxx,
> + where xx is the last 24 bits of the ipv6 address.
> + */
> + mac[0] = 0x33;
> + mac[1] = 0x33;
> + mac[2] = 0xff;
> + for (i = 3; i < 6; i++, c++)
> + mac[i] = (grub_uint8_t)((cpu_addr >> (16 - 8 * c)) & 0xff);
> +}
> +
> +static void
> +add_addr (struct grub_net_card *dev,
> + const grub_net_network_level_address_t *address)
> +{
> + grub_efi_simple_network_t *net = dev->efi_net;
> + grub_efi_mac_address_t mac_filters[16];
> + grub_efi_status_t st;
> + unsigned slot = net->mode->mcast_filter_count;
> +
> + /* We don't need to add anything for ipv4 addresses. */
> + if (address->type != GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6)
> + return;
> +
> + if ((slot >= net->mode->max_mcast_filter_count)
> + || !(GRUB_EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST &
> + net->mode->receive_filter_mask))
> + return;
> +
> + grub_memcpy(mac_filters, net->mode->mcast_filter,
> + sizeof (grub_efi_mac_address_t) * slot);
> + solicited_node_mcast_addr_to_mac (address->ipv6[1], mac_filters[slot++]);
> + st = efi_call_6 (net->receive_filters, net,
> + GRUB_EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST, 0, 0, slot,
> + mac_filters);
What about overlapping addresses (see also below)?
> + if (st != GRUB_EFI_SUCCESS)
> + grub_dprintf("efinet", "failed to add new receive filter %u\n",
> + (unsigned)st);
> +}
> +
> static void
> close_card (struct grub_net_card *dev)
> {
> @@ -228,7 +293,8 @@ static struct grub_net_card_driver efidriver =
> .open = open_card,
> .close = close_card,
> .send = send_card_buffer,
> - .recv = get_card_packet
> + .recv = get_card_packet,
> + .add_addr = add_addr,
Well ... if you add function to add filter, you should also add function
to remove filter when address is removed. And here it becomes
complicated; mapping is not 1-to-1 so we need to reference count used
multicast addresses.
> };
>
> grub_efi_handle_t
> diff --git a/grub-core/net/net.c b/grub-core/net/net.c
> index 65bea28..c4d2484 100644
> --- a/grub-core/net/net.c
> +++ b/grub-core/net/net.c
> @@ -252,6 +252,8 @@ grub_net_add_addr_real (char *name,
> inter->dhcp_ack = NULL;
> inter->dhcp_acklen = 0;
>
> + if (card->driver->add_addr)
> + card->driver->add_addr(card, addr);
> grub_net_network_level_interface_register (inter);
>
> return inter;
> diff --git a/include/grub/net.h b/include/grub/net.h
> index a1ff519..885f0be 100644
> --- a/include/grub/net.h
> +++ b/include/grub/net.h
> @@ -67,6 +67,32 @@ typedef enum grub_net_card_flags
> GRUB_NET_CARD_NO_MANUAL_INTERFACES = 2
> } grub_net_card_flags_t;
>
> +typedef enum grub_network_level_protocol_id
> +{
> + GRUB_NET_NETWORK_LEVEL_PROTOCOL_DHCP_RECV,
> + GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV4,
> + GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6
> +} grub_network_level_protocol_id_t;
> +
> +typedef enum
> +{
> + DNS_OPTION_IPV4,
> + DNS_OPTION_IPV6,
> + DNS_OPTION_PREFER_IPV4,
> + DNS_OPTION_PREFER_IPV6
> +} grub_dns_option_t;
> +
> +typedef struct grub_net_network_level_address
> +{
> + grub_network_level_protocol_id_t type;
> + union
> + {
> + grub_uint32_t ipv4;
> + grub_uint64_t ipv6[2];
> + };
> + grub_dns_option_t option;
> +} grub_net_network_level_address_t;
> +
> struct grub_net_card;
>
> struct grub_net_card_driver
> @@ -79,6 +105,8 @@ struct grub_net_card_driver
> grub_err_t (*send) (struct grub_net_card *dev,
> struct grub_net_buff *buf);
> struct grub_net_buff * (*recv) (struct grub_net_card *dev);
> + void (*add_addr) (struct grub_net_card *dev,
> + const grub_net_network_level_address_t *address);
> };
>
> typedef struct grub_net_packet
> @@ -150,32 +178,6 @@ struct grub_net_card
>
> struct grub_net_network_level_interface;
>
> -typedef enum grub_network_level_protocol_id
> -{
> - GRUB_NET_NETWORK_LEVEL_PROTOCOL_DHCP_RECV,
> - GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV4,
> - GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6
> -} grub_network_level_protocol_id_t;
> -
> -typedef enum
> -{
> - DNS_OPTION_IPV4,
> - DNS_OPTION_IPV6,
> - DNS_OPTION_PREFER_IPV4,
> - DNS_OPTION_PREFER_IPV6
> -} grub_dns_option_t;
> -
> -typedef struct grub_net_network_level_address
> -{
> - grub_network_level_protocol_id_t type;
> - union
> - {
> - grub_uint32_t ipv4;
> - grub_uint64_t ipv6[2];
> - };
> - grub_dns_option_t option;
> -} grub_net_network_level_address_t;
> -
> typedef struct grub_net_network_level_netaddress
> {
> grub_network_level_protocol_id_t type;
>