[PATCH net-next 1/2] ipv4: replace ip_hdr() with skb->data for optimization

[PATCH net-next 1/2] ipv4: replace ip_hdr() with skb->data for optimization

[Yafang Shao]

In ip receive path, when ip header hasn't been pulled yet, ip_hdr() and
skb->data are pointing to the same byte.

In ip output path, when ip header is just pushed, ip_hdr() and skb->data
are pointing to the same byte.

As ip_hdr() is more expensive than using skb->data, so replace ip_hdr()
with skb->data in these situations for optimization.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 net/ipv4/ip_input.c  | 8 ++++----
 net/ipv4/ip_output.c | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 7582713..7a03e8c 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -309,7 +309,7 @@ static inline bool ip_rcv_options(struct sk_buff *skb)
 
 static int ip_rcv_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
 {
-	const struct iphdr *iph = ip_hdr(skb);
+	const struct iphdr *iph = (const struct iphdr *)skb->data;
 	int (*edemux)(struct sk_buff *skb);
 	struct net_device *dev = skb->dev;
 	struct rtable *rt;
@@ -335,7 +335,7 @@ static int ip_rcv_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
 			if (unlikely(err))
 				goto drop_error;
 			/* must reload iph, skb->head might have changed */
-			iph = ip_hdr(skb);
+			iph = (const struct iphdr *)skb->data;
 		}
 	}
 
@@ -433,7 +433,7 @@ int ip_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt,
 	if (!pskb_may_pull(skb, sizeof(struct iphdr)))
 		goto inhdr_error;
 
-	iph = ip_hdr(skb);
+	iph = (const struct iphdr *)skb->data;
 
 	/*
 	 *	RFC1122: 3.2.1.2 MUST silently discard any IP frame that fails the checksum.
@@ -459,7 +459,7 @@ int ip_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt,
 	if (!pskb_may_pull(skb, iph->ihl*4))
 		goto inhdr_error;
 
-	iph = ip_hdr(skb);
+	iph = (const struct iphdr *)skb->data;
 
 	if (unlikely(ip_fast_csum((u8 *)iph, iph->ihl)))
 		goto csum_error;
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index af5a830..f5014cd 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -96,7 +96,7 @@ void ip_send_check(struct iphdr *iph)
 
 int __ip_local_out(struct net *net, struct sock *sk, struct sk_buff *skb)
 {
-	struct iphdr *iph = ip_hdr(skb);
+	struct iphdr *iph = (struct iphdr *)skb->data;
 
 	iph->tot_len = htons(skb->len);
 	ip_send_check(iph);
@@ -151,7 +151,7 @@ int ip_build_and_send_pkt(struct sk_buff *skb, const struct sock *sk,
 	/* Build the IP header. */
 	skb_push(skb, sizeof(struct iphdr) + (opt ? opt->opt.optlen : 0));
 	skb_reset_network_header(skb);
-	iph = ip_hdr(skb);
+	iph = (struct iphdr *)skb->data;
 	iph->version  = 4;
 	iph->ihl      = 5;
 	iph->tos      = inet->tos;
@@ -477,7 +477,7 @@ int ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl)
 	/* OK, we know where to send it, allocate and build IP header. */
 	skb_push(skb, sizeof(struct iphdr) + (inet_opt ? inet_opt->opt.optlen : 0));
 	skb_reset_network_header(skb);
-	iph = ip_hdr(skb);
+	iph = (struct iphdr *)skb->data;
 	*((__be16 *)iph) = htons((4 << 12) | (5 << 8) | (inet->tos & 0xff));
 	if (ip_dont_fragment(sk, &rt->dst) && !skb->ignore_df)
 		iph->frag_off = htons(IP_DF);
@@ -659,7 +659,7 @@ int ip_do_fragment(struct net *net, struct sock *sk, struct sk_buff *skb,
 				__skb_push(frag, hlen);
 				skb_reset_network_header(frag);
 				memcpy(skb_network_header(frag), iph, hlen);
-				iph = ip_hdr(frag);
+				iph = (struct iphdr *)skb->data;
 				iph->tot_len = htons(frag->len);
 				ip_copy_metadata(frag, skb);
 				if (offset == 0)
-- 
1.8.3.1

[PATCH net-next 2/2] ipv6: replace ip_hdr() with skb->data for optimization

[Yafang Shao]

In ipv6 receive path, when ip header hasn't been pulled yet, ip_hdr()
and skb->data are pointing to the same byte.

In ipv6 output path, when ip header is just pushed, ip_hdr() and skb->data
are pointing to the same byte.

As ip_hdr() is more expensive than using skb->data, so replace ip_hdr()
with skb->data in these situations for optimization.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 net/ipv6/ip6_input.c  | 4 ++--
 net/ipv6/ip6_output.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index f08d344..2ff4fe8 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -113,7 +113,7 @@ int ipv6_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt
 	if (unlikely(!pskb_may_pull(skb, sizeof(*hdr))))
 		goto err;
 
-	hdr = ipv6_hdr(skb);
+	hdr = (const struct ipv6hdr *)skb->data;
 
 	if (hdr->version != 6)
 		goto err;
@@ -189,7 +189,7 @@ int ipv6_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt
 			__IP6_INC_STATS(net, idev, IPSTATS_MIB_INHDRERRORS);
 			goto drop;
 		}
