[PATCH] packet: introduce pkt_iph_len()

[Nikola Pajkovsky]

func returns internet header length from pkt_hdr dependently ETH_P_IP or ETH_P_IPV6

--
I do love macros ;)

Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com&gt;
---
 src/itrafmon.c |    7 ++-----
 src/packet.c   |   10 +++++-----
 src/packet.h   |   15 +++++++++++++--
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/src/itrafmon.c b/src/itrafmon.c
index 1142fdb..c0c8373 100644
--- a/src/itrafmon.c
+++ b/src/itrafmon.c
@@ -585,9 +585,6 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
 
 	unsigned int br;	/* bytes read.  Differs from readlen */
 
-	/* only when packets fragmented */
-	unsigned int iphlen;
-
 	struct tcptable table;
 	struct tcptableent *tcpentry;
 	struct tcptableent *tmptcp;
@@ -1016,11 +1013,9 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
 
 		switch(pkt.pkt_protocol) {
 		case ETH_P_IP:
-			iphlen = pkt_ipv4_len(&pkt);
 			frag_off = pkt.iphdr->frag_off;
 			break;
 		case ETH_P_IPV6:
-			iphlen = 40;
 			frag_off = 0;
 			break;
 		default:
@@ -1033,6 +1028,8 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
 			continue;
 		}
 
+		/* only when packets fragmented */
+		__u8 iphlen = pkt_iph_len(&pkt);
 		transpacket = (struct tcphdr *) (pkt.pkt_payload + iphlen);
 
 		__u8 ip_protocol = pkt_ip_protocol(&pkt);
diff --git a/src/packet.c b/src/packet.c
index 25d11ff..0c262be 100644
--- a/src/packet.c
+++ b/src/packet.c
@@ -206,7 +206,7 @@ again:
 
 		ip_checksum = ip->check;
 		ip->check = 0;
-		hdr_check = in_cksum((u_short *) pkt->iphdr, pkt_ipv4_len(pkt));
+		hdr_check = in_cksum((u_short *) pkt->iphdr, pkt_iph_len(pkt));
 
 		if ((hdr_check != ip_checksum))
 			return CHECKSUM_ERROR;
