> -----Original Message-----
> From: Maciej Fijalkowski <maciej.fijalkow...@intel.com>
> Sent: Tuesday, November 24, 2020 21:12
> To: Camelia Alexandra Groza <camelia.gr...@nxp.com>
> Cc: k...@kernel.org; bro...@redhat.com; sa...@kernel.org;
> da...@davemloft.net; Madalin Bucur (OSS)
> <madalin.bu...@oss.nxp.com>; Ioana Ciornei <ioana.cior...@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.bu...@oss.nxp.com>
> > Signed-off-by: Camelia Groza <camelia.gr...@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.fijalkow...@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
> >

Reply via email to