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

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

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
>