Re: net/mlx4_en: Pad ethernet packets smaller than 17 bytes

[Amir Vadai <amirv@mellanox.com>]

On 27/02/14 13:08 +0000, David Laight wrote:
> From: Amir Vadai
> > Hardware can't accept packets smaller than 17 bytes. Therefore need to pad with
> > zeros.
> 
> Can they actually happen?
> A 16 byte packet would only have 2 bytes following the ethertype/length.
> The shortest LLC packets have 3 bytes.
Raw sockets might send such packets and it will put the HW into
unstable state.

> It may well be safe to discard them.
Yep - that another option.

> 
> ...
> > 
> > diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> > index 8dc7637..268cc4a 100644
> > --- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> > +++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> > @@ -585,12 +585,19 @@ static void build_inline_wqe(struct mlx4_en_tx_desc *tx_desc, struct sk_buff *sk
> >  	int spc = MLX4_INLINE_ALIGN - CTRL_SIZE - sizeof *inl;
> > 
> >  	if (skb->len <= spc) {
> > -		inl->byte_count = cpu_to_be32(1 << 31 | skb->len);
> > +		inl->byte_count = cpu_to_be32(1 << 31 |
> > +					      max_t(typeof(skb->len),
> > +						    skb->len,
> > +						    MIN_PKT_LEN));
> >  		skb_copy_from_linear_data(skb, inl + 1, skb_headlen(skb));
> >  		if (skb_shinfo(skb)->nr_frags)
> >  			memcpy(((void *)(inl + 1)) + skb_headlen(skb), fragptr,
> >  			       skb_frag_size(&skb_shinfo(skb)->frags[0]));
> 
> Is there guaranteed to be only 1 fragment here?
> 
Yes, skb will not be classified as inline-able if nr_frags != 1.
See is_inline()

> > +		if (skb->len < MIN_PKT_LEN)
> > +			memset(((void *)(inl + 1)) + skb->len, 0,
> > +			       MIN_PKT_LEN - skb->len);
> > +
> 
> In any case you don't want 2 checks for skb->len < MIN_PKT_LEN,
> so reassign to inl->byte_count here.
> An unlikely() probably wouldn't go amiss either.
Right - will fix it for V1

> 
> 	David
> 

Thanks,
Amir