netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net-next repost] MPLS: Use mpls_features to activate software MPLS GSO segmentation
@ 2014-05-30  5:35 Simon Horman
  2014-05-30 22:43 ` David Miller
  0 siblings, 1 reply; 3+ messages in thread
From: Simon Horman @ 2014-05-30  5:35 UTC (permalink / raw)
  To: David Miller, netdev-u79uwXL29TY76Z2rM5mHXA; +Cc: dev-yBygre7rU0TnMu66kgdUjQ

If an MPLS packet requires segmentation then use mpls_features
to determine if the software implementation should be used.

As no driver advertises MPLS GSO segmentation this will always be
the case.

I had not noticed that this was necessary before as software MPLS GSO
segmentation was already being used in my test environment. I believe that
the reason for that is the skbs in question always had fragments and the
driver I used does not advertise NETIF_F_FRAGLIST (which seems to be the
case for most drivers). Thus software segmentation was activated by
skb_gso_ok().

Thanks to Jesse Gross for prompting me to investigate this.

Acked-by: Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Simon Horman <horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>

---
v2
* Added Ack from Jesse Gross
* Removed duplicate 'Thus' from changelog
---
 net/core/dev.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index 0355ca5..15f3e6f 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2540,6 +2540,40 @@ netdev_features_t netif_skb_features(struct sk_buff *skb)
 }
 EXPORT_SYMBOL(netif_skb_features);
 
+/* If MPLS offload request, verify we are testing hardware MPLS features
+ * instead of standard features for the netdev.
+ */
+#ifdef CONFIG_NET_MPLS_GSO
+static inline netdev_features_t net_mpls_features(struct sk_buff *skb,
+	struct net_device *dev, netdev_features_t features)
+{
+	/* There is no support for MPLS LRO. So the only way that
+	 * an MPLS skb could require GSO segmentation is if it
+	 * was received as a non-MPLS skb and then became an MPLS skb.
+	 * This may be effected by Open vSwitch in which case the
+	 * mac_len will non-zero and not equal to skb_network_offset
+	 * as the former indicates the end of L2 the latter indicates
+	 * the beginning of L3 and there is a gap between them occupied
+	 * by the MPLS label stack.
+	 *
+	 * Thus it is possible to avoid traversing any VLAN tags that are
+	 * present to determine if the ethtype is MPLS. Instead the
+	 * inequality of mac_llen and skb_network_offset are used to
+	 * determine if a packet is MPLS for the purpose of determining
+	 * offload features.
+	 */
+	if (skb->mac_len && skb->mac_len != skb_network_offset(skb))
+		features &= dev->mpls_features;
+	return features;
+}
+#else
+static inline netdev_features_t net_mpls_features(struct sk_buff *skb,
+	struct net_device *dev, netdev_features_t features)
+{
+	return features;
+}
+#endif
+
 int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 			struct netdev_queue *txq)
 {
@@ -2576,6 +2610,8 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 		if (skb->encapsulation)
 			features &= dev->hw_enc_features;
 
+		features = net_mpls_features(skb, dev, features);
+
 		if (netif_needs_gso(skb, features)) {
 			if (unlikely(dev_gso_segment(skb, features)))
 				goto out_kfree_skb;
-- 
1.8.5.2

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 net-next repost] MPLS: Use mpls_features to activate software MPLS GSO segmentation
  2014-05-30  5:35 [PATCH v2 net-next repost] MPLS: Use mpls_features to activate software MPLS GSO segmentation Simon Horman
@ 2014-05-30 22:43 ` David Miller
       [not found]   ` <20140530.154318.305349726927694948.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
  0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2014-05-30 22:43 UTC (permalink / raw)
  To: horms; +Cc: netdev, jesse, tgraf, dev

From: Simon Horman <horms@verge.net.au>
Date: Fri, 30 May 2014 14:35:19 +0900

> If an MPLS packet requires segmentation then use mpls_features
> to determine if the software implementation should be used.
> 
> As no driver advertises MPLS GSO segmentation this will always be
> the case.
> 
> I had not noticed that this was necessary before as software MPLS GSO
> segmentation was already being used in my test environment. I believe that
> the reason for that is the skbs in question always had fragments and the
> driver I used does not advertise NETIF_F_FRAGLIST (which seems to be the
> case for most drivers). Thus software segmentation was activated by
> skb_gso_ok().
> 
> Thanks to Jesse Gross for prompting me to investigate this.
> 
> Acked-by: Jesse Gross <jesse@nicira.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>

Just requesting some stylistic changes:

> +/* If MPLS offload request, verify we are testing hardware MPLS features
> + * instead of standard features for the netdev.
> + */
> +#ifdef CONFIG_NET_MPLS_GSO
> +static inline netdev_features_t net_mpls_features(struct sk_buff *skb,
> +	struct net_device *dev, netdev_features_t features)

Do not use inline in a foo.c file, let the compiler make the decision.

Also, line up the arguments on the second line correctly, they should
start exactly at the first column after the openning parenthesis of
the previous line.

> +static inline netdev_features_t net_mpls_features(struct sk_buff *skb,
> +	struct net_device *dev, netdev_features_t features)
> +{
> +	return features;
> +}

Similarly.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 net-next repost] MPLS: Use mpls_features to activate software MPLS GSO segmentation
       [not found]   ` <20140530.154318.305349726927694948.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
