diff mbox series

[v5,bpf-next,3/9] veth: Avoid drops by oversized packets when XDP is enabled

Message ID 20180726144032.2116-4-toshiaki.makita1@gmail.com
State Changes Requested, archived
Delegated to: BPF Maintainers
Headers show
Series veth: Driver XDP | expand

Commit Message

Toshiaki Makita July 26, 2018, 2:40 p.m. UTC 
From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>

All oversized packets including GSO packets are dropped if XDP is
enabled on receiver side, so don't send such packets from peer.

Drop TSO and SCTP fragmentation features so that veth devices themselves
segment packets with XDP enabled. Also cap MTU accordingly.

v4:
- Don't auto-adjust MTU but cap max MTU.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 drivers/net/veth.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

Comments

Jakub Kicinski July 27, 2018, 12:51 a.m. UTC | #1 
On Thu, 26 Jul 2018 23:40:26 +0900, Toshiaki Makita wrote:
> +		max_mtu = PAGE_SIZE - VETH_XDP_HEADROOM -
> +			  peer->hard_header_len -
> +			  SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> +		if (peer->mtu > max_mtu) {
> +			NL_SET_ERR_MSG_MOD(extack, "Peer MTU is too large to set XDP");
> +			err = -ERANGE;
> +			goto err;
> +		}

You need to add .ndo_change_mtu and check this condition there too.
Toshiaki Makita July 27, 2018, 1:06 a.m. UTC | #2 
On 2018/07/27 9:51, Jakub Kicinski wrote:
> On Thu, 26 Jul 2018 23:40:26 +0900, Toshiaki Makita wrote:
>> +		max_mtu = PAGE_SIZE - VETH_XDP_HEADROOM -
>> +			  peer->hard_header_len -
>> +			  SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
>> +		if (peer->mtu > max_mtu) {
>> +			NL_SET_ERR_MSG_MOD(extack, "Peer MTU is too large to set XDP");
>> +			err = -ERANGE;
>> +			goto err;
>> +		}
> 
> You need to add .ndo_change_mtu and check this condition there too.

I'm setting peer->max_mtu so no need to add .ndo_change_mtu.
Inappropriate MTU will be refused in dev_set_mtu().
Jakub Kicinski July 27, 2018, 1:08 a.m. UTC | #3 
On Fri, 27 Jul 2018 10:06:41 +0900, Toshiaki Makita wrote:
> On 2018/07/27 9:51, Jakub Kicinski wrote:
> > On Thu, 26 Jul 2018 23:40:26 +0900, Toshiaki Makita wrote:  
> >> +		max_mtu = PAGE_SIZE - VETH_XDP_HEADROOM -
> >> +			  peer->hard_header_len -
> >> +			  SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> >> +		if (peer->mtu > max_mtu) {
> >> +			NL_SET_ERR_MSG_MOD(extack, "Peer MTU is too large to set XDP");
> >> +			err = -ERANGE;
> >> +			goto err;
> >> +		}  
> > 
> > You need to add .ndo_change_mtu and check this condition there too.  
> 
> I'm setting peer->max_mtu so no need to add .ndo_change_mtu.
> Inappropriate MTU will be refused in dev_set_mtu().

missed that, sorry
diff mbox series

Patch

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 78fa08cb6e24..1b4006d3df32 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -542,6 +542,23 @@  static int veth_get_iflink(const struct net_device *dev)
 	return iflink;
 }
 
+static netdev_features_t veth_fix_features(struct net_device *dev,
+					   netdev_features_t features)
+{
+	struct veth_priv *priv = netdev_priv(dev);
+	struct net_device *peer;
+
+	peer = rtnl_dereference(priv->peer);
+	if (peer) {
+		struct veth_priv *peer_priv = netdev_priv(peer);
+
+		if (peer_priv->_xdp_prog)
+			features &= ~NETIF_F_GSO_SOFTWARE;
+	}
+
+	return features;
+}
+
 static void veth_set_rx_headroom(struct net_device *dev, int new_hr)
 {
 	struct veth_priv *peer_priv, *priv = netdev_priv(dev);
@@ -571,6 +588,7 @@  static int veth_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 	struct veth_priv *priv = netdev_priv(dev);
 	struct bpf_prog *old_prog;
 	struct net_device *peer;
+	unsigned int max_mtu;
 	int err;
 
 	old_prog = priv->_xdp_prog;
@@ -584,6 +602,15 @@  static int veth_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 			goto err;
 		}
 
+		max_mtu = PAGE_SIZE - VETH_XDP_HEADROOM -
+			  peer->hard_header_len -
+			  SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+		if (peer->mtu > max_mtu) {
+			NL_SET_ERR_MSG_MOD(extack, "Peer MTU is too large to set XDP");
+			err = -ERANGE;
+			goto err;
+		}
+
 		if (dev->flags & IFF_UP) {
 			err = veth_enable_xdp(dev);
 			if (err) {
@@ -591,14 +618,29 @@  static int veth_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 				goto err;
 			}
 		}
+
+		if (!old_prog) {
+			peer->hw_features &= ~NETIF_F_GSO_SOFTWARE;
+			peer->max_mtu = max_mtu;
+		}
 	}
 
 	if (old_prog) {
-		if (!prog && dev->flags & IFF_UP)
-			veth_disable_xdp(dev);
+		if (!prog) {
+			if (dev->flags & IFF_UP)
+				veth_disable_xdp(dev);
+
+			if (peer) {
+				peer->hw_features |= NETIF_F_GSO_SOFTWARE;
+				peer->max_mtu = ETH_MAX_MTU;
+			}
+		}
 		bpf_prog_put(old_prog);
 	}
 
+	if ((!!old_prog ^ !!prog) && peer)
+		netdev_update_features(peer);
+
 	return 0;
 err:
 	priv->_xdp_prog = old_prog;
@@ -643,6 +685,7 @@  static const struct net_device_ops veth_netdev_ops = {
 	.ndo_poll_controller	= veth_poll_controller,
 #endif
 	.ndo_get_iflink		= veth_get_iflink,
+	.ndo_fix_features	= veth_fix_features,
 	.ndo_features_check	= passthru_features_check,
 	.ndo_set_rx_headroom	= veth_set_rx_headroom,
 	.ndo_bpf		= veth_xdp,