All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Ahern <dsa@cumulusnetworks.com>
To: Simon Horman <simon.horman@netronome.com>,
	Pravin B Shelar <pshelar@nicira.com>
Cc: netdev@vger.kernel.org, davem@davemloft.net,
	buytenh@wantstofly.org, ebiederm@xmission.com,
	rshearma@brocade.com, tom@herbertland.com, tgraf@suug.ch,
	olivier.dugeon@orange.com, alexander.duyck@gmail.com,
	roopa@cumulusnetworks.com
Subject: Re: [PATCH net-next 2/3] net: mpls: Fixups for GSO
Date: Tue, 23 Aug 2016 13:24:51 -0600	[thread overview]
Message-ID: <3297517c-2889-19fe-5aa4-89e1d14eadca@cumulusnetworks.com> (raw)
In-Reply-To: <20160822145134.GA26491@penelope.isobedori.kobe.vergenet.net>

On 8/22/16 8:51 AM, Simon Horman wrote:
> 
> The scheme that OvS uses so far is that mac_len denotes the number of bytes
> from the start of the MAC header until its end. In the absence of MPLS that
> will be the beginning of the network header. And in the presence of MPLS it
> will be the beginning of the MPLS label stack. The network header is... the
> network header. This allows the MAC header, MPLS label stack and network
> header to be tracked.

The neigh output functions do '__skb_pull(skb, skb_network_offset(skb))' so if mpls_xmit does not reset the network header the labels get dropped. To me this says MPLS labels can not be lumped with the mac header which leaves the only option as the outer network header.

> 
> Pravin (CCed) may have different ideas but I wonder if the above scheme can
> be preserved while also meeting the needs of your new MPLS GSO scheme if
> you set skb_set_network_header() and skb_set_inner_network_header() in
> net/openvswitch/actions.c:do_output().
> 
> It may also be possible to teach OvS to use skb_set_network_header to
> denote the beginning of the MPLS LSE and skb_set_inner_network_header to
> denote the network header in the presence of MPLS. Which is my current
> understanding of what you are trying to achieve. But I think its likely
> that I misunderstand things as it seems strange to me to pretend that an
> MPLS LSE is a network header and the outer most network header is an inner
> network header
> 

This is the only option I can see working, but open to patches showing an alternative.

I would like to get it resolved this week so I can move on to gso in the mpls forward case.

  reply	other threads:[~2016-08-23 19:32 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-19 17:08 [PATCH v3 net-next 0/3] net: mpls: fragmentation and gso fixes for locally originated traffic David Ahern
2016-08-19 17:09 ` [PATCH net-next 1/3] net: lwtunnel: Handle fragmentation David Ahern
2016-08-19 17:09 ` [PATCH net-next 2/3] net: mpls: Fixups for GSO David Ahern
2016-08-19 20:17   ` Alexander Duyck
2016-08-22 12:21   ` Simon Horman
2016-08-22 13:11     ` David Ahern
2016-08-22 14:51       ` Simon Horman
2016-08-23 19:24         ` David Ahern [this message]
2016-08-24  7:20           ` Simon Horman
2016-08-24 16:28             ` pravin shelar
2016-08-24 16:37               ` David Ahern
2016-08-24 17:41                 ` pravin shelar
2016-08-24 18:53                   ` David Ahern
2016-08-25  3:12                     ` David Ahern
2016-08-25  3:58                     ` pravin shelar
2016-09-26 15:56                 ` Jiri Benc
2016-09-26 17:02                   ` Jiri Benc
2016-09-27  2:04                     ` David Ahern
2016-09-27  7:45                       ` Jiri Benc
2016-09-27 16:38                         ` David Ahern
2016-09-27 16:45                           ` Jiri Benc
2016-08-19 17:09 ` [PATCH net-next 3/3] net: veth: Set features for MPLS David Ahern
  -- strict thread matches above, loose matches on Subject: below --
2016-08-17 21:49 [PATCH v2 net-next 0/3] net: mpls: fragmentation and gso fixes for locally originated traffic David Ahern
2016-08-17 21:49 ` [PATCH net-next 2/3] net: mpls: Fixups for GSO David Ahern
2016-08-17 23:16   ` Alexander Duyck
2016-08-17 23:23     ` David Ahern
2016-08-18  1:06       ` Alexander Duyck
2016-08-18  2:59         ` David Ahern
2016-08-18 14:37   ` Alexander Duyck
2016-08-18 16:18     ` David Ahern

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3297517c-2889-19fe-5aa4-89e1d14eadca@cumulusnetworks.com \
    --to=dsa@cumulusnetworks.com \
    --cc=alexander.duyck@gmail.com \
    --cc=buytenh@wantstofly.org \
    --cc=davem@davemloft.net \
    --cc=ebiederm@xmission.com \
    --cc=netdev@vger.kernel.org \
    --cc=olivier.dugeon@orange.com \
    --cc=pshelar@nicira.com \
    --cc=roopa@cumulusnetworks.com \
    --cc=rshearma@brocade.com \
    --cc=simon.horman@netronome.com \
    --cc=tgraf@suug.ch \
    --cc=tom@herbertland.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.