Skip to content

Commit

Permalink
netkit: Fix pkt_type override upon netkit pass verdict
Browse files Browse the repository at this point in the history
When running Cilium connectivity test suite with netkit in L2 mode, we
found that compared to tcx a few tests were failing which pushed traffic
into an L7 proxy sitting in host namespace. The problem in particular is
around the invocation of eth_type_trans() in netkit.

In case of tcx, this is run before the tcx ingress is triggered inside
host namespace and thus if the BPF program uses the bpf_skb_change_type()
helper the newly set type is retained. However, in case of netkit, the
late eth_type_trans() invocation overrides the earlier decision from the
BPF program which eventually leads to the test failure.

Instead of eth_type_trans(), split out the relevant parts, meaning, reset
of mac header and call to eth_skb_pkt_type() before the BPF program is run
in order to have the same behavior as with tcx, and refactor a small helper
called eth_skb_pull_mac() which is run in case it's passed up the stack
where the mac header must be pulled. With this all connectivity tests pass.

Fixes: 35dfaad ("netkit, bpf: Add bpf programmable net device")
Signed-off-by: Daniel Borkmann <[email protected]>
Acked-by: Nikolay Aleksandrov <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Alexei Starovoitov <[email protected]>
  • Loading branch information
borkmann authored and Alexei Starovoitov committed May 25, 2024
1 parent d6fe532 commit 3998d18
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 4 deletions.
4 changes: 3 additions & 1 deletion drivers/net/netkit.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ static void netkit_prep_forward(struct sk_buff *skb, bool xnet)
skb_scrub_packet(skb, xnet);
skb->priority = 0;
nf_skip_egress(skb, true);
skb_reset_mac_header(skb);
}

static struct netkit *netkit_priv(const struct net_device *dev)
Expand All @@ -78,14 +79,15 @@ static netdev_tx_t netkit_xmit(struct sk_buff *skb, struct net_device *dev)
skb_orphan_frags(skb, GFP_ATOMIC)))
goto drop;
netkit_prep_forward(skb, !net_eq(dev_net(dev), dev_net(peer)));
eth_skb_pkt_type(skb, peer);
skb->dev = peer;
entry = rcu_dereference(nk->active);
if (entry)
ret = netkit_run(entry, skb, ret);
switch (ret) {
case NETKIT_NEXT:
case NETKIT_PASS:
skb->protocol = eth_type_trans(skb, skb->dev);
eth_skb_pull_mac(skb);
skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
if (likely(__netif_rx(skb) == NET_RX_SUCCESS)) {
dev_sw_netstats_tx_add(dev, 1, len);
Expand Down
8 changes: 8 additions & 0 deletions include/linux/etherdevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,14 @@ static inline void eth_skb_pkt_type(struct sk_buff *skb,
}
}

static inline struct ethhdr *eth_skb_pull_mac(struct sk_buff *skb)
{
struct ethhdr *eth = (struct ethhdr *)skb->data;

skb_pull_inline(skb, ETH_HLEN);
return eth;
}

/**
* eth_skb_pad - Pad buffer to mininum number of octets for Ethernet frame
* @skb: Buffer to pad
Expand Down
4 changes: 1 addition & 3 deletions net/ethernet/eth.c
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,7 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev)
skb->dev = dev;
skb_reset_mac_header(skb);

eth = (struct ethhdr *)skb->data;
skb_pull_inline(skb, ETH_HLEN);

eth = eth_skb_pull_mac(skb);
eth_skb_pkt_type(skb, dev);

/*
Expand Down

0 comments on commit 3998d18

Please sign in to comment.