Re: [PATCH 4/4] iphdr, ip6_hdr: add new members into pkt_hdr struct

[Nikola Pajkovsky]

Vitezslav Samel <vitezslav(a)samel.cz&gt; writes:

>  On Fri, Jul 13, 2012 at 04:24:31PM +0200, Nikola Pajkovsky wrote:
> > Vitezslav Samel <vitezslav(a)samel.cz&gt; writes:
> > 
> > > On Fri, Jul 13, 2012 at 03:19:18PM +0200, Nikola Pajkovsky wrote:
> > >> not everything is possible converth right now
> > >> 
> > >> Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com&gt;
> > >> ---
> > >>  src/detstats.c |    6 ++---
> > >>  src/itrafmon.c |   70
> +++++++++++++++++++++++++-------------------------------
> > >>  src/packet.c   |    6 +++++
> > >>  src/packet.h   |    4 ++++
> > >>  src/serv.c     |   13 +++++------
> > >>  5 files changed, 49 insertions(+), 50 deletions(-)
> > >> 
> > >> diff --git a/src/packet.c b/src/packet.c
> > >> index fbd94f1..2a22dbc 100644
> > >> --- a/src/packet.c
> > >> +++ b/src/packet.c
> > >> @@ -47,6 +47,12 @@ static int packet_adjust(struct pkt_hdr *pkt)
> > >>  	case ARPHRD_ETHER:
> > >>  	case ARPHRD_LOOPBACK:
> > >>  		pkt_cast_hdrp(ethhdr, pkt, 0);
> > >> +		if (pkt->pkt_protocol == ETH_P_IP)
> > >> +			pkt_cast_hdrp(iphdr, pkt, ETH_HLEN);
> > >> +
> > >> +		if (pkt->pkt_protocol == ETH_P_IPV6)
> > >> +			pkt_cast_hdrp(ip6_hdr, pkt, ETH_HLEN);
> > >> +
> > >>  		pkt->pkt_payload = pkt->pkt_buf;
> > >>  		pkt->pkt_payload += ETH_HLEN;
> > >>  		pkt->pkt_len -= ETH_HLEN;
> > >
> > >  NAK.
> > >
> > >   Tests for upper layer protocol should be moved after this switch and
> > > pkt_cast_hdrp() (better some new function) should use pkt->pkt_payload
> > > pointer. (You are mixing two protocol layers together - don't do this!)
> > 
> > I'm not sure if I'm on the same page.
>
>    Those two if()s must be in every case statement of the switch() in
>  packet_adjust() function, because IP or IPv6 can be seen not only on
>  loopback or ethernet but also on other devices e.g. on FDDI or else. So
>  it's better to move initializing of iphdr and ip6_hdr pointers to the new
>  function but then we must use the pkt->pkt_payload pointer. And we have
>  a problem when we are changing pkt->pkt_protocol (e.g. when tunneling
>  IPv6 in IP). 
Ah, I see... I incorporated your patch and did a few changes (renames)
and I introduce PACKET_INIT_STRUCT static inline func. Funcs is there to
clear all pointers before we fetch raw packet.

I still think, that we should have some generic func, which will be able
cast headers on specific OSI level. I will meditate for a while if it's
doable.

>    I made a patch (on top of this series) which takes care of it (see
> the
>  attached patch). 
I will resend the PATCH 4/4.

-- 
Nikola