Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong ICMPv6 checksum with passthru MACVLAN attached #109

Open
tomty89 opened this issue Oct 9, 2023 · 0 comments
Open

Wrong ICMPv6 checksum with passthru MACVLAN attached #109

tomty89 opened this issue Oct 9, 2023 · 0 comments

Comments

@tomty89
Copy link

tomty89 commented Oct 9, 2023

I have a USB WLAN NIC (a 0411:0242 by Buffalo) that recently I would like to pass to an nspawn container. Since the NIC itself cannot be moved into another netns, I create a passthru mode MACVLAN on top of it for the goal.

Everything seems to work fine with IPv4 (and ARP), but then I notice that IPv6 (LL/ULA only; so nothing to do with the router) does not work for some reason. After some tcpdump -X and some other tests, I notice that the problem is ICMP(v6)-specific and has something to do with the ICMP checksum, and is specific to this WLAN NIC. (Well I mean like, this driver, not limited to this fork though.)

After a quick search on github I found some seemingly related code:

struct icmp6hdr *hdr = (struct icmp6hdr *)(skb->data + ETH_HLEN + sizeof(*iph));
hdr->icmp6_cksum = 0;
hdr->icmp6_cksum = csum_ipv6_magic(&iph->saddr, &iph->daddr,
iph->payload_len,
IPPROTO_ICMPV6,
csum_partial((__u8 *)hdr, iph->payload_len, 0));

Then I rebuild the driver with the code above commented, ICMPv6 starts to work.

I have no idea what "NAT25" is, or under what circumstances this actually needs to be called (and hence the checksum needs to be updated):

static int update_nd_link_layer_addr(unsigned char *data, int len, unsigned char *replace_mac)

(In the case of passthru MACVLAN, the MAC address of the MACVLAN is at least by default the same as its underlying "link"; and at least in the case of WiFi, AFAIK we cannot really change MAC after SSID aasociation.)

The bottom line is, AFAICT the code won't come up with correct checksum anyway, so it seems to me that it is more or less safe to be commented out? (The worst case would be, AFAICT, the checksum is not updated to a correct value, while this code seems to give a wrong checksum even when the correct value has not changed.)

Certainly it would be wonderful if you know what's exactly wrong in the code and we can just have it fixed instead.

Btw even with MAC changing before association (e.g. per-network MAC randomization done by iwd), things seem to work fine as well with the code commented out.

Also ICMPv6 works fine if the NIC is used without any MACVLAN. Together with the name rtw_br_ext.c, I wonder if this piece of code is some kind of broken hack that allows the NIC to be bridged even when working in station mode?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant