[net-next PATCH 0/5] Add eth_proto_is_802_3 to provide improved means of checking Ethertype

[net-next PATCH 0/5] Add eth_proto_is_802_3 to provide improved means of checking Ethertype

[Alexander Duyck]

This patch series implements and makes use of eth_proto_is_802_3().  The
idea behind the function is to provide an optimized means of testing to
determine if a given Ethertype value is a length or 802.3 protocol number.
The standard path for this was to use ntohs(proto) and then perform a
comparison.  This adds a slight cost as it usually requires either a 16b
rotate or byte swap which can cost 1 cycle or more depending on the
processor.

I had previously addressed this for eth_type_trans, however in doing so I had
overlooked checking with sparse and had introduced a couple sparse warnings.
The first patch in this series fixes those sparse warnings as well as does
some additional optimization for big endian systems.  In addition it pushes
the code out into a separate function which can then be used in the other
patches to reduce the instruction count/processing time in those functions
as well.

---

Alexander Duyck (5):
      etherdev: Fix sparse error, make test usable by other functions
      ebtables: Use eth_proto_is_802_3
      ipv4/ip_tunnel_core: Use eth_proto_is_802_3
      openvswitch: Use eth_proto_is_802_3
      vlan: Use eth_proto_is_802_3


 include/linux/etherdevice.h     |   18 ++++++++++++++++++
 include/linux/if_vlan.h         |    2 +-
 net/bridge/netfilter/ebtables.c |    2 +-
 net/ethernet/eth.c              |    2 +-
 net/ipv4/ip_tunnel_core.c       |    2 +-
 net/openvswitch/datapath.c      |    2 +-
 net/openvswitch/flow.c          |    4 ++--
 net/openvswitch/flow_netlink.c  |    2 +-
 8 files changed, 26 insertions(+), 8 deletions(-)

--

[net-next PATCH 1/5] etherdev: Fix sparse error, make test usable by other functions

[Alexander Duyck]

This change does two things.  First it fixes a sparse error for the fact
that the __be16 degrades to an integer.  Since that is actually what I am
kind of doing I am simply working around that by forcing both sides of the
comparison to u16.

Also I realized on some compilers I was generating another instruction for
big endian systems such as PowerPC since it was masking the value before
doing the comparison.  So to resolve that I have simply pulled the mask out
and wrapped it in an #ifndef __BIG_ENDIAN.

Lastly I pulled this all out into its own function.  I notices there are
similar checks in a number of other places so this function can be reused
there to help reduce overhead in these paths as well.

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 include/linux/etherdevice.h |   18 ++++++++++++++++++
 net/ethernet/eth.c          |    2 +-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index c4a10f991fe0..9012f8775208 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -191,6 +191,24 @@ static inline bool is_valid_ether_addr(const u8 *addr)
 }
 
 /**
+ * eth_proto_is_802_3 - Determine if a given Ethertype/length is a protocol
+ * @proto: Ethertype/length value to be tested
+ *
+ * Check that the value from the Ethertype/length field is a valid Ethertype.
+ *
+ * Return true if the valid is an 802.3 supported Ethertype.
+ */
+static inline bool eth_proto_is_802_3(__be16 proto)
+{
+#ifndef __BIG_ENDIAN
+	/* if CPU is little endian mask off bits representing LSB */
+	proto &= htons(0xFF00);
+#endif
+	/* cast both to u16 and compare since LSB can be ignored */
+	return (__force u16)proto >= (__force u16)htons(ETH_P_802_3_MIN);
+}
+
+/**
  * eth_random_addr - Generate software assigned random Ethernet address
  * @addr: Pointer to a six-byte array containing the Ethernet address
  *
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 314e4c5a5a5e..9045e2a1108f 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -179,7 +179,7 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev)
 	if (unlikely(netdev_uses_dsa(dev)))
 		return htons(ETH_P_XDSA);
 
-	if (likely((eth->h_proto & htons(0xFF00)) >= htons(ETH_P_802_3_MIN)))
+	if (likely(eth_proto_is_802_3(eth->h_proto)))
 		return eth->h_proto;
 
 	/*

[net-next PATCH 2/5] ebtables: Use eth_proto_is_802_3

[Alexander Duyck]

Replace "ntohs(proto) >= ETH_P_802_3_MIN" w/ eth_proto_is_802_3(proto).

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 net/bridge/netfilter/ebtables.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 91180a7fc943..5149d9e71114 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -139,7 +139,7 @@ ebt_basic_match(const struct ebt_entry *e, const struct sk_buff *skb,
 		ethproto = h->h_proto;
 
 	if (e->bitmask & EBT_802_3) {
-		if (FWINV2(ntohs(ethproto) >= ETH_P_802_3_MIN, EBT_IPROTO))
+		if (FWINV2(eth_proto_is_802_3(ethproto), EBT_IPROTO))
 			return 1;
 	} else if (!(e->bitmask & EBT_NOPROTO) &&
 	   FWINV2(e->ethproto != ethproto, EBT_IPROTO))

[net-next PATCH 3/5] ipv4/ip_tunnel_core: Use eth_proto_is_802_3

[Alexander Duyck]

Replace "ntohs(proto) >= ETH_P_802_3_MIN" w/ eth_proto_is_802_3(proto).

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 net/ipv4/ip_tunnel_core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index ce63ab21b6cd..3998b1822d85 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -98,7 +98,7 @@ int iptunnel_pull_header(struct sk_buff *skb, int hdr_len, __be16 inner_proto)
 			return -ENOMEM;
 
 		eh = (struct ethhdr *)skb->data;
-		if (likely(ntohs(eh->h_proto) >= ETH_P_802_3_MIN))
+		if (likely(eth_proto_is_802_3(eh->h_proto)))
 			skb->protocol = eh->h_proto;
 		else
 			skb->protocol = htons(ETH_P_802_2);

[net-next PATCH 4/5] openvswitch: Use eth_proto_is_802_3

[Alexander Duyck]

Replace "ntohs(proto) >= ETH_P_802_3_MIN" w/ eth_proto_is_802_3(proto).

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 net/openvswitch/datapath.c     |    2 +-
 net/openvswitch/flow.c         |    4 ++--
 net/openvswitch/flow_netlink.c |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 096c6276e6b9..3b90461317ec 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -545,7 +545,7 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
 	/* Normally, setting the skb 'protocol' field would be handled by a
 	 * call to eth_type_trans(), but it assumes there's a sending
 	 * device, which we may not have. */
