| Message ID | 654d6300001825e542341bc052c31433b48b1913.1606150838.git.camelia.groza@nxp.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [net-next,v4,1/7] dpaa_eth: add struct for software backpointers | expand |
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 >
> -----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 --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, };