Patchwork net: Fix inner_network_header assignment in skb-copy.

login
register
mail settings
Submitter Pravin B Shelar
Date Feb. 2, 2013, 12:26 a.m.
Message ID <1359764763-1618-1-git-send-email-pshelar@nicira.com>
Download mbox | patch
Permalink /patch/217628/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Pravin B Shelar - Feb. 2, 2013, 12:26 a.m.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
 net/core/skbuff.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Paul Gortmaker - Feb. 2, 2013, 12:57 a.m.
On Fri, Feb 1, 2013 at 7:26 PM, Pravin B Shelar <pshelar@nicira.com> wrote:
> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>

Hi Pravin,

As a general rule of thumb, a commit log should cover these three
basic things:

1) What are the end user visible symptoms of this issue (i.e. does it panic,
does it hang, do applications segfault, etc etc).

2) Why does this happen?  i.e. explain where the logic error is in the
existing code base and so forth.

3) How do we best fix this?   i.e. explain what our options are to fix
the issue, and why your choice of fixes is technically the best choice.

With no long log whatsoever, your change does not cover any of
the #1, #2 or #3.  And without that, it also makes review by the
maintainers more difficult, and reduces the chance that your
patch will be integrated into mainline.  Can you perhaps resend
with a more detailed commit log, given the above information?

Thanks,
Paul.
--


> ---
>  net/core/skbuff.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index bddc1dd..55f7ef6 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -686,7 +686,7 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
>         new->network_header     = old->network_header;
>         new->mac_header         = old->mac_header;
>         new->inner_transport_header = old->inner_transport_header;
> -       new->inner_network_header = old->inner_transport_header;
> +       new->inner_network_header = old->inner_network_header;
>         skb_dst_copy(new, old);
>         new->rxhash             = old->rxhash;
>         new->ooo_okay           = old->ooo_okay;
> --
> 1.7.10
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pravin B Shelar - Feb. 2, 2013, 1:14 a.m.
On Fri, Feb 1, 2013 at 4:57 PM, Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:
> On Fri, Feb 1, 2013 at 7:26 PM, Pravin B Shelar <pshelar@nicira.com> wrote:
>> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
>
> Hi Pravin,
>
> As a general rule of thumb, a commit log should cover these three
> basic things:
>
> 1) What are the end user visible symptoms of this issue (i.e. does it panic,
> does it hang, do applications segfault, etc etc).
>
> 2) Why does this happen?  i.e. explain where the logic error is in the
> existing code base and so forth.
>
> 3) How do we best fix this?   i.e. explain what our options are to fix
> the issue, and why your choice of fixes is technically the best choice.
>
> With no long log whatsoever, your change does not cover any of
> the #1, #2 or #3.  And without that, it also makes review by the
> maintainers more difficult, and reduces the chance that your
> patch will be integrated into mainline.  Can you perhaps resend
> with a more detailed commit log, given the above information?
>

Well its trivial change so I did not wrote commit msg. Anyways I will
send patch with a commit msg.

Thanks,
Pravin.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index bddc1dd..55f7ef6 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -686,7 +686,7 @@  static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
 	new->network_header	= old->network_header;
 	new->mac_header		= old->mac_header;
 	new->inner_transport_header = old->inner_transport_header;
-	new->inner_network_header = old->inner_transport_header;
+	new->inner_network_header = old->inner_network_header;
 	skb_dst_copy(new, old);
 	new->rxhash		= old->rxhash;
 	new->ooo_okay		= old->ooo_okay;