-	if (ntohs(eth->h_proto) >= ETH_P_802_3_MIN)
+	if (eth_proto_is_802_3(eth->h_proto))
 		packet->protocol = eth->h_proto;
 	else
 		packet->protocol = htons(ETH_P_802_2);
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 2dacc7b5af23..bc7b0aba994a 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -332,7 +332,7 @@ static __be16 parse_ethertype(struct sk_buff *skb)
 	proto = *(__be16 *) skb->data;
 	__skb_pull(skb, sizeof(__be16));
 
-	if (ntohs(proto) >= ETH_P_802_3_MIN)
+	if (eth_proto_is_802_3(proto))
 		return proto;
 
 	if (skb->len < sizeof(struct llc_snap_hdr))
@@ -349,7 +349,7 @@ static __be16 parse_ethertype(struct sk_buff *skb)
 
 	__skb_pull(skb, sizeof(struct llc_snap_hdr));
 
-	if (ntohs(llc->ethertype) >= ETH_P_802_3_MIN)
+	if (eth_proto_is_802_3(llc->ethertype))
 		return llc->ethertype;
 
 	return htons(ETH_P_802_2);
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index c691b1a1eee0..624e41c4267f 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -816,7 +816,7 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs,
 		if (is_mask) {
 			/* Always exact match EtherType. */
 			eth_type = htons(0xffff);
-		} else if (ntohs(eth_type) < ETH_P_802_3_MIN) {
+		} else if (!eth_proto_is_802_3(eth_type)) {
 			OVS_NLERR(log, "EtherType %x is less than min %x",
 				  ntohs(eth_type), ETH_P_802_3_MIN);
 			return -EINVAL;

[net-next PATCH 5/5] vlan: Use eth_proto_is_802_3

[Alexander Duyck]

Replace "ntohs(proto) >= ETH_P_802_3_MIN" w/ eth_proto_is_802_3(proto).

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 include/linux/if_vlan.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 920e4457ce6e..b9ab677c0c0a 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -539,7 +539,7 @@ static inline void vlan_set_encap_proto(struct sk_buff *skb,
 	 */
 
 	proto = vhdr->h_vlan_encapsulated_proto;
-	if (ntohs(proto) >= ETH_P_802_3_MIN) {
+	if (eth_proto_is_802_3(proto)) {
 		skb->protocol = proto;
 		return;
 	}

Re: [net-next PATCH 0/5] Add eth_proto_is_802_3 to provide improved means of checking Ethertype

[David Miller]

From: Alexander Duyck <alexander.h.duyck@redhat.com>
Date: Mon, 04 May 2015 14:33:42 -0700

> This patch series implements and makes use of eth_proto_is_802_3().  The
> idea behind the function is to provide an optimized means of testing to
> determine if a given Ethertype value is a length or 802.3 protocol number.
> The standard path for this was to use ntohs(proto) and then perform a
> comparison.  This adds a slight cost as it usually requires either a 16b
> rotate or byte swap which can cost 1 cycle or more depending on the
> processor.
> 
> I had previously addressed this for eth_type_trans, however in doing so I had
> overlooked checking with sparse and had introduced a couple sparse warnings.
> The first patch in this series fixes those sparse warnings as well as does
> some additional optimization for big endian systems.  In addition it pushes
> the code out into a separate function which can then be used in the other
> patches to reduce the instruction count/processing time in those functions
> as well.

Series applied, thanks!