29.11.2015 10:02, Andrei Borzenkov пишет:
> 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 <jba...@fb.com>
>> ---
>> 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)
Should not we fall back to promiscuous in this case as the last resort?
>> + || !(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.
>
Actually I think it should simply build filter completely every time
from addresses (interfaces) on this card. This automatically gives us
duplicates elimination as well as correct handling of removed 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;
>>
>
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
