Patchwork GRE: Use IS_ERR_OR_NULL in gre_gso_segment

login
register
mail settings
Submitter Eric Dumazet
Date April 25, 2013, 1:58 p.m.
Message ID <1366898314.8964.135.camel@edumazet-glaptop>
Download mbox | patch
Permalink /patch/239513/
State RFC
Delegated to: David Miller
Headers show

Comments

Eric Dumazet - April 25, 2013, 1:58 p.m.
On Thu, 2013-04-25 at 17:11 +0800, Herbert Xu wrote:

> In any case, returning an error in this case makes little sense
> because when we return NULL it is precisely because the packet is
> well-formed and ready for direct processing by the hardware which
> will perform GSO instead of us.
> 
> The reason this dichotomy exists is because we've reused the
> normal software GSO path to do header verification for hardware
> GSO.
> 

OK, then current code is fine. Comments will save future 'cleanups'.



--
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
Herbert Xu - April 26, 2013, 3:38 a.m.
On Thu, Apr 25, 2013 at 06:58:34AM -0700, Eric Dumazet wrote:
> On Thu, 2013-04-25 at 17:11 +0800, Herbert Xu wrote:
> 
> > In any case, returning an error in this case makes little sense
> > because when we return NULL it is precisely because the packet is
> > well-formed and ready for direct processing by the hardware which
> > will perform GSO instead of us.
> > 
> > The reason this dichotomy exists is because we've reused the
> > normal software GSO path to do header verification for hardware
> > GSO.
> > 
> 
> OK, then current code is fine. Comments will save future 'cleanups'.

Looks good to me.  Either this or add a wrapper so that the comments
don't get out-of-sync since this construct is used in multiple
places.

Thanks,

Patch

diff --git a/net/ipv4/gre.c b/net/ipv4/gre.c
index d2d5a99..ddf72eb 100644
--- a/net/ipv4/gre.c
+++ b/net/ipv4/gre.c
@@ -168,7 +168,11 @@  static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
 	/* segment inner packet. */
 	enc_features = skb->dev->hw_enc_features & netif_skb_features(skb);
 	segs = skb_mac_gso_segment(skb, enc_features);
-	if (!segs || IS_ERR(segs))
+	/* if no segmentation is needed, we're done */
+	if (!segs)
+		goto out;
+	/* if an error happened during segmentation, we're done */
+	if (IS_ERR(segs))
 		goto out;
 
 	skb = segs;