Re: [PATCH] Gigabit Ethernet driver of Topcliff PCH

[Stephen Hemminger <shemminger@vyatta.com>]

On Thu, 02 Sep 2010 15:40:47 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Le jeudi 02 septembre 2010 à 21:39 +0900, Masayuki Ohtake a écrit :
> > Hi Eric
> > 
> > Thank you for your comments.
> > 
> > > I find hard to believe this driver needs to copy all outgoing frames on
> > > pre-allocated skbs.
> > >
> > > +       /* [Header:14][payload] ---> [Header:14][paddong:2][payload]    */
> > > +       memcpy(tmp_skb->data, skb->data, ETH_HLEN);
> > > +       tmp_skb->data[ETH_HLEN] = 0x00;
> > > +       tmp_skb->data[ETH_HLEN + 1] = 0x00;
> > > +       tmp_skb->len = skb->len;
> > > +       memcpy(&tmp_skb->data[ETH_HLEN + 2], &skb->data[ETH_HLEN],
> > > +              (skb->len - ETH_HLEN));
> > > +       buffer_info->kernel_skb = skb;
> > > +       skb = tmp_skb;
> > >
> > > Whats the deal here please ?
> > 
> > This processing depends on hardware specification.
> > 
> > At the time of transmission.
> > Hardware accepts a packet in the following format.
> >   [Header: 14octet] + [padding: 2octet] + [payload]
> >   Also, it is necessary to align the head of a [Header: 14octet] at 64byte.
> > 
> > In my knowledge, SKB received by kernel are the following format.
> >   [padding: 2octet] + [Header:14octet] + [payload]
> >   Also, The head of [payload] has aligned at 16 byte.
> > 
> > So, it has adjusted with the format of hardware by a copy.
> 
> The two bytes padding can be handled by network stack, if you
> force in your setup phase :
> 	dev->hard_header_len = ETH_HLEN + 2;
> 
> then, you can do a single memcpy:
> 
> memcpy(tmp_skb->data, skb->data, skb->len);
> 
> About the 64 byte alignement, it might be a bit more complex :)

With Eric's suggestion, you might find most packets are going
to be aligned but the code should not depend on it always being true.
Packets that get forwarded have header determined by the receiving
interface.

Something like:
       skb = skb_realloc_headroom(skb, 2);
       if (!skb)
           goto drop;
	__skb_push(skb, 2);
        memmove(skb->data, skb->data + 2, ETH_HLEN);

	skb->data[ETH_HLEN] = 0;
	skb->data[ETH_HLEN+1] = 0;
    
        if (!IS_ALIGNED(skb->data, 16)) {
	    nskb = netdev_alloc_skb(dev, skb->len + 16);
	    if (!nskb)
                goto drop2;
            skb_reserve(nskb, PTR_ALIGN(skb->data, 16) - skb->data);
	    skb_put(skb, skb->len);
            memcpy(nskb->data, skb->data, skb->len);
	    dev_kfree_skb(skb);
            skb = nskb;
        }
       



--