Skip to content

Commit

Permalink
vxlan: calculate correct header length for GPE
Browse files Browse the repository at this point in the history
VXLAN-GPE does not add an extra inner Ethernet header. Take that into
account when calculating header length.

This causes problems in skb_tunnel_check_pmtu, where incorrect PMTU is
cached.

In the collect_md mode (which is the only mode that VXLAN-GPE
supports), there's no magic auto-setting of the tunnel interface MTU.
It can't be, since the destination and thus the underlying interface
may be different for each packet.

So, the administrator is responsible for setting the correct tunnel
interface MTU. Apparently, the administrators are capable enough to
calculate that the maximum MTU for VXLAN-GPE is (their_lower_MTU - 36).
They set the tunnel interface MTU to 1464. If you run a TCP stream over
such interface, it's then segmented according to the MTU 1464, i.e.
producing 1514 bytes frames. Which is okay, this still fits the lower
MTU.

However, skb_tunnel_check_pmtu (called from vxlan_xmit_one) uses 50 as
the header size and thus incorrectly calculates the frame size to be
1528. This leads to ICMP too big message being generated (locally),
PMTU of 1450 to be cached and the TCP stream to be resegmented.

The fix is to use the correct actual header size, especially for
skb_tunnel_check_pmtu calculation.

Fixes: e1e5314 ("vxlan: implement GPE")
Signed-off-by: Jiri Benc <jbenc@redhat.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Jiri Benc authored and davem330 committed Jul 24, 2023
1 parent a071c6a commit 94d166c
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 18 deletions.
2 changes: 1 addition & 1 deletion drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -8479,7 +8479,7 @@ static void ixgbe_atr(struct ixgbe_ring *ring,
struct ixgbe_adapter *adapter = q_vector->adapter;

if (unlikely(skb_tail_pointer(skb) < hdr.network +
VXLAN_HEADROOM))
vxlan_headroom(0)))
return;

/* verify the port is recognized as VXLAN */
Expand Down
23 changes: 10 additions & 13 deletions drivers/net/vxlan/vxlan_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -2516,7 +2516,7 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
}

ndst = &rt->dst;
err = skb_tunnel_check_pmtu(skb, ndst, VXLAN_HEADROOM,
err = skb_tunnel_check_pmtu(skb, ndst, vxlan_headroom(flags & VXLAN_F_GPE),
netif_is_any_bridge_port(dev));
if (err < 0) {
goto tx_error;
Expand Down Expand Up @@ -2577,7 +2577,8 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
goto out_unlock;
}

err = skb_tunnel_check_pmtu(skb, ndst, VXLAN6_HEADROOM,
err = skb_tunnel_check_pmtu(skb, ndst,
vxlan_headroom((flags & VXLAN_F_GPE) | VXLAN_F_IPV6),
netif_is_any_bridge_port(dev));
if (err < 0) {
goto tx_error;
Expand Down Expand Up @@ -2989,14 +2990,12 @@ static int vxlan_change_mtu(struct net_device *dev, int new_mtu)
struct vxlan_rdst *dst = &vxlan->default_dst;
struct net_device *lowerdev = __dev_get_by_index(vxlan->net,
dst->remote_ifindex);
bool use_ipv6 = !!(vxlan->cfg.flags & VXLAN_F_IPV6);

/* This check is different than dev->max_mtu, because it looks at
* the lowerdev->mtu, rather than the static dev->max_mtu
*/
if (lowerdev) {
int max_mtu = lowerdev->mtu -
(use_ipv6 ? VXLAN6_HEADROOM : VXLAN_HEADROOM);
int max_mtu = lowerdev->mtu - vxlan_headroom(vxlan->cfg.flags);
if (new_mtu > max_mtu)
return -EINVAL;
}
Expand Down Expand Up @@ -3644,11 +3643,11 @@ static void vxlan_config_apply(struct net_device *dev,
struct vxlan_dev *vxlan = netdev_priv(dev);
struct vxlan_rdst *dst = &vxlan->default_dst;
unsigned short needed_headroom = ETH_HLEN;
bool use_ipv6 = !!(conf->flags & VXLAN_F_IPV6);
int max_mtu = ETH_MAX_MTU;
u32 flags = conf->flags;

if (!changelink) {
if (conf->flags & VXLAN_F_GPE)
if (flags & VXLAN_F_GPE)
vxlan_raw_setup(dev);
else
vxlan_ether_setup(dev);
Expand All @@ -3673,8 +3672,7 @@ static void vxlan_config_apply(struct net_device *dev,

dev->needed_tailroom = lowerdev->needed_tailroom;

max_mtu = lowerdev->mtu - (use_ipv6 ? VXLAN6_HEADROOM :
VXLAN_HEADROOM);
max_mtu = lowerdev->mtu - vxlan_headroom(flags);
if (max_mtu < ETH_MIN_MTU)
max_mtu = ETH_MIN_MTU;

Expand All @@ -3685,10 +3683,9 @@ static void vxlan_config_apply(struct net_device *dev,
if (dev->mtu > max_mtu)
dev->mtu = max_mtu;

if (use_ipv6 || conf->flags & VXLAN_F_COLLECT_METADATA)
needed_headroom += VXLAN6_HEADROOM;
else
needed_headroom += VXLAN_HEADROOM;
if (flags & VXLAN_F_COLLECT_METADATA)
flags |= VXLAN_F_IPV6;
needed_headroom += vxlan_headroom(flags);
dev->needed_headroom = needed_headroom;

memcpy(&vxlan->cfg, conf, sizeof(*conf));
Expand Down
13 changes: 9 additions & 4 deletions include/net/vxlan.h
Original file line number Diff line number Diff line change
Expand Up @@ -386,10 +386,15 @@ static inline netdev_features_t vxlan_features_check(struct sk_buff *skb,
return features;
}

/* IP header + UDP + VXLAN + Ethernet header */
#define VXLAN_HEADROOM (20 + 8 + 8 + 14)
/* IPv6 header + UDP + VXLAN + Ethernet header */
#define VXLAN6_HEADROOM (40 + 8 + 8 + 14)
static inline int vxlan_headroom(u32 flags)
{
/* VXLAN: IP4/6 header + UDP + VXLAN + Ethernet header */
/* VXLAN-GPE: IP4/6 header + UDP + VXLAN */
return (flags & VXLAN_F_IPV6 ? sizeof(struct ipv6hdr) :
sizeof(struct iphdr)) +
sizeof(struct udphdr) + sizeof(struct vxlanhdr) +
(flags & VXLAN_F_GPE ? 0 : ETH_HLEN);
}

static inline struct vxlanhdr *vxlan_hdr(struct sk_buff *skb)
{
Expand Down

0 comments on commit 94d166c

Please sign in to comment.