-		hdr = ipv6_hdr(skb);
+		hdr = (const struct ipv6hdr *)skb->data;
 	}
 
 	if (hdr->nexthdr == NEXTHDR_HOP) {
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 021e5ae..8bb3bc1 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -235,7 +235,7 @@ int ip6_xmit(const struct sock *sk, struct sk_buff *skb, struct flowi6 *fl6,
 
 	skb_push(skb, sizeof(struct ipv6hdr));
 	skb_reset_network_header(skb);
-	hdr = ipv6_hdr(skb);
+	hdr = (struct ipv6hdr *)skb->data;
 
 	/*
 	 *	Fill in the IPv6 header
@@ -1659,7 +1659,7 @@ struct sk_buff *__ip6_make_skb(struct sock *sk,
 
 	skb_push(skb, sizeof(struct ipv6hdr));
 	skb_reset_network_header(skb);
-	hdr = ipv6_hdr(skb);
+	hdr = (struct ipv6hdr *)skb->data;
 
 	ip6_flow_hdr(hdr, v6_cork->tclass,
 		     ip6_make_flowlabel(net, skb, fl6->flowlabel,
-- 
1.8.3.1

Re: [PATCH net-next 1/2] ipv4: replace ip_hdr() with skb->data for optimization

[Paolo Abeni]

On Tue, 2018-06-05 at 08:04 -0400, Yafang Shao wrote:
> In ip receive path, when ip header hasn't been pulled yet, ip_hdr() and
> skb->data are pointing to the same byte.
> 
> In ip output path, when ip header is just pushed, ip_hdr() and skb->data
> are pointing to the same byte.
> 
> As ip_hdr() is more expensive than using skb->data, so replace ip_hdr()
> with skb->data in these situations for optimization.

IMHO this makes the code less readable and more error prone. Which kind
of performance improvement do you measure here?

Thanks,

Paolo

Re: [PATCH net-next 1/2] ipv4: replace ip_hdr() with skb->data for optimization

[Yafang Shao]

On Tue, Jun 5, 2018 at 8:20 PM, Paolo Abeni <pabeni@redhat.com> wrote:
> On Tue, 2018-06-05 at 08:04 -0400, Yafang Shao wrote:
>> In ip receive path, when ip header hasn't been pulled yet, ip_hdr() and
>> skb->data are pointing to the same byte.
>>
>> In ip output path, when ip header is just pushed, ip_hdr() and skb->data
>> are pointing to the same byte.
>>
>> As ip_hdr() is more expensive than using skb->data, so replace ip_hdr()
>> with skb->data in these situations for optimization.
>
> IMHO this makes the code less readable and more error prone. Which kind
> of performance improvement do you measure here?
>

Correct the cc list.

Hi Paolo,

There's a "+" opertaion in ip_hdr(),  using skb->data and avoid this operation.

Thanks
Yafang

Re: [PATCH net-next 1/2] ipv4: replace ip_hdr() with skb->data for optimization

[David Miller]

From: Yafang Shao <laoar.shao@gmail.com>
Date: Tue, 5 Jun 2018 20:29:05 +0800

> On Tue, Jun 5, 2018 at 8:20 PM, Paolo Abeni <pabeni@redhat.com> wrote:
>> On Tue, 2018-06-05 at 08:04 -0400, Yafang Shao wrote:
>>> In ip receive path, when ip header hasn't been pulled yet, ip_hdr() and
>>> skb->data are pointing to the same byte.
>>>
>>> In ip output path, when ip header is just pushed, ip_hdr() and skb->data
>>> are pointing to the same byte.
>>>
>>> As ip_hdr() is more expensive than using skb->data, so replace ip_hdr()
>>> with skb->data in these situations for optimization.
>>
>> IMHO this makes the code less readable and more error prone. Which kind
>> of performance improvement do you measure here?
>>
> 
> Correct the cc list.
> 
> Hi Paolo,
> 
> There's a "+" opertaion in ip_hdr(),  using skb->data and avoid this operation.

Paolo is asking what performance improvement did you "measure".

I don't think this can possibly show up in a benchmark at all, and I
agree the code becomes less readable, so I am not applying this,
sorry.