[Intel-wired-lan] [PATCH] i40e: Add error message when MTU on device is out of the range

[Paul Menzel]

Dear Eryk,


Am 18.12.20 um 17:52 schrieb Eryk Rybak:

Maybe a shorter summary:

 > i40e: Log error for oversized MTU on device

> When attempting to link XDP prog with MTU larger than supported,
> user is not informed why XDP linking fails. Adding proper

Nit: Imperative mood could be used instead: Add proper …

> error message.

Personally, when adding new log message, I find it nice to see them in 
the commit message too.

> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski at intel.com>
> Signed-off-by: Eryk Rybak <eryk.roch.rybak at intel.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e_main.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 630258e..4fdef00 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -12933,9 +12933,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;
> @@ -12944,8 +12945,10 @@ 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");

Could you please also print out both values? Maybe (unsure about the units):

    MTU of %u bytes(?) too large to enable XDP (maximum: %u bytes)


Kind regards,

Paul


>   		return -EINVAL;
> +	}
>   
>   	/* When turning XDP on->off/off->on we reset and rebuild the rings. */
>   	need_reset = (i40e_enabled_xdp_vsi(vsi) != !!prog);
> @@ -13254,7 +13257,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);
> 
> base-commit: d2c2bbad93b818020657a82121a147e9ace1230d