diff mbox series

[net-next,v4,3/7] dpaa_eth: limit the possible MTU range when XDP is enabled

Message ID 654d6300001825e542341bc052c31433b48b1913.1606150838.git.camelia.groza@nxp.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series dpaa_eth: add XDP support | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 60 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Camelia Alexandra Groza Nov. 23, 2020, 5:36 p.m. UTC 
Implement the ndo_change_mtu callback to prevent users from setting an
MTU that would permit processing of S/G frames. The maximum MTU size
is dependent on the buffer size.

Acked-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
Signed-off-by: Camelia Groza <camelia.groza@nxp.com>
---
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 40 ++++++++++++++++++++------
 1 file changed, 31 insertions(+), 9 deletions(-)

Comments

Fijalkowski, Maciej Nov. 24, 2020, 7:11 p.m. UTC | #1 
On Mon, Nov 23, 2020 at 07:36:21PM +0200, Camelia Groza wrote:
> Implement the ndo_change_mtu callback to prevent users from setting an
> MTU that would permit processing of S/G frames. The maximum MTU size
> is dependent on the buffer size.
> 
> Acked-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
> Signed-off-by: Camelia Groza <camelia.groza@nxp.com>
> ---
>  drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 40 ++++++++++++++++++++------
>  1 file changed, 31 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> index 8acce62..ee076f4 100644
> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> @@ -2756,23 +2756,44 @@ static int dpaa_eth_stop(struct net_device *net_dev)
>  	return err;
>  }
>  
> +static bool xdp_validate_mtu(struct dpaa_priv *priv, int mtu)
> +{
> +	int max_contig_data = priv->dpaa_bp->size - priv->rx_headroom;
> +
> +	/* We do not support S/G fragments when XDP is enabled.
> +	 * Limit the MTU in relation to the buffer size.
> +	 */
> +	if (mtu + VLAN_ETH_HLEN + ETH_FCS_LEN > max_contig_data) {

Do you support VLAN double tagging? We normally take into acount to two vlan
headers in these checks.

Other than that:
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

> +		dev_warn(priv->net_dev->dev.parent,
> +			 "The maximum MTU for XDP is %d\n",
> +			 max_contig_data - VLAN_ETH_HLEN - ETH_FCS_LEN);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +static int dpaa_change_mtu(struct net_device *net_dev, int new_mtu)
> +{
> +	struct dpaa_priv *priv = netdev_priv(net_dev);
> +
> +	if (priv->xdp_prog && !xdp_validate_mtu(priv, new_mtu))
> +		return -EINVAL;
> +
> +	net_dev->mtu = new_mtu;
> +	return 0;
> +}
> +
>  static int dpaa_setup_xdp(struct net_device *net_dev, struct bpf_prog *prog)
>  {
>  	struct dpaa_priv *priv = netdev_priv(net_dev);
>  	struct bpf_prog *old_prog;
> -	int err, max_contig_data;
> +	int err;
>  	bool up;
>  
> -	max_contig_data = priv->dpaa_bp->size - priv->rx_headroom;
> -
>  	/* S/G fragments are not supported in XDP-mode */
> -	if (prog &&
> -	    (net_dev->mtu + VLAN_ETH_HLEN + ETH_FCS_LEN > max_contig_data)) {
> -		dev_warn(net_dev->dev.parent,
> -			 "The maximum MTU for XDP is %d\n",
> -			 max_contig_data - VLAN_ETH_HLEN - ETH_FCS_LEN);
> +	if (prog && !xdp_validate_mtu(priv, net_dev->mtu))
>  		return -EINVAL;
> -	}
>  
>  	up = netif_running(net_dev);
>  
> @@ -2870,6 +2891,7 @@ static int dpaa_ioctl(struct net_device *net_dev, struct ifreq *rq, int cmd)
>  	.ndo_set_rx_mode = dpaa_set_rx_mode,
>  	.ndo_do_ioctl = dpaa_ioctl,
>  	.ndo_setup_tc = dpaa_setup_tc,
> +	.ndo_change_mtu = dpaa_change_mtu,
>  	.ndo_bpf = dpaa_xdp,
>  };
>  
> -- 
> 1.9.1
>
Camelia Alexandra Groza Nov. 25, 2020, 3:47 p.m. UTC | #2 
> -----Original Message-----
> From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Sent: Tuesday, November 24, 2020 21:12
> To: Camelia Alexandra Groza <camelia.groza@nxp.com>
> Cc: kuba@kernel.org; brouer@redhat.com; saeed@kernel.org;
> davem@davemloft.net; Madalin Bucur (OSS)
> <madalin.bucur@oss.nxp.com>; Ioana Ciornei <ioana.ciornei@nxp.com>;
> netdev@vger.kernel.org
> Subject: Re: [PATCH net-next v4 3/7] dpaa_eth: limit the possible MTU range
> when XDP is enabled
> 
> On Mon, Nov 23, 2020 at 07:36:21PM +0200, Camelia Groza wrote:
> > Implement the ndo_change_mtu callback to prevent users from setting an
> > MTU that would permit processing of S/G frames. The maximum MTU size
> > is dependent on the buffer size.
> >
> > Acked-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
> > Signed-off-by: Camelia Groza <camelia.groza@nxp.com>
> > ---
> >  drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 40
> ++++++++++++++++++++------
> >  1 file changed, 31 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > index 8acce62..ee076f4 100644
> > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > @@ -2756,23 +2756,44 @@ static int dpaa_eth_stop(struct net_device
> *net_dev)
> >  	return err;
> >  }
> >
> > +static bool xdp_validate_mtu(struct dpaa_priv *priv, int mtu)
> > +{
> > +	int max_contig_data = priv->dpaa_bp->size - priv->rx_headroom;
> > +
> > +	/* We do not support S/G fragments when XDP is enabled.
> > +	 * Limit the MTU in relation to the buffer size.
> > +	 */
> > +	if (mtu + VLAN_ETH_HLEN + ETH_FCS_LEN > max_contig_data) {
> 
> Do you support VLAN double tagging? We normally take into acount to two
> vlan
> headers in these checks.
> 
> Other than that:
> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

No, we only account for one.

> > +		dev_warn(priv->net_dev->dev.parent,
> > +			 "The maximum MTU for XDP is %d\n",
> > +			 max_contig_data - VLAN_ETH_HLEN -
> ETH_FCS_LEN);
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +static int dpaa_change_mtu(struct net_device *net_dev, int new_mtu)
> > +{
> > +	struct dpaa_priv *priv = netdev_priv(net_dev);
> > +
> > +	if (priv->xdp_prog && !xdp_validate_mtu(priv, new_mtu))
> > +		return -EINVAL;
> > +
> > +	net_dev->mtu = new_mtu;
> > +	return 0;
> > +}
> > +
> >  static int dpaa_setup_xdp(struct net_device *net_dev, struct bpf_prog
> *prog)
> >  {
> >  	struct dpaa_priv *priv = netdev_priv(net_dev);
> >  	struct bpf_prog *old_prog;
> > -	int err, max_contig_data;
> > +	int err;
> >  	bool up;
> >
> > -	max_contig_data = priv->dpaa_bp->size - priv->rx_headroom;
> > -
> >  	/* S/G fragments are not supported in XDP-mode */
> > -	if (prog &&
> > -	    (net_dev->mtu + VLAN_ETH_HLEN + ETH_FCS_LEN >
> max_contig_data)) {
> > -		dev_warn(net_dev->dev.parent,
> > -			 "The maximum MTU for XDP is %d\n",
> > -			 max_contig_data - VLAN_ETH_HLEN -
> ETH_FCS_LEN);
> > +	if (prog && !xdp_validate_mtu(priv, net_dev->mtu))
> >  		return -EINVAL;
> > -	}
> >
> >  	up = netif_running(net_dev);
> >
> > @@ -2870,6 +2891,7 @@ static int dpaa_ioctl(struct net_device *net_dev,
> struct ifreq *rq, int cmd)
> >  	.ndo_set_rx_mode = dpaa_set_rx_mode,
> >  	.ndo_do_ioctl = dpaa_ioctl,
> >  	.ndo_setup_tc = dpaa_setup_tc,
> > +	.ndo_change_mtu = dpaa_change_mtu,
> >  	.ndo_bpf = dpaa_xdp,
> >  };
> >
> > --
> > 1.9.1
> >
diff mbox series

Patch

diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 8acce62..ee076f4 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -2756,23 +2756,44 @@  static int dpaa_eth_stop(struct net_device *net_dev)
 	return err;
 }
 
+static bool xdp_validate_mtu(struct dpaa_priv *priv, int mtu)
+{
+	int max_contig_data = priv->dpaa_bp->size - priv->rx_headroom;
+
+	/* We do not support S/G fragments when XDP is enabled.
+	 * Limit the MTU in relation to the buffer size.
+	 */
+	if (mtu + VLAN_ETH_HLEN + ETH_FCS_LEN > max_contig_data) {
+		dev_warn(priv->net_dev->dev.parent,
+			 "The maximum MTU for XDP is %d\n",
+			 max_contig_data - VLAN_ETH_HLEN - ETH_FCS_LEN);
+		return false;
+	}
+
+	return true;
+}
+
+static int dpaa_change_mtu(struct net_device *net_dev, int new_mtu)
+{
+	struct dpaa_priv *priv = netdev_priv(net_dev);
+
+	if (priv->xdp_prog && !xdp_validate_mtu(priv, new_mtu))
+		return -EINVAL;
+
+	net_dev->mtu = new_mtu;
+	return 0;
+}
+
 static int dpaa_setup_xdp(struct net_device *net_dev, struct bpf_prog *prog)
 {
 	struct dpaa_priv *priv = netdev_priv(net_dev);
 	struct bpf_prog *old_prog;
-	int err, max_contig_data;
+	int err;
 	bool up;
 
-	max_contig_data = priv->dpaa_bp->size - priv->rx_headroom;
-
 	/* S/G fragments are not supported in XDP-mode */
-	if (prog &&
-	    (net_dev->mtu + VLAN_ETH_HLEN + ETH_FCS_LEN > max_contig_data)) {
-		dev_warn(net_dev->dev.parent,
-			 "The maximum MTU for XDP is %d\n",
-			 max_contig_data - VLAN_ETH_HLEN - ETH_FCS_LEN);
+	if (prog && !xdp_validate_mtu(priv, net_dev->mtu))
 		return -EINVAL;
-	}
 
 	up = netif_running(net_dev);
 
@@ -2870,6 +2891,7 @@  static int dpaa_ioctl(struct net_device *net_dev, struct ifreq *rq, int cmd)
 	.ndo_set_rx_mode = dpaa_set_rx_mode,
 	.ndo_do_ioctl = dpaa_ioctl,
 	.ndo_setup_tc = dpaa_setup_tc,
+	.ndo_change_mtu = dpaa_change_mtu,
 	.ndo_bpf = dpaa_xdp,
 };