From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Martin Willi <martin@strongswan.org>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Jesper Dangaard Brouer <hawk@kernel.org>,
John Fastabend <john.fastabend@gmail.com>
Cc: netdev@vger.kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH net-next] net: xdp: Update pkt_type if generic XDP changes unicast MAC
Date: Mon, 19 Apr 2021 16:53:20 +0200 [thread overview]
Message-ID: <87tuo2gwbj.fsf@toke.dk> (raw)
In-Reply-To: <20210419141559.8611-1-martin@strongswan.org>
Martin Willi <martin@strongswan.org> writes:
> If a generic XDP program changes the destination MAC address from/to
> multicast/broadcast, the skb->pkt_type is updated to properly handle
> the packet when passed up the stack. When changing the MAC from/to
> the NICs MAC, PACKET_HOST/OTHERHOST is not updated, though, making
> the behavior different from that of native XDP.
>
> Remember the PACKET_HOST/OTHERHOST state before calling the program
> in generic XDP, and update pkt_type accordingly if the destination
> MAC address has changed. As eth_type_trans() assumes a default
> pkt_type of PACKET_HOST, restore that before calling it.
>
> The use case for this is when a XDP program wants to push received
> packets up the stack by rewriting the MAC to the NICs MAC, for
> example by cluster nodes sharing MAC addresses.
>
> Fixes: 297249569932 ("net: fix generic XDP to handle if eth header was mangled")
> Signed-off-by: Martin Willi <martin@strongswan.org>
> ---
> net/core/dev.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index d9bf63dbe4fd..eed028aec6a4 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4723,10 +4723,10 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
> void *orig_data, *orig_data_end, *hard_start;
> struct netdev_rx_queue *rxqueue;
> u32 metalen, act = XDP_DROP;
> + bool orig_bcast, orig_host;
> u32 mac_len, frame_sz;
> __be16 orig_eth_type;
> struct ethhdr *eth;
> - bool orig_bcast;
> int off;
>
> /* Reinjected packets coming from act_mirred or similar should
> @@ -4773,6 +4773,7 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
> orig_data_end = xdp->data_end;
> orig_data = xdp->data;
> eth = (struct ethhdr *)xdp->data;
> + orig_host = ether_addr_equal_64bits(eth->h_dest, skb->dev->dev_addr);
ether_addr_equal_64bits() seems to assume that the addresses passed to
it are padded to be 8 bytes long, which is not the case for eth->h_dest.
AFAICT the only reason the _64bits variant works for multicast is that
it happens to be only checking the top-most bit, but unless I'm missing
something you'll have to use the boring old ether_addr_equal() here, no?
> orig_bcast = is_multicast_ether_addr_64bits(eth->h_dest);
> orig_eth_type = eth->h_proto;
>
> @@ -4800,8 +4801,11 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
> /* check if XDP changed eth hdr such SKB needs update */
> eth = (struct ethhdr *)xdp->data;
> if ((orig_eth_type != eth->h_proto) ||
> + (orig_host != ether_addr_equal_64bits(eth->h_dest,
> + skb->dev->dev_addr)) ||
> (orig_bcast != is_multicast_ether_addr_64bits(eth->h_dest))) {
> __skb_push(skb, ETH_HLEN);
> + skb->pkt_type = PACKET_HOST;
> skb->protocol = eth_type_trans(skb, skb->dev);
> }
Okay, so this was a bit confusing to me at fist glance: eth_type_trans()
will reset the type, but not back to PACKET_HOST. So this works, just a
bit confusing :)
-Toke
next prev parent reply other threads:[~2021-04-19 14:54 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-19 14:15 [PATCH net-next] net: xdp: Update pkt_type if generic XDP changes unicast MAC Martin Willi
2021-04-19 14:53 ` Toke Høiland-Jørgensen [this message]
2021-04-20 5:15 ` Martin Willi
2021-04-20 9:35 ` Toke Høiland-Jørgensen
2021-04-22 21:30 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87tuo2gwbj.fsf@toke.dk \
--to=toke@redhat.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=hawk@kernel.org \
--cc=john.fastabend@gmail.com \
--cc=kuba@kernel.org \
--cc=martin@strongswan.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).