All of lore.kernel.org
 help / color / mirror / Atom feed
From: Antonio Quartulli <antonio@meshcoding.com>
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, b.a.t.m.a.n@lists.open-mesh.org,
	Marek Lindner <lindner_marek@yahoo.de>,
	Antonio Quartulli <ordex@autistici.org>
Subject: [B.A.T.M.A.N.] [PATCH 05/16] batman-adv: h_vlan_encapsulated_proto access refactoring
Date: Sun, 13 Oct 2013 13:22:50 +0200	[thread overview]
Message-ID: <1381663381-626-6-git-send-email-antonio@meshcoding.com> (raw)
In-Reply-To: <1381663381-626-1-git-send-email-antonio@meshcoding.com>

From: Antonio Quartulli <ordex@autistici.org>

In case of a 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. While this
trickery is correct it makes the code harder to understand
and may lead to bugs in case of re-use of ethhdr for other
purposes.

This patch introduces a proto variable to make things
cleaner and easier to understand.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
---
 net/batman-adv/gateway_client.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index 1bce63a..ac97ca7 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -655,24 +655,29 @@ bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len)
 	struct iphdr *iphdr;
 	struct ipv6hdr *ipv6hdr;
 	struct udphdr *udphdr;
+	struct vlan_ethhdr *vhdr;
+	__be16 proto;
 
 	/* check for ethernet header */
 	if (!pskb_may_pull(skb, *header_len + ETH_HLEN))
 		return false;
 	ethhdr = (struct ethhdr *)skb->data;
+	proto = ethhdr->h_proto;
 	*header_len += ETH_HLEN;
 
 	/* check for initial vlan header */
