netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* question about eth_header
@ 2012-11-20 13:44 Dmitry Kravkov
  2012-11-20 14:51 ` Rami Rosen
  0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Kravkov @ 2012-11-20 13:44 UTC (permalink / raw)
  To: netdev

Hi,

I am trying to use eth_hdr() but it looks like it doesn't point to mac
header (mac header is present at skb->data). Shouldn't
skb_reset_mac_header() be called inside eth_header()?

Thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: question about eth_header
  2012-11-20 13:44 question about eth_header Dmitry Kravkov
@ 2012-11-20 14:51 ` Rami Rosen
  2012-11-20 17:39   ` Dmitry Kravkov
  0 siblings, 1 reply; 3+ messages in thread
From: Rami Rosen @ 2012-11-20 14:51 UTC (permalink / raw)
  To: Dmitry Kravkov; +Cc: netdev

Hi,
I think that you should call skb_reset_mac_header()
before calling eth_hdr() and that skb_reset_mac_header() should not be
inside eth_hdr(). Following is explanation:

In the RX path, when we get a packet in the driver, what essentially
happens is that at this point, we have a pointer to
the packet buffer in skb->data, and its length in skb->len.

With ethernet packets, the driver will call eth_type_trans(); please
look at ethernet drivers code.
eth_type_trans() indeed sets the MAC header pointer by calling
skb_reset_mac_header() on the skb.

Afterwards, the eth_type_trans() will advance skb->data by
14, the size of the Ethernet header, and decrease skb->len
by calling skb_pull_inline(skb, ETH_HLEN).

In case you are wondering why the skb_reset_mac_header() is not inside
the eth_hdr(), I think that the reason is that, once
skb_reset_mac_header() on the skb was called, there are cases when we
want to make several calls eth_hdr() without each time again resetting
the mac header, for example, in the bridging code.  There is no reason
to doing it in terms of performance.

rgs,
Rami Rosen

http://ramirose.wix.com/ramirosen


On Tue, Nov 20, 2012 at 3:44 PM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
> Hi,
>
> I am trying to use eth_hdr() but it looks like it doesn't point to mac
> header (mac header is present at skb->data). Shouldn't
> skb_reset_mac_header() be called inside eth_header()?
>
> Thanks.
>
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: question about eth_header
  2012-11-20 14:51 ` Rami Rosen
@ 2012-11-20 17:39   ` Dmitry Kravkov
  0 siblings, 0 replies; 3+ messages in thread
From: Dmitry Kravkov @ 2012-11-20 17:39 UTC (permalink / raw)
  To: Rami Rosen; +Cc: netdev

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Rami Rosen
> Sent: Tuesday, November 20, 2012 4:51 PM
> To: Dmitry Kravkov
> Cc: netdev@vger.kernel.org
> Subject: Re: question about eth_header
> 
> Hi,
> I think that you should call skb_reset_mac_header()
> before calling eth_hdr() and that skb_reset_mac_header() should not be
> inside eth_hdr(). Following is explanation:
> 
> In the RX path, when we get a packet in the driver, what essentially
> happens is that at this point, we have a pointer to
> the packet buffer in skb->data, and its length in skb->len.

Actually I'm on TX path, and do not intend  to update skb, just received it 
for transmission, I want to pick mac addresess from eth header.
Now bnx2x does it by accessing skb->data (which points to the correct mac)
and I wanted to replace it with eth_hrd instead of doing ugly casting in driver code.
Moreover, in the future, when inner_mac_address will be used for tunneling
acceleration, inner_mac_address will be updated with mac_address offset, and so
may also point to the improper location.   
 
> With ethernet packets, the driver will call eth_type_trans(); please
> look at ethernet drivers code.
> eth_type_trans() indeed sets the MAC header pointer by calling
> skb_reset_mac_header() on the skb.
> 
> Afterwards, the eth_type_trans() will advance skb->data by
> 14, the size of the Ethernet header, and decrease skb->len
> by calling skb_pull_inline(skb, ETH_HLEN).
> 
> In case you are wondering why the skb_reset_mac_header() is not inside
> the eth_hdr(), I think that the reason is that, once
> skb_reset_mac_header() on the skb was called, there are cases when we
> want to make several calls eth_hdr() without each time again resetting
> the mac header, for example, in the bridging code.  There is no reason
> to doing it in terms of performance.

My question is about eth_header() function which is create() callback for
eth_header_ops.

Thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-11-20 17:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-20 13:44 question about eth_header Dmitry Kravkov
2012-11-20 14:51 ` Rami Rosen
2012-11-20 17:39   ` Dmitry Kravkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).