On Sun, May 12, 2013 at 03:25:10PM +0800, Marek Lindner wrote: > > On Tuesday, May 07, 2013 19:54:02 Antonio Quartulli wrote: > > > In the gateway code in case of VLAN tagged frame the ethhdr > > > pointer is moved forward by 4 bytes so that the offset of > > > h_proto in struct ethhdr matches the real > > > h_vlan_encapsulated_proto address in the skb. > > > > > > This trick is correct but the code is not easy to understand > > > and may lead to bugs in case of re-use of ethhdr for other > > > purposes. > > > > > > Use a standalone protocol variable and assign it accordingly > > > to the header type > > > > How about adding the key word "refactoring" somewhere into the subject line > > or > > the commit message ? >
oky
>
> >
> >
> > > @@ -626,23 +626,29 @@ bool batadv_gw_is_dhcp_target(struct sk_buff *skb,
> > > unsigned int *header_len) struct iphdr *iphdr;
> > > struct ipv6hdr *ipv6hdr;
> > > struct udphdr *udphdr;
> > > + uint16_t proto;
> > >
> > > /* check for ethernet header */
> > > if (!pskb_may_pull(skb, *header_len + ETH_HLEN))
> > > return false;
> > > ethhdr = (struct ethhdr *)skb->data;
> > > + proto = ntohs(ethhdr->h_proto);
> > > *header_len += ETH_HLEN;
> > >
> > > /* check for initial vlan header */
> > > - if (ntohs(ethhdr->h_proto) == ETH_P_8021Q) {
> > > + if (proto == ETH_P_8021Q) {
> > > + struct vlan_ethhdr *vhdr;
> > > +
> >
> > While we are refactoring I suggest to not ntohs() the protocol every time
> > but
> > the protocol constants. The compiler will replace the constants with the
> > appropriate number during the compilation, thus saving us from a runtime
> > operation every time we check a packet.
> >
>
oh right. I did not think about it. I guess I have to use __constant_htons
instead of the plain hntons.
cheers,
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
signature.asc
Description: Digital signature
