[ofa-general] Re: [RFC][1/2] IPoIB UD 4K MTU support

[Or Gerlitz]

Shirley Ma wrote:
> --- a/drivers/infiniband/ulp/ipoib/ipoib.h
> +++ b/drivers/infiniband/ulp/ipoib/ipoib.h
> @@ -61,6 +61,10 @@ enum {
>  
>  	IPOIB_ENCAP_LEN		  = 4,
>  
> +	IPOIB_UD_MAX_PAYLOAD	  = 4096,
> +	IPOIB_UD_HEAD_SIZE	  = IB_GRH_BYTES + IPOIB_ENCAP_LEN,
> +	IPOIB_UD_RX_SG		  = (IPOIB_UD_MAX_PAYLOAD + IB_GRH_BYTES) / PAGE_SIZE,
>   
Reading ipoib_ud_skb_put_frags below and its usage in the patch that follows, its unclear to me if IPOIB_UD_MAX_PAYLOAD is being made of (4K - IPOIB_ENCAP_LEN) + IPOIB_ENCAP_LEN or from adjustment to some IP header alignment constraint. Specifically, the design I'd like to see here is that the IPoIB header telling the type of the frame (ARP, IPv4, IPv6, etc) is provided up to the stack as part of the packet in the skb (eg its very useful with tcpdump/etc filters). 

Reading earlier threads I see that Roland suggested to allow for upto 4K-4 mtu towards the stack and use some internal buffer for the GRH where this buffer can be allocated and dma mapped once and being forget from till the driver cleans up, etc. Was there any problem with this approach?


> +static inline void ipoib_ud_skb_put_frags(struct ipoib_dev_priv *priv,
> +					  struct sk_buff *skb,
> +					  unsigned int length)
> +{
> +	if (ipoib_ud_need_sg(priv->max_ib_mtu)) {
> +		skb_frag_t *frag = &skb_shinfo(skb)->frags[0];
> +		/*
> +		 * There is only two buffers needed for max_payload = 4K,
> +		 * first buf size is IPOIB_UD_HEAD_SIZE
> +		 */
> +		skb->tail += IPOIB_UD_HEAD_SIZE;
> +		frag->size = length - IPOIB_UD_HEAD_SIZE;
> +		skb->data_len += frag->size;
> +		skb->truesize += frag->size;
> +		skb->len += length;
> +	} else
> +		skb_put(skb, length);
> +
> +}
>   
I fail to follow what this code really wants to do and how it does it. 
Is there a must to touch "by hand" all the internal skb fields? also 
this function is called once by ipoib_ib_handle_rx_wc in the patch that 
follows, any reason not to make it static over there?

Or.