@ 2014-06-02  0:08     ` Simon Horman
  0 siblings, 0 replies; 3+ messages in thread
From: Simon Horman @ 2014-06-02  0:08 UTC (permalink / raw)
  To: David Miller; +Cc: netdev-u79uwXL29TY76Z2rM5mHXA, dev-yBygre7rU0TnMu66kgdUjQ

On Fri, May 30, 2014 at 03:43:18PM -0700, David Miller wrote:
> From: Simon Horman <horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
> Date: Fri, 30 May 2014 14:35:19 +0900
> 
> > If an MPLS packet requires segmentation then use mpls_features
> > to determine if the software implementation should be used.
> > 
> > As no driver advertises MPLS GSO segmentation this will always be
> > the case.
> > 
> > I had not noticed that this was necessary before as software MPLS GSO
> > segmentation was already being used in my test environment. I believe that
> > the reason for that is the skbs in question always had fragments and the
> > driver I used does not advertise NETIF_F_FRAGLIST (which seems to be the
> > case for most drivers). Thus software segmentation was activated by
> > skb_gso_ok().
> > 
> > Thanks to Jesse Gross for prompting me to investigate this.
> > 
> > Acked-by: Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
> > Signed-off-by: Simon Horman <horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
> 
> Just requesting some stylistic changes:
> 
> > +/* If MPLS offload request, verify we are testing hardware MPLS features
> > + * instead of standard features for the netdev.
> > + */
> > +#ifdef CONFIG_NET_MPLS_GSO
> > +static inline netdev_features_t net_mpls_features(struct sk_buff *skb,
> > +	struct net_device *dev, netdev_features_t features)
> 
> Do not use inline in a foo.c file, let the compiler make the decision.
> 
> Also, line up the arguments on the second line correctly, they should
> start exactly at the first column after the openning parenthesis of
> the previous line.
> 
> > +static inline netdev_features_t net_mpls_features(struct sk_buff *skb,
> > +	struct net_device *dev, netdev_features_t features)
> > +{
> > +	return features;
> > +}
> 
> Similarly.

Thanks, I'll fix things up and repost.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2014-06-02  0:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-30  5:35 [PATCH v2 net-next repost] MPLS: Use mpls_features to activate software MPLS GSO segmentation Simon Horman
2014-05-30 22:43 ` David Miller
     [not found]   ` <20140530.154318.305349726927694948.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2014-06-02  0:08     ` Simon Horman

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).