@@ -240,7 +240,7 @@ again:
 			} else {
 				struct tcphdr *tcp;
 				struct udphdr *udp;
-				char *ip_payload = (char *) ip + pkt_ipv4_len(pkt);
+				char *ip_payload = (char *) ip + pkt_iph_len(pkt);
 
 				switch (ip->protocol) {
 				case IPPROTO_TCP:
@@ -283,8 +283,8 @@ again:
 			return PACKET_FILTERED;
 		if (v6inv4asv6 && (ip->protocol == IPPROTO_IPV6)) {
 			pkt->pkt_protocol = ETH_P_IPV6;
-			pkt->pkt_payload += pkt_ipv4_len(pkt);
-			pkt->pkt_len -= pkt_ipv4_len(pkt);
+			pkt->pkt_payload += pkt_iph_len(pkt);
+			pkt->pkt_len -= pkt_iph_len(pkt);
 			goto again;
 		}
 		return PACKET_OK;
@@ -292,7 +292,7 @@ again:
 		struct tcphdr *tcp;
 		struct udphdr *udp;
 		struct ip6_hdr *ip6 = pkt->ip6_hdr;
-		char *ip_payload = (char *) ip6 + 40;
+		char *ip_payload = (char *) ip6 + pkt_iph_len(pkt);
 
 		//TODO: Filter packets
 		switch (ip6->ip6_nxt) {	/* FIXME: extension headers ??? */
diff --git a/src/packet.h b/src/packet.h
index 2b4f802..21d4c37 100644
--- a/src/packet.h
+++ b/src/packet.h
@@ -55,8 +55,19 @@ static inline void PACKET_INIT_STRUCT(struct pkt_hdr *p)
 	struct pkt_hdr packet;					\
 	PACKET_INIT_STRUCT(&packet)
 
-#define pkt_ipv4_len(pkt)				\
-	(pkt)->iphdr->ihl * 4
+#define pkt_iph_len(pkt) __extension__ ({				\
+		__u8 len__ = 0;						\
+		switch ((pkt)->pkt_protocol) {				\
+		case ETH_P_IP:						\
+			len__ = (pkt)->iphdr->ihl * 4;			\
+			break;						\
+		case ETH_P_IPV6:					\
+			len__ = 40;					\
+			break;						\
+		};							\
+		len__;							\
+	})
+
 
 static inline __u8 pkt_ip_protocol(const struct pkt_hdr *p)
 {
-- 
1.7.10.2

[Vitezslav Samel]

On Tue, Jul 17, 2012 at 03:27:41PM +0200, Nikola Pajkovsky wrote:
>  func returns internet header length from pkt_hdr dependently ETH_P_IP
> or ETH_P_IPV6
>  
>  --
>  I do love macros ;)
>  
>  Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com&gt; 
  Acked-by: Vitezslav Samel <vitezslav(a)samel.cz&gt;

>  ---
>   src/itrafmon.c |    7 ++-----
>   src/packet.c   |   10 +++++-----
>   src/packet.h   |   15 +++++++++++++--
>   3 files changed, 20 insertions(+), 12 deletions(-)
>  
>  diff --git a/src/itrafmon.c b/src/itrafmon.c
>  index 1142fdb..c0c8373 100644
>  --- a/src/itrafmon.c
>  +++ b/src/itrafmon.c
>  @@ -585,9 +585,6 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
>   
>   	unsigned int br;	/* bytes read.  Differs from readlen */
>   
>  -	/* only when packets fragmented */
>  -	unsigned int iphlen;
>  -
>   	struct tcptable table;
>   	struct tcptableent *tcpentry;
>   	struct tcptableent *tmptcp;
>  @@ -1016,11 +1013,9 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
>   
>   		switch(pkt.pkt_protocol) {
>   		case ETH_P_IP:
>  -			iphlen = pkt_ipv4_len(&pkt);
>   			frag_off = pkt.iphdr->frag_off;
>   			break;
>   		case ETH_P_IPV6:
>  -			iphlen = 40;
>   			frag_off = 0;
>   			break;
>   		default:
>  @@ -1033,6 +1028,8 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
>   			continue;
>   		}
>   
>  +		/* only when packets fragmented */
>  +		__u8 iphlen = pkt_iph_len(&pkt);
>   		transpacket = (struct tcphdr *) (pkt.pkt_payload + iphlen);
>   
>   		__u8 ip_protocol = pkt_ip_protocol(&pkt);
>  diff --git a/src/packet.c b/src/packet.c
>  index 25d11ff..0c262be 100644
>  --- a/src/packet.c
>  +++ b/src/packet.c
>  @@ -206,7 +206,7 @@ again:
>   
>   		ip_checksum = ip->check;
>   		ip->check = 0;
>  -		hdr_check = in_cksum((u_short *) pkt->iphdr, pkt_ipv4_len(pkt));
>  +		hdr_check = in_cksum((u_short *) pkt->iphdr, pkt_iph_len(pkt));
>   
>   		if ((hdr_check != ip_checksum))
>   			return CHECKSUM_ERROR;
>  @@ -240,7 +240,7 @@ again:
>   			} else {
>   				struct tcphdr *tcp;
>   				struct udphdr *udp;
>  -				char *ip_payload = (char *) ip + pkt_ipv4_len(pkt);
>  +				char *ip_payload = (char *) ip + pkt_iph_len(pkt);
>   
>   				switch (ip->protocol) {
>   				case IPPROTO_TCP:
>  @@ -283,8 +283,8 @@ again:
>   			return PACKET_FILTERED;
>   		if (v6inv4asv6 && (ip->protocol == IPPROTO_IPV6)) {
>   			pkt->pkt_protocol = ETH_P_IPV6;
>  -			pkt->pkt_payload += pkt_ipv4_len(pkt);
>  -			pkt->pkt_len -= pkt_ipv4_len(pkt);
>  +			pkt->pkt_payload += pkt_iph_len(pkt);
>  +			pkt->pkt_len -= pkt_iph_len(pkt);
>   			goto again;
>   		}
>   		return PACKET_OK;
>  @@ -292,7 +292,7 @@ again:
>   		struct tcphdr *tcp;
>   		struct udphdr *udp;
>   		struct ip6_hdr *ip6 = pkt->ip6_hdr;
>  -		char *ip_payload = (char *) ip6 + 40;
>  +		char *ip_payload = (char *) ip6 + pkt_iph_len(pkt);
>   
>   		//TODO: Filter packets
>   		switch (ip6->ip6_nxt) {	/* FIXME: extension headers ??? */
>  diff --git a/src/packet.h b/src/packet.h
>  index 2b4f802..21d4c37 100644
>  --- a/src/packet.h
>  +++ b/src/packet.h
>  @@ -55,8 +55,19 @@ static inline void PACKET_INIT_STRUCT(struct pkt_hdr *p)
>   	struct pkt_hdr packet;					\
>   	PACKET_INIT_STRUCT(&packet)
>   
>  -#define pkt_ipv4_len(pkt)				\
>  -	(pkt)->iphdr->ihl * 4
>  +#define pkt_iph_len(pkt) __extension__ ({				\
>  +		__u8 len__ = 0;						\
>  +		switch ((pkt)->pkt_protocol) {				\
>  +		case ETH_P_IP:						\
>  +			len__ = (pkt)->iphdr->ihl * 4;			\
>  +			break;						\
>  +		case ETH_P_IPV6:					\
>  +			len__ = 40;					\
>  +			break;						\
>  +		};							\
>  +		len__;							\
>  +	})
>  +
>   
>   static inline __u8 pkt_ip_protocol(const struct pkt_hdr *p)
>   {
>  -- 
>  1.7.10.2

[Vitezslav Samel]

On Tue, Jul 17, 2012 at 03:27:41PM +0200, Nikola Pajkovsky wrote:
>  func returns internet header length from pkt_hdr dependently ETH_P_IP
> or ETH_P_IPV6
>  
>  --
>  I do love macros ;) 
  Looking at it one more time I would prefer a little inline function
(as pkt_ip_protocol()) over macro with __extension__. Think of compiling
with another C compiler (C-Lang, intel-cc, ...). Would you accept a
patch which changes that?

	Vita

>  Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com&gt;
>  ---
>   src/itrafmon.c |    7 ++-----
>   src/packet.c   |   10 +++++-----
>   src/packet.h   |   15 +++++++++++++--
>   3 files changed, 20 insertions(+), 12 deletions(-)
>  
>  diff --git a/src/itrafmon.c b/src/itrafmon.c
>  index 1142fdb..c0c8373 100644
>  --- a/src/itrafmon.c
>  +++ b/src/itrafmon.c
>  @@ -585,9 +585,6 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
>   
>   	unsigned int br;	/* bytes read.  Differs from readlen */
>   
>  -	/* only when packets fragmented */
>  -	unsigned int iphlen;
>  -
>   	struct tcptable table;
>   	struct tcptableent *tcpentry;
>   	struct tcptableent *tmptcp;
>  @@ -1016,11 +1013,9 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
>   
>   		switch(pkt.pkt_protocol) {
>   		case ETH_P_IP:
>  -			iphlen = pkt_ipv4_len(&pkt);
>   			frag_off = pkt.iphdr->frag_off;
>   			break;
>   		case ETH_P_IPV6:
>  -			iphlen = 40;
>   			frag_off = 0;
>   			break;
>   		default:
>  @@ -1033,6 +1028,8 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
>   			continue;
>   		}
>   
>  +		/* only when packets fragmented */
>  +		__u8 iphlen = pkt_iph_len(&pkt);
>   		transpacket = (struct tcphdr *) (pkt.pkt_payload + iphlen);
>   
>   		__u8 ip_protocol = pkt_ip_protocol(&pkt);
>  diff --git a/src/packet.c b/src/packet.c
>  index 25d11ff..0c262be 100644
>  --- a/src/packet.c
>  +++ b/src/packet.c
>  @@ -206,7 +206,7 @@ again:
>   
>   		ip_checksum = ip->check;
>   		ip->check = 0;
>  -		hdr_check = in_cksum((u_short *) pkt->iphdr, pkt_ipv4_len(pkt));
>  +		hdr_check = in_cksum((u_short *) pkt->iphdr, pkt_iph_len(pkt));
>   
>   		if ((hdr_check != ip_checksum))
>   			return CHECKSUM_ERROR;
>  @@ -240,7 +240,7 @@ again:
>   			} else {
>   				struct tcphdr *tcp;
>   				struct udphdr *udp;
>  -				char *ip_payload = (char *) ip + pkt_ipv4_len(pkt);
>  +				char *ip_payload = (char *) ip + pkt_iph_len(pkt);
>   
>   				switch (ip->protocol) {
>   				case IPPROTO_TCP:
>  @@ -283,8 +283,8 @@ again:
>   			return PACKET_FILTERED;
>   		if (v6inv4asv6 && (ip->protocol == IPPROTO_IPV6)) {
>   			pkt->pkt_protocol = ETH_P_IPV6;
>  -			pkt->pkt_payload += pkt_ipv4_len(pkt);
>  -			pkt->pkt_len -= pkt_ipv4_len(pkt);
>  +			pkt->pkt_payload += pkt_iph_len(pkt);
>  +			pkt->pkt_len -= pkt_iph_len(pkt);
>   			goto again;
>   		}
>   		return PACKET_OK;
>  @@ -292,7 +292,7 @@ again:
>   		struct tcphdr *tcp;
>   		struct udphdr *udp;
>   		struct ip6_hdr *ip6 = pkt->ip6_hdr;
>  -		char *ip_payload = (char *) ip6 + 40;
>  +		char *ip_payload = (char *) ip6 + pkt_iph_len(pkt);
>   
>   		//TODO: Filter packets
>   		switch (ip6->ip6_nxt) {	/* FIXME: extension headers ??? */
>  diff --git a/src/packet.h b/src/packet.h
>  index 2b4f802..21d4c37 100644
>  --- a/src/packet.h
>  +++ b/src/packet.h
>  @@ -55,8 +55,19 @@ static inline void PACKET_INIT_STRUCT(struct pkt_hdr *p)
>   	struct pkt_hdr packet;					\
>   	PACKET_INIT_STRUCT(&packet)
>   
>  -#define pkt_ipv4_len(pkt)				\
>  -	(pkt)->iphdr->ihl * 4
>  +#define pkt_iph_len(pkt) __extension__ ({				\
>  +		__u8 len__ = 0;						\
>  +		switch ((pkt)->pkt_protocol) {				\
>  +		case ETH_P_IP:						\
>  +			len__ = (pkt)->iphdr->ihl * 4;			\
>  +			break;						\
>  +		case ETH_P_IPV6:					\
>  +			len__ = 40;					\
>  +			break;						\
>  +		};							\
>  +		len__;							\
>  +	})
>  +
>   
>   static inline __u8 pkt_ip_protocol(const struct pkt_hdr *p)
>   {
>  -- 
>  1.7.10.2

[Nikola Pajkovsky]

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

>  On Tue, Jul 17, 2012 at 03:27:41PM +0200, Nikola Pajkovsky wrote:
> > func returns internet header length from pkt_hdr dependently ETH_P_IP or ETH_P_IPV6
> > 
> > --
> > I do love macros ;)
>
>    Looking at it one more time I would prefer a little inline function
>  (as pkt_ip_protocol()) over macro with __extension__. Think of compiling
>  with another C compiler (C-Lang, intel-cc, ...). Would you accept a
>  patch which changes that? 
yup

-- 
Nikola