* [PATCH net-next] xen-netback: Fix pull size in checksum_setup_ip*
@ 2013-11-29 18:56 Zoltan Kiss
2013-11-29 20:26 ` Zoltan Kiss
2013-11-30 21:10 ` David Miller
0 siblings, 2 replies; 5+ messages in thread
From: Zoltan Kiss @ 2013-11-29 18:56 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies, paul.durrant
Cc: Zoltan Kiss
Before checksum setup we need to make sure we have enough data in linear
buffer, so we do a pull if not. The calculation of the required data counts
with skb->network_header, which is the headroom size actually. This is not
necessary, and can cause unreasonable pulling. I've also removed
MAX_IPOPTLEN, as we only need the base header at that point.
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
---
drivers/net/xen-netback/netback.c | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 919b650..adadf76 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1169,8 +1169,7 @@ static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
off = sizeof(struct iphdr);
- header_size = skb->network_header + off + MAX_IPOPTLEN;
- maybe_pull_tail(skb, header_size);
+ maybe_pull_tail(skb, off);
off = iph->ihl * 4;
@@ -1183,8 +1182,7 @@ static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
if (recalculate_partial_csum) {
struct tcphdr *tcph = tcp_hdr(skb);
- header_size = skb->network_header +
- off +
+ header_size = off +
sizeof(struct tcphdr);
maybe_pull_tail(skb, header_size);
@@ -1201,8 +1199,7 @@ static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
if (recalculate_partial_csum) {
struct udphdr *udph = udp_hdr(skb);
- header_size = skb->network_header +
- off +
+ header_size = off +
sizeof(struct udphdr);
maybe_pull_tail(skb, header_size);
@@ -1241,8 +1238,7 @@ static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
off = sizeof(struct ipv6hdr);
- header_size = skb->network_header + off;
- maybe_pull_tail(skb, header_size);
+ maybe_pull_tail(skb, off);
nexthdr = ipv6h->nexthdr;
@@ -1254,8 +1250,7 @@ static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
case IPPROTO_ROUTING: {
struct ipv6_opt_hdr *hp = (void *)(skb->data + off);
- header_size = skb->network_header +
- off +
+ header_size = off +
sizeof(struct ipv6_opt_hdr);
maybe_pull_tail(skb, header_size);
@@ -1266,8 +1261,7 @@ static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
case IPPROTO_AH: {
struct ip_auth_hdr *hp = (void *)(skb->data + off);
- header_size = skb->network_header +
- off +
+ header_size = off +
sizeof(struct ip_auth_hdr);
maybe_pull_tail(skb, header_size);
@@ -1305,8 +1299,7 @@ static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
if (recalculate_partial_csum) {
struct tcphdr *tcph = tcp_hdr(skb);
- header_size = skb->network_header +
- off +
+ header_size = off +
sizeof(struct tcphdr);
maybe_pull_tail(skb, header_size);
@@ -1324,8 +1317,7 @@ static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
if (recalculate_partial_csum) {
struct udphdr *udph = udp_hdr(skb);
- header_size = skb->network_header +
- off +
+ header_size = off +
sizeof(struct udphdr);
maybe_pull_tail(skb, header_size);
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] xen-netback: Fix pull size in checksum_setup_ip*
2013-11-29 18:56 [PATCH net-next] xen-netback: Fix pull size in checksum_setup_ip* Zoltan Kiss
@ 2013-11-29 20:26 ` Zoltan Kiss
2013-12-02 9:42 ` Paul Durrant
2013-11-30 21:10 ` David Miller
1 sibling, 1 reply; 5+ messages in thread
From: Zoltan Kiss @ 2013-11-29 20:26 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies, paul.durrant
Cc: Zoltan Kiss
Or would it make sense to drop header_size and use off + something as
function parameters?
Zoli
On 29/11/13 18:56, Zoltan Kiss wrote:
> Before checksum setup we need to make sure we have enough data in linear
> buffer, so we do a pull if not. The calculation of the required data counts
> with skb->network_header, which is the headroom size actually. This is not
> necessary, and can cause unreasonable pulling. I've also removed
> MAX_IPOPTLEN, as we only need the base header at that point.
>
> Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
>
> ---
> drivers/net/xen-netback/netback.c | 24 ++++++++----------------
> 1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index 919b650..adadf76 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -1169,8 +1169,7 @@ static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
>
> off = sizeof(struct iphdr);
>
> - header_size = skb->network_header + off + MAX_IPOPTLEN;
> - maybe_pull_tail(skb, header_size);
> + maybe_pull_tail(skb, off);
>
> off = iph->ihl * 4;
>
> @@ -1183,8 +1182,7 @@ static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
> if (recalculate_partial_csum) {
> struct tcphdr *tcph = tcp_hdr(skb);
>
> - header_size = skb->network_header +
> - off +
> + header_size = off +
> sizeof(struct tcphdr);
> maybe_pull_tail(skb, header_size);
>
> @@ -1201,8 +1199,7 @@ static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
> if (recalculate_partial_csum) {
> struct udphdr *udph = udp_hdr(skb);
>
> - header_size = skb->network_header +
> - off +
> + header_size = off +
> sizeof(struct udphdr);
> maybe_pull_tail(skb, header_size);
>
> @@ -1241,8 +1238,7 @@ static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
>
> off = sizeof(struct ipv6hdr);
>
> - header_size = skb->network_header + off;
> - maybe_pull_tail(skb, header_size);
> + maybe_pull_tail(skb, off);
>
> nexthdr = ipv6h->nexthdr;
>
> @@ -1254,8 +1250,7 @@ static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
> case IPPROTO_ROUTING: {
> struct ipv6_opt_hdr *hp = (void *)(skb->data + off);
>
> - header_size = skb->network_header +
> - off +
> + header_size = off +
> sizeof(struct ipv6_opt_hdr);
> maybe_pull_tail(skb, header_size);
>
> @@ -1266,8 +1261,7 @@ static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
> case IPPROTO_AH: {
> struct ip_auth_hdr *hp = (void *)(skb->data + off);
>
> - header_size = skb->network_header +
> - off +
> + header_size = off +
> sizeof(struct ip_auth_hdr);
> maybe_pull_tail(skb, header_size);
>
> @@ -1305,8 +1299,7 @@ static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
> if (recalculate_partial_csum) {
> struct tcphdr *tcph = tcp_hdr(skb);
>
> - header_size = skb->network_header +
> - off +
> + header_size = off +
> sizeof(struct tcphdr);
> maybe_pull_tail(skb, header_size);
>
> @@ -1324,8 +1317,7 @@ static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
> if (recalculate_partial_csum) {
> struct udphdr *udph = udp_hdr(skb);
>
> - header_size = skb->network_header +
> - off +
> + header_size = off +
> sizeof(struct udphdr);
> maybe_pull_tail(skb, header_size);
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] xen-netback: Fix pull size in checksum_setup_ip*
2013-11-29 18:56 [PATCH net-next] xen-netback: Fix pull size in checksum_setup_ip* Zoltan Kiss
2013-11-29 20:26 ` Zoltan Kiss
@ 2013-11-30 21:10 ` David Miller
2013-12-02 13:04 ` Paul Durrant
1 sibling, 1 reply; 5+ messages in thread
From: David Miller @ 2013-11-30 21:10 UTC (permalink / raw)
To: zoltan.kiss
Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies, paul.durrant
From: Zoltan Kiss <zoltan.kiss@citrix.com>
Date: Fri, 29 Nov 2013 18:56:47 +0000
> Before checksum setup we need to make sure we have enough data in linear
> buffer, so we do a pull if not. The calculation of the required data counts
> with skb->network_header, which is the headroom size actually. This is not
> necessary, and can cause unreasonable pulling. I've also removed
> MAX_IPOPTLEN, as we only need the base header at that point.
>
> Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
The net-next tree is no open, therefore it is not appropriate to submit
patches targetting to that tree at this time.
I will make an explicit announcement on netdev when I have openned
the net-next tree, so it will be clear exactly when you can submit
patches targetting that tree again.
Thank you.
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH net-next] xen-netback: Fix pull size in checksum_setup_ip*
2013-11-29 20:26 ` Zoltan Kiss
@ 2013-12-02 9:42 ` Paul Durrant
0 siblings, 0 replies; 5+ messages in thread
From: Paul Durrant @ 2013-12-02 9:42 UTC (permalink / raw)
To: Zoltan Kiss, Ian Campbell, Wei Liu, xen-devel, netdev,
linux-kernel, Jonathan Davies
> -----Original Message-----
> From: Zoltan Kiss
> Sent: 29 November 2013 20:26
> To: Ian Campbell; Wei Liu; xen-devel@lists.xenproject.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Jonathan Davies;
> Paul Durrant
> Cc: Zoltan Kiss
> Subject: Re: [PATCH net-next] xen-netback: Fix pull size in
> checksum_setup_ip*
>
> Or would it make sense to drop header_size and use off + something as
> function parameters?
>
Possibly best not to top-post to your own thread ;-)
Agreed; now that we've gone from mostly 3 variable additions to 2, I don't think we need header_size to neaten things up any more.
Paul
> Zoli
>
> On 29/11/13 18:56, Zoltan Kiss wrote:
> > Before checksum setup we need to make sure we have enough data in
> linear
> > buffer, so we do a pull if not. The calculation of the required data counts
> > with skb->network_header, which is the headroom size actually. This is not
> > necessary, and can cause unreasonable pulling. I've also removed
> > MAX_IPOPTLEN, as we only need the base header at that point.
> >
> > Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
> >
> > ---
> > drivers/net/xen-netback/netback.c | 24 ++++++++----------------
> > 1 file changed, 8 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-
> netback/netback.c
> > index 919b650..adadf76 100644
> > --- a/drivers/net/xen-netback/netback.c
> > +++ b/drivers/net/xen-netback/netback.c
> > @@ -1169,8 +1169,7 @@ static int checksum_setup_ip(struct xenvif *vif,
> struct sk_buff *skb,
> >
> > off = sizeof(struct iphdr);
> >
> > - header_size = skb->network_header + off + MAX_IPOPTLEN;
> > - maybe_pull_tail(skb, header_size);
> > + maybe_pull_tail(skb, off);
> >
> > off = iph->ihl * 4;
> >
> > @@ -1183,8 +1182,7 @@ static int checksum_setup_ip(struct xenvif *vif,
> struct sk_buff *skb,
> > if (recalculate_partial_csum) {
> > struct tcphdr *tcph = tcp_hdr(skb);
> >
> > - header_size = skb->network_header +
> > - off +
> > + header_size = off +
> > sizeof(struct tcphdr);
> > maybe_pull_tail(skb, header_size);
> >
> > @@ -1201,8 +1199,7 @@ static int checksum_setup_ip(struct xenvif *vif,
> struct sk_buff *skb,
> > if (recalculate_partial_csum) {
> > struct udphdr *udph = udp_hdr(skb);
> >
> > - header_size = skb->network_header +
> > - off +
> > + header_size = off +
> > sizeof(struct udphdr);
> > maybe_pull_tail(skb, header_size);
> >
> > @@ -1241,8 +1238,7 @@ static int checksum_setup_ipv6(struct xenvif *vif,
> struct sk_buff *skb,
> >
> > off = sizeof(struct ipv6hdr);
> >
> > - header_size = skb->network_header + off;
> > - maybe_pull_tail(skb, header_size);
> > + maybe_pull_tail(skb, off);
> >
> > nexthdr = ipv6h->nexthdr;
> >
> > @@ -1254,8 +1250,7 @@ static int checksum_setup_ipv6(struct xenvif *vif,
> struct sk_buff *skb,
> > case IPPROTO_ROUTING: {
> > struct ipv6_opt_hdr *hp = (void *)(skb->data + off);
> >
> > - header_size = skb->network_header +
> > - off +
> > + header_size = off +
> > sizeof(struct ipv6_opt_hdr);
> > maybe_pull_tail(skb, header_size);
> >
> > @@ -1266,8 +1261,7 @@ static int checksum_setup_ipv6(struct xenvif *vif,
> struct sk_buff *skb,
> > case IPPROTO_AH: {
> > struct ip_auth_hdr *hp = (void *)(skb->data + off);
> >
> > - header_size = skb->network_header +
> > - off +
> > + header_size = off +
> > sizeof(struct ip_auth_hdr);
> > maybe_pull_tail(skb, header_size);
> >
> > @@ -1305,8 +1299,7 @@ static int checksum_setup_ipv6(struct xenvif *vif,
> struct sk_buff *skb,
> > if (recalculate_partial_csum) {
> > struct tcphdr *tcph = tcp_hdr(skb);
> >
> > - header_size = skb->network_header +
> > - off +
> > + header_size = off +
> > sizeof(struct tcphdr);
> > maybe_pull_tail(skb, header_size);
> >
> > @@ -1324,8 +1317,7 @@ static int checksum_setup_ipv6(struct xenvif *vif,
> struct sk_buff *skb,
> > if (recalculate_partial_csum) {
> > struct udphdr *udph = udp_hdr(skb);
> >
> > - header_size = skb->network_header +
> > - off +
> > + header_size = off +
> > sizeof(struct udphdr);
> > maybe_pull_tail(skb, header_size);
> >
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH net-next] xen-netback: Fix pull size in checksum_setup_ip*
2013-11-30 21:10 ` David Miller
@ 2013-12-02 13:04 ` Paul Durrant
0 siblings, 0 replies; 5+ messages in thread
From: Paul Durrant @ 2013-12-02 13:04 UTC (permalink / raw)
To: David Miller, Zoltan Kiss
Cc: Ian Campbell, Wei Liu, xen-devel, netdev, linux-kernel, Jonathan Davies
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: 30 November 2013 21:10
> To: Zoltan Kiss
> Cc: Ian Campbell; Wei Liu; xen-devel@lists.xenproject.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Jonathan Davies;
> Paul Durrant
> Subject: Re: [PATCH net-next] xen-netback: Fix pull size in
> checksum_setup_ip*
>
> From: Zoltan Kiss <zoltan.kiss@citrix.com>
> Date: Fri, 29 Nov 2013 18:56:47 +0000
>
> > Before checksum setup we need to make sure we have enough data in
> linear
> > buffer, so we do a pull if not. The calculation of the required data counts
> > with skb->network_header, which is the headroom size actually. This is not
> > necessary, and can cause unreasonable pulling. I've also removed
> > MAX_IPOPTLEN, as we only need the base header at that point.
> >
> > Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
>
> The net-next tree is no open, therefore it is not appropriate to submit
> patches targetting to that tree at this time.
>
> I will make an explicit announcement on netdev when I have openned
> the net-next tree, so it will be clear exactly when you can submit
> patches targetting that tree again.
>
> Thank you.
I think this patch should be for net rather than net-next anyway. Since it appears that maybe_pull_tail() has bigger problems I'll incorporate this fix as part of v4 of the fix for fragment detection.
Paul
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-12-02 13:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-29 18:56 [PATCH net-next] xen-netback: Fix pull size in checksum_setup_ip* Zoltan Kiss
2013-11-29 20:26 ` Zoltan Kiss
2013-12-02 9:42 ` Paul Durrant
2013-11-30 21:10 ` David Miller
2013-12-02 13:04 ` Paul Durrant
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).