| Message ID | 20210202022420.1328397-7-anthony.l.nguyen@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | 40GbE Intel Wired LAN Driver Updates 2021-02-01 | expand |
On Mon, 1 Feb 2021 18:24:20 -0800 Tony Nguyen wrote: > From: Eryk Rybak <eryk.roch.rybak@intel.com> > > When attempting to link XDP prog with MTU larger than supported, > user is not informed why XDP linking fails. Adding proper > error message: "MTU too large to enable XDP". > Due to the lack of support for non-static variables in netlinks > extended ACK feature, additional information has been added to dmesg > to better inform about invalid MTU setting. > > Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> > Signed-off-by: Eryk Rybak <eryk.roch.rybak@intel.com> > Tested-by: Kiran Bhandare <kiranx.bhandare@intel.com> > Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com> > @@ -12459,8 +12460,13 @@ static int i40e_xdp_setup(struct i40e_vsi *vsi, > int i; > > /* Don't allow frames that span over multiple buffers */ > - if (frame_size > vsi->rx_buf_len) > + if (frame_size > vsi->rx_buf_len) { > + NL_SET_ERR_MSG_MOD(extack, "MTU too large to enable XDP"); > + dev_info(&pf->pdev->dev, > + "MTU of %u bytes is too large to enable XDP (maximum: %u bytes)\n", > + vsi->netdev->mtu, vsi->rx_buf_len); Extack should be enough.
Good day Jakub We want to be user friendly to help users troubleshoot faster. Only dmesg message can have template parameters so we can provide exact acceptable maximum bytes. Can you could you take this into account? Thank you -----Original Message----- From: Jakub Kicinski <kuba@kernel.org> Sent: Wednesday, February 3, 2021 3:35 AM To: Nguyen, Anthony L <anthony.l.nguyen@intel.com> Cc: davem@davemloft.net; Rybak, Eryk Roch <eryk.roch.rybak@intel.com>; netdev@vger.kernel.org; sassmann@redhat.com; Topel, Bjorn <bjorn.topel@intel.com>; Fijalkowski, Maciej <maciej.fijalkowski@intel.com>; Karlsson, Magnus <magnus.karlsson@intel.com>; Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; Bhandare, KiranX <kiranx.bhandare@intel.com> Subject: Re: [PATCH net-next 6/6] i40e: Log error for oversized MTU on device On Mon, 1 Feb 2021 18:24:20 -0800 Tony Nguyen wrote: > From: Eryk Rybak <eryk.roch.rybak@intel.com> > > When attempting to link XDP prog with MTU larger than supported, user > is not informed why XDP linking fails. Adding proper error message: > "MTU too large to enable XDP". > Due to the lack of support for non-static variables in netlinks > extended ACK feature, additional information has been added to dmesg > to better inform about invalid MTU setting. > > Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> > Signed-off-by: Eryk Rybak <eryk.roch.rybak@intel.com> > Tested-by: Kiran Bhandare <kiranx.bhandare@intel.com> > Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com> > @@ -12459,8 +12460,13 @@ static int i40e_xdp_setup(struct i40e_vsi *vsi, > int i; > > /* Don't allow frames that span over multiple buffers */ > - if (frame_size > vsi->rx_buf_len) > + if (frame_size > vsi->rx_buf_len) { > + NL_SET_ERR_MSG_MOD(extack, "MTU too large to enable XDP"); > + dev_info(&pf->pdev->dev, > + "MTU of %u bytes is too large to enable XDP (maximum: %u bytes)\n", > + vsi->netdev->mtu, vsi->rx_buf_len); Extack should be enough. --------------------------------------------------------------------- Intel Technology Poland sp. z o.o. ul. Sowackiego 173 | 80-298 Gdask | Sd Rejonowy Gdask Pnoc | VII Wydzia Gospodarczy Krajowego Rejestru Sdowego - KRS 101882 | NIP 957-07-52-316 | Kapita zakadowy 200.000 PLN. Ta wiadomo wraz z zacznikami jest przeznaczona dla okrelonego adresata i moe zawiera informacje poufne. W razie przypadkowego otrzymania tej wiadomoci, prosimy o powiadomienie nadawcy oraz trwae jej usunicie; jakiekolwiek przegldanie lub rozpowszechnianie jest zabronione. This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.
On Wed, 3 Feb 2021 09:14:24 +0000 Loktionov, Aleksandr wrote: > Good day Jakub Please don't top post. > We want to be user friendly to help users troubleshoot faster. > Only dmesg message can have template parameters so we can provide > exact acceptable maximum bytes. Can you could you take this into > account? I was making the same exact point when adding the message for NFP years ago and it was shot down :) Today upstream is getting close to removing the page-per-packet requirement, so this will hopefully become irrelevant soon. Maciej should have the details on that, he seems to be keeping up the most with upstream in ITP.
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index f35bd9164106..b52a324a9f28 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -12448,9 +12448,10 @@ static netdev_features_t i40e_features_check(struct sk_buff *skb, * i40e_xdp_setup - add/remove an XDP program * @vsi: VSI to changed * @prog: XDP program + * @extack: netlink extended ack **/ -static int i40e_xdp_setup(struct i40e_vsi *vsi, - struct bpf_prog *prog) +static int i40e_xdp_setup(struct i40e_vsi *vsi, struct bpf_prog *prog, + struct netlink_ext_ack *extack) { int frame_size = vsi->netdev->mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN; struct i40e_pf *pf = vsi->back; @@ -12459,8 +12460,13 @@ static int i40e_xdp_setup(struct i40e_vsi *vsi, int i; /* Don't allow frames that span over multiple buffers */ - if (frame_size > vsi->rx_buf_len) + if (frame_size > vsi->rx_buf_len) { + NL_SET_ERR_MSG_MOD(extack, "MTU too large to enable XDP"); + dev_info(&pf->pdev->dev, + "MTU of %u bytes is too large to enable XDP (maximum: %u bytes)\n", + vsi->netdev->mtu, vsi->rx_buf_len); return -EINVAL; + } if (!i40e_enabled_xdp_vsi(vsi) && !prog) return 0; @@ -12772,7 +12778,7 @@ static int i40e_xdp(struct net_device *dev, switch (xdp->command) { case XDP_SETUP_PROG: - return i40e_xdp_setup(vsi, xdp->prog); + return i40e_xdp_setup(vsi, xdp->prog, xdp->extack); case XDP_SETUP_XSK_POOL: return i40e_xsk_pool_setup(vsi, xdp->xsk.pool, xdp->xsk.queue_id);