-	if (ntohs(ethhdr->h_proto) == ETH_P_8021Q) {
+	if (proto == htons(ETH_P_8021Q)) {
 		if (!pskb_may_pull(skb, *header_len + VLAN_HLEN))
 			return false;
-		ethhdr = (struct ethhdr *)(skb->data + VLAN_HLEN);
+
+		vhdr = (struct vlan_ethhdr *)skb->data;
+		proto = vhdr->h_vlan_encapsulated_proto;
 		*header_len += VLAN_HLEN;
 	}
 
 	/* check for ip header */
-	switch (ntohs(ethhdr->h_proto)) {
-	case ETH_P_IP:
+	switch (proto) {
+	case htons(ETH_P_IP):
 		if (!pskb_may_pull(skb, *header_len + sizeof(*iphdr)))
 			return false;
 		iphdr = (struct iphdr *)(skb->data + *header_len);
@@ -683,7 +688,7 @@ bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len)
 			return false;
 
 		break;
-	case ETH_P_IPV6:
+	case htons(ETH_P_IPV6):
 		if (!pskb_may_pull(skb, *header_len + sizeof(*ipv6hdr)))
 			return false;
 		ipv6hdr = (struct ipv6hdr *)(skb->data + *header_len);
@@ -710,11 +715,11 @@ bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len)
 	*header_len += sizeof(*udphdr);
 
 	/* check for bootp port */
-	if ((ntohs(ethhdr->h_proto) == ETH_P_IP) &&
+	if ((proto == htons(ETH_P_IP)) &&
 	    (ntohs(udphdr->dest) != 67))
 		return false;
 
-	if ((ntohs(ethhdr->h_proto) == ETH_P_IPV6) &&
+	if ((proto == htons(ETH_P_IPV6)) &&
 	    (ntohs(udphdr->dest) != 547))
 		return false;
 
-- 
1.8.3.2


WARNING: multiple messages have this Message-ID (diff)
From: Antonio Quartulli <antonio@meshcoding.com>
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, b.a.t.m.a.n@lists.open-mesh.org,
	Antonio Quartulli <ordex@autistici.org>,
	Marek Lindner <lindner_marek@yahoo.de>
Subject: [PATCH 05/16] batman-adv: h_vlan_encapsulated_proto access refactoring
Date: Sun, 13 Oct 2013 13:22:50 +0200	[thread overview]
Message-ID: <1381663381-626-6-git-send-email-antonio@meshcoding.com> (raw)
In-Reply-To: <1381663381-626-1-git-send-email-antonio@meshcoding.com>

From: Antonio Quartulli <ordex@autistici.org>

In case of a 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. While this
trickery is correct it makes the code harder to understand
and may lead to bugs in case of re-use of ethhdr for other
purposes.

This patch introduces a proto variable to make things
cleaner and easier to understand.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
---
 net/batman-adv/gateway_client.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index 1bce63a..ac97ca7 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -655,24 +655,29 @@ bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len)
 	struct iphdr *iphdr;
 	struct ipv6hdr *ipv6hdr;
 	struct udphdr *udphdr;
+	struct vlan_ethhdr *vhdr;
+	__be16 proto;
 
 	/* check for ethernet header */
 	if (!pskb_may_pull(skb, *header_len + ETH_HLEN))
 		return false;
 	ethhdr = (struct ethhdr *)skb->data;
+	proto = ethhdr->h_proto;
 	*header_len += ETH_HLEN;
 
 	/* check for initial vlan header */
-	if (ntohs(ethhdr->h_proto) == ETH_P_8021Q) {
+	if (proto == htons(ETH_P_8021Q)) {
 		if (!pskb_may_pull(skb, *header_len + VLAN_HLEN))
 			return false;
-		ethhdr = (struct ethhdr *)(skb->data + VLAN_HLEN);
+
+		vhdr = (struct vlan_ethhdr *)skb->data;
+		proto = vhdr->h_vlan_encapsulated_proto;
 		*header_len += VLAN_HLEN;
 	}
 
 	/* check for ip header */
-	switch (ntohs(ethhdr->h_proto)) {
-	case ETH_P_IP:
+	switch (proto) {
+	case htons(ETH_P_IP):
 		if (!pskb_may_pull(skb, *header_len + sizeof(*iphdr)))
 			return false;
 		iphdr = (struct iphdr *)(skb->data + *header_len);
@@ -683,7 +688,7 @@ bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len)
 			return false;
 
 		break;
-	case ETH_P_IPV6:
+	case htons(ETH_P_IPV6):
 		if (!pskb_may_pull(skb, *header_len + sizeof(*ipv6hdr)))
 			return false;
 		ipv6hdr = (struct ipv6hdr *)(skb->data + *header_len);
@@ -710,11 +715,11 @@ bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len)
 	*header_len += sizeof(*udphdr);
 
 	/* check for bootp port */
-	if ((ntohs(ethhdr->h_proto) == ETH_P_IP) &&
+	if ((proto == htons(ETH_P_IP)) &&
 	    (ntohs(udphdr->dest) != 67))
 		return false;
 
-	if ((ntohs(ethhdr->h_proto) == ETH_P_IPV6) &&
+	if ((proto == htons(ETH_P_IPV6)) &&
 	    (ntohs(udphdr->dest) != 547))
 		return false;
 
-- 
1.8.3.2

  parent reply	other threads:[~2013-10-13 11:22 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-13 11:22 [B.A.T.M.A.N.] pull request: batman-adv 2013-10-13 Antonio Quartulli
2013-10-13 11:22 ` Antonio Quartulli
2013-10-13 11:22 ` [B.A.T.M.A.N.] [PATCH 01/16] batman-adv: only add recordroute information to icmp request/reply Antonio Quartulli
2013-10-13 11:22   ` Antonio Quartulli
2013-10-13 11:22 ` [B.A.T.M.A.N.] [PATCH 02/16] batman-adv: make tt_global_add static and return bool Antonio Quartulli
2013-10-13 11:22   ` Antonio Quartulli
2013-10-13 11:22 ` [B.A.T.M.A.N.] [PATCH 03/16] batman-adv: remove batadv_tt_global_add_orig declaration Antonio Quartulli
2013-10-13 11:22   ` Antonio Quartulli
2013-10-13 11:22 ` [B.A.T.M.A.N.] [PATCH 04/16] batman-adv: don't use call_rcu if not needed Antonio Quartulli
2013-10-13 11:22   ` Antonio Quartulli
2013-10-13 11:22 ` Antonio Quartulli [this message]
2013-10-13 11:22   ` [PATCH 05/16] batman-adv: h_vlan_encapsulated_proto access refactoring Antonio Quartulli
2013-10-13 11:22 ` [B.A.T.M.A.N.] [PATCH 06/16] batman-adv: use VLAN_ETH_HLEN instead of sizeof(struct vlan_eth_hdr) Antonio Quartulli
2013-10-13 11:22   ` Antonio Quartulli
2013-10-13 11:22 ` [B.A.T.M.A.N.] [PATCH 07/16] batman-adv: Remove old fragmentation code Antonio Quartulli
2013-10-13 11:22   ` Antonio Quartulli
2013-10-13 11:22 ` [B.A.T.M.A.N.] [PATCH 08/16] batman-adv: Receive fragmented packets and merge Antonio Quartulli
2013-10-13 11:22   ` Antonio Quartulli
2013-10-13 11:22 ` [B.A.T.M.A.N.] [PATCH 09/16] batman-adv: Fragment and send skbs larger than mtu Antonio Quartulli
2013-10-13 11:22   ` Antonio Quartulli
2013-10-13 11:22 ` [B.A.T.M.A.N.] [PATCH 10/16] batman-adv: use htons when possible Antonio Quartulli
2013-10-13 11:22   ` Antonio Quartulli
2013-10-13 11:22 ` [B.A.T.M.A.N.] [PATCH 11/16] batman-adv: create common header for ICMP packets Antonio Quartulli
2013-10-13 11:22   ` Antonio Quartulli
2013-10-13 11:22 ` [B.A.T.M.A.N.] [PATCH 12/16] batman-adv: consider network coding overhead when calculating required mtu Antonio Quartulli
2013-10-13 11:22   ` Antonio Quartulli
2013-10-13 11:22 ` [B.A.T.M.A.N.] [PATCH 13/16] batman-adv: remove useless find_router look up Antonio Quartulli
2013-10-13 11:22   ` Antonio Quartulli
2013-10-13 11:22 ` [B.A.T.M.A.N.] [PATCH 14/16] batman-adv: implement batadv_tt_entries Antonio Quartulli
2013-10-13 11:22   ` Antonio Quartulli
2013-10-13 11:23 ` [B.A.T.M.A.N.] [PATCH 15/16] batman-adv: make batadv_tt_save_orig_buffer() generic Antonio Quartulli
2013-10-13 11:23   ` Antonio Quartulli
2013-10-13 11:23 ` [B.A.T.M.A.N.] [PATCH 16/16] batman-adv: Add dummy soft-interface rx mode handler Antonio Quartulli
2013-10-13 11:23   ` Antonio Quartulli
2013-10-17 18:27 ` [B.A.T.M.A.N.] pull request: batman-adv 2013-10-13 David Miller
2013-10-17 18:27   ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1381663381-626-6-git-send-email-antonio@meshcoding.com \
    --to=antonio@meshcoding.com \
    --cc=b.a.t.m.a.n@lists.open-mesh.org \
    --cc=davem@davemloft.net \
    --cc=lindner_marek@yahoo.de \
    --cc=netdev@vger.kernel.org \
    --cc=ordex@autistici.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.