* [PATCH -next] xen-netback: fix some error return code
@ 2013-12-16 5:49 Wei Yongjun
2013-12-16 13:13 ` Wei Liu
0 siblings, 1 reply; 9+ messages in thread
From: Wei Yongjun @ 2013-12-16 5:49 UTC (permalink / raw)
To: ian.campbell, wei.liu2; +Cc: yongjun_wei, xen-devel, netdev
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Fix to return -EPROTO error code instead of 0(overwrite by maybe_pull_tail())
if skb_partial_csum_set() return false.
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
drivers/net/xen-netback/netback.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 800496b..b16daa7 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1108,8 +1108,10 @@ static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
goto out;
if (!skb_partial_csum_set(skb, off,
- offsetof(struct tcphdr, check)))
+ offsetof(struct tcphdr, check))) {
+ err = -EPROTO;
goto out;
+ }
if (recalculate_partial_csum)
tcp_hdr(skb)->check =
@@ -1126,8 +1128,10 @@ static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
goto out;
if (!skb_partial_csum_set(skb, off,
- offsetof(struct udphdr, check)))
+ offsetof(struct udphdr, check))) {
+ err = -EPROTO;
goto out;
+ }
if (recalculate_partial_csum)
udp_hdr(skb)->check =
@@ -1249,8 +1253,10 @@ static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
goto out;
if (!skb_partial_csum_set(skb, off,
- offsetof(struct tcphdr, check)))
+ offsetof(struct tcphdr, check))) {
+ err = -EPROTO;
goto out;
+ }
if (recalculate_partial_csum)
tcp_hdr(skb)->check =
@@ -1267,8 +1273,10 @@ static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
goto out;
if (!skb_partial_csum_set(skb, off,
- offsetof(struct udphdr, check)))
+ offsetof(struct udphdr, check))) {
+ err = -EPROTO;
goto out;
+ }
if (recalculate_partial_csum)
udp_hdr(skb)->check =
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH -next] xen-netback: fix some error return code
2013-12-16 5:49 [PATCH -next] xen-netback: fix some error return code Wei Yongjun
@ 2013-12-16 13:13 ` Wei Liu
2013-12-16 13:40 ` Ian Campbell
0 siblings, 1 reply; 9+ messages in thread
From: Wei Liu @ 2013-12-16 13:13 UTC (permalink / raw)
To: Wei Yongjun; +Cc: ian.campbell, wei.liu2, yongjun_wei, xen-devel, netdev
On Mon, Dec 16, 2013 at 01:49:29PM +0800, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> Fix to return -EPROTO error code instead of 0(overwrite by maybe_pull_tail())
> if skb_partial_csum_set() return false.
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
> drivers/net/xen-netback/netback.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index 800496b..b16daa7 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -1108,8 +1108,10 @@ static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
> goto out;
>
> if (!skb_partial_csum_set(skb, off,
> - offsetof(struct tcphdr, check)))
> + offsetof(struct tcphdr, check))) {
> + err = -EPROTO;
Why is this needed? It is set to -EPROTO before the 'switch'.
Wei.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH -next] xen-netback: fix some error return code
2013-12-16 13:13 ` Wei Liu
@ 2013-12-16 13:40 ` Ian Campbell
2013-12-16 13:57 ` Wei Yongjun
0 siblings, 1 reply; 9+ messages in thread
From: Ian Campbell @ 2013-12-16 13:40 UTC (permalink / raw)
To: Wei Liu; +Cc: Wei Yongjun, yongjun_wei, xen-devel, netdev
On Mon, 2013-12-16 at 13:13 +0000, Wei Liu wrote:
> On Mon, Dec 16, 2013 at 01:49:29PM +0800, Wei Yongjun wrote:
> > From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> >
> > Fix to return -EPROTO error code instead of 0(overwrite by maybe_pull_tail())
> > if skb_partial_csum_set() return false.
> >
> > Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> > ---
> > drivers/net/xen-netback/netback.c | 16 ++++++++++++----
> > 1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> > index 800496b..b16daa7 100644
> > --- a/drivers/net/xen-netback/netback.c
> > +++ b/drivers/net/xen-netback/netback.c
> > @@ -1108,8 +1108,10 @@ static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
> > goto out;
> >
> > if (!skb_partial_csum_set(skb, off,
> > - offsetof(struct tcphdr, check)))
> > + offsetof(struct tcphdr, check))) {
> > + err = -EPROTO;
>
> Why is this needed? It is set to -EPROTO before the 'switch'.
Agreed.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH -next] xen-netback: fix some error return code
2013-12-16 13:40 ` Ian Campbell
@ 2013-12-16 13:57 ` Wei Yongjun
2013-12-16 14:12 ` Ian Campbell
0 siblings, 1 reply; 9+ messages in thread
From: Wei Yongjun @ 2013-12-16 13:57 UTC (permalink / raw)
To: Ian.Campbell; +Cc: wei.liu2, yongjun_wei, xen-devel, netdev
On 12/16/2013 09:40 PM, Ian Campbell wrote:
> On Mon, 2013-12-16 at 13:13 +0000, Wei Liu wrote:
>> On Mon, Dec 16, 2013 at 01:49:29PM +0800, Wei Yongjun wrote:
>>> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>>
>>> Fix to return -EPROTO error code instead of 0(overwrite by maybe_pull_tail())
>>> if skb_partial_csum_set() return false.
>>>
>>> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>> ---
>>> drivers/net/xen-netback/netback.c | 16 ++++++++++++----
>>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
>>> index 800496b..b16daa7 100644
>>> --- a/drivers/net/xen-netback/netback.c
>>> +++ b/drivers/net/xen-netback/netback.c
>>> @@ -1108,8 +1108,10 @@ static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
>>> goto out;
>>>
>>> if (!skb_partial_csum_set(skb, off,
>>> - offsetof(struct tcphdr, check)))
>>> + offsetof(struct tcphdr, check))) {
>>> + err = -EPROTO;
>> Why is this needed? It is set to -EPROTO before the 'switch'.
> Agreed.
As I said in the description, err is overwrited to 0 by the above
'err = maybe_pull_tail(...)' call in the same 'case XXX'.
Refer to the source code:
1104 err = maybe_pull_tail(skb,
1105 off + sizeof(struct tcphdr),
1106 MAX_IP_HDR_LEN);
1107 if (err < 0)
1108 goto out;
1109
1110 if (!skb_partial_csum_set(skb, off,
1111 offsetof(struct tcphdr, check)))
1112 goto out;
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH -next] xen-netback: fix some error return code
2013-12-16 13:57 ` Wei Yongjun
@ 2013-12-16 14:12 ` Ian Campbell
2013-12-17 2:32 ` Wei Yongjun
2013-12-17 2:42 ` [PATCH v2] " Wei Yongjun
0 siblings, 2 replies; 9+ messages in thread
From: Ian Campbell @ 2013-12-16 14:12 UTC (permalink / raw)
To: Wei Yongjun; +Cc: wei.liu2, yongjun_wei, xen-devel, netdev
On Mon, 2013-12-16 at 21:57 +0800, Wei Yongjun wrote:
> On 12/16/2013 09:40 PM, Ian Campbell wrote:
> > On Mon, 2013-12-16 at 13:13 +0000, Wei Liu wrote:
> >> On Mon, Dec 16, 2013 at 01:49:29PM +0800, Wei Yongjun wrote:
> >>> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> >>>
> >>> Fix to return -EPROTO error code instead of 0(overwrite by maybe_pull_tail())
> >>> if skb_partial_csum_set() return false.
> >>>
> >>> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> >>> ---
> >>> drivers/net/xen-netback/netback.c | 16 ++++++++++++----
> >>> 1 file changed, 12 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> >>> index 800496b..b16daa7 100644
> >>> --- a/drivers/net/xen-netback/netback.c
> >>> +++ b/drivers/net/xen-netback/netback.c
> >>> @@ -1108,8 +1108,10 @@ static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
> >>> goto out;
> >>>
> >>> if (!skb_partial_csum_set(skb, off,
> >>> - offsetof(struct tcphdr, check)))
> >>> + offsetof(struct tcphdr, check))) {
> >>> + err = -EPROTO;
> >> Why is this needed? It is set to -EPROTO before the 'switch'.
> > Agreed.
>
> As I said in the description, err is overwrited to 0 by the above
> 'err = maybe_pull_tail(...)' call in the same 'case XXX'.
> Refer to the source code:
What tree/commit is this? Neither Linus', net or net-next look like this
to me.
>
> 1104 err = maybe_pull_tail(skb,
> 1105 off + sizeof(struct tcphdr),
> 1106 MAX_IP_HDR_LEN);
> 1107 if (err < 0)
> 1108 goto out;
> 1109
> 1110 if (!skb_partial_csum_set(skb, off,
> 1111 offsetof(struct tcphdr, check)))
> 1112 goto out;
>
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH -next] xen-netback: fix some error return code
2013-12-16 14:12 ` Ian Campbell
@ 2013-12-17 2:32 ` Wei Yongjun
2013-12-17 2:42 ` [PATCH v2] " Wei Yongjun
1 sibling, 0 replies; 9+ messages in thread
From: Wei Yongjun @ 2013-12-17 2:32 UTC (permalink / raw)
To: Ian.Campbell; +Cc: wei.liu2, yongjun_wei, xen-devel, netdev
On 12/16/2013 10:12 PM, Ian Campbell wrote:
> On Mon, 2013-12-16 at 21:57 +0800, Wei Yongjun wrote:
>> On 12/16/2013 09:40 PM, Ian Campbell wrote:
>>> On Mon, 2013-12-16 at 13:13 +0000, Wei Liu wrote:
>>>> On Mon, Dec 16, 2013 at 01:49:29PM +0800, Wei Yongjun wrote:
>>>>> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>>>>
>>>>> Fix to return -EPROTO error code instead of 0(overwrite by maybe_pull_tail())
>>>>> if skb_partial_csum_set() return false.
>>>>>
>>>>> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>>>> ---
>>>>> drivers/net/xen-netback/netback.c | 16 ++++++++++++----
>>>>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
>>>>> index 800496b..b16daa7 100644
>>>>> --- a/drivers/net/xen-netback/netback.c
>>>>> +++ b/drivers/net/xen-netback/netback.c
>>>>> @@ -1108,8 +1108,10 @@ static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
>>>>> goto out;
>>>>>
>>>>> if (!skb_partial_csum_set(skb, off,
>>>>> - offsetof(struct tcphdr, check)))
>>>>> + offsetof(struct tcphdr, check))) {
>>>>> + err = -EPROTO;
>>>> Why is this needed? It is set to -EPROTO before the 'switch'.
>>> Agreed.
>> As I said in the description, err is overwrited to 0 by the above
>> 'err = maybe_pull_tail(...)' call in the same 'case XXX'.
>> Refer to the source code:
> What tree/commit is this? Neither Linus', net or net-next look like this
> to me.
Sorry, I used linux-next tree for this patch. It looks like the linux.git and net tree
has this code, but not net-next.
This commit cause this issue:
https://git.kernel.org/cgit/linux/kernel/git/davem/net.git/commit/drivers/net/xen-netback/netback.c?id=d52eb0d46f3606b9de9965cebb2beb2202a0dc62
So this patch is need for net tree, not net-next. I will rebased this
patch with description changed.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] xen-netback: fix some error return code
2013-12-16 14:12 ` Ian Campbell
2013-12-17 2:32 ` Wei Yongjun
@ 2013-12-17 2:42 ` Wei Yongjun
2013-12-17 9:49 ` Wei Liu
2013-12-19 19:59 ` David Miller
1 sibling, 2 replies; 9+ messages in thread
From: Wei Yongjun @ 2013-12-17 2:42 UTC (permalink / raw)
To: Ian.Campbell; +Cc: wei.liu2, yongjun_wei, xen-devel, netdev, davem
'err' is overwrited to 0 after maybe_pull_tail() call, so the error
code was not set if skb_partial_csum_set() call failed. Fix to return
error -EPROTO from those error handling case instead of 0.
Fixes: d52eb0d46f36 ('xen-netback: make sure skb linear area covers checksum field')
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
v1 -> v2: rebased and fixed the description
---
drivers/net/xen-netback/netback.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index e884ee1..5d28e64 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1206,8 +1206,10 @@ static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
goto out;
if (!skb_partial_csum_set(skb, off,
- offsetof(struct tcphdr, check)))
+ offsetof(struct tcphdr, check))) {
+ err = -EPROTO;
goto out;
+ }
if (recalculate_partial_csum)
tcp_hdr(skb)->check =
@@ -1224,8 +1226,10 @@ static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
goto out;
if (!skb_partial_csum_set(skb, off,
- offsetof(struct udphdr, check)))
+ offsetof(struct udphdr, check))) {
+ err = -EPROTO;
goto out;
+ }
if (recalculate_partial_csum)
udp_hdr(skb)->check =
@@ -1347,8 +1351,10 @@ static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
goto out;
if (!skb_partial_csum_set(skb, off,
- offsetof(struct tcphdr, check)))
+ offsetof(struct tcphdr, check))) {
+ err = -EPROTO;
goto out;
+ }
if (recalculate_partial_csum)
tcp_hdr(skb)->check =
@@ -1365,8 +1371,10 @@ static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
goto out;
if (!skb_partial_csum_set(skb, off,
- offsetof(struct udphdr, check)))
+ offsetof(struct udphdr, check))) {
+ err = -EPROTO;
goto out;
+ }
if (recalculate_partial_csum)
udp_hdr(skb)->check =
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] xen-netback: fix some error return code
2013-12-17 2:42 ` [PATCH v2] " Wei Yongjun
@ 2013-12-17 9:49 ` Wei Liu
2013-12-19 19:59 ` David Miller
1 sibling, 0 replies; 9+ messages in thread
From: Wei Liu @ 2013-12-17 9:49 UTC (permalink / raw)
To: Wei Yongjun; +Cc: Ian.Campbell, wei.liu2, yongjun_wei, xen-devel, netdev, davem
On Tue, Dec 17, 2013 at 10:42:09AM +0800, Wei Yongjun wrote:
> 'err' is overwrited to 0 after maybe_pull_tail() call, so the error
> code was not set if skb_partial_csum_set() call failed. Fix to return
> error -EPROTO from those error handling case instead of 0.
>
> Fixes: d52eb0d46f36 ('xen-netback: make sure skb linear area covers checksum field')
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Acked-by: Wei Liu <wei.liu2@citrix.com>
This makes sense. This patch applies to 'net' tree. You should specify
'net' or 'net-next' tree in subject line for your future patch. You can
find relevant infomation in netdev-FAQ.txt in kernel documentation
directory.
Thanks
Wei.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] xen-netback: fix some error return code
2013-12-17 2:42 ` [PATCH v2] " Wei Yongjun
2013-12-17 9:49 ` Wei Liu
@ 2013-12-19 19:59 ` David Miller
1 sibling, 0 replies; 9+ messages in thread
From: David Miller @ 2013-12-19 19:59 UTC (permalink / raw)
To: weiyj.lk; +Cc: Ian.Campbell, wei.liu2, yongjun_wei, xen-devel, netdev
From: Wei Yongjun <weiyj.lk@gmail.com>
Date: Tue, 17 Dec 2013 10:42:09 +0800
> 'err' is overwrited to 0 after maybe_pull_tail() call, so the error
> code was not set if skb_partial_csum_set() call failed. Fix to return
> error -EPROTO from those error handling case instead of 0.
>
> Fixes: d52eb0d46f36 ('xen-netback: make sure skb linear area covers checksum field')
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
> v1 -> v2: rebased and fixed the description
Applied and queued up for -stable.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-12-19 19:59 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-16 5:49 [PATCH -next] xen-netback: fix some error return code Wei Yongjun
2013-12-16 13:13 ` Wei Liu
2013-12-16 13:40 ` Ian Campbell
2013-12-16 13:57 ` Wei Yongjun
2013-12-16 14:12 ` Ian Campbell
2013-12-17 2:32 ` Wei Yongjun
2013-12-17 2:42 ` [PATCH v2] " Wei Yongjun
2013-12-17 9:49 ` Wei Liu
2013-12-19 19:59 ` David Miller
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).