| Message ID | 1468808649-24552-4-git-send-email-pshelar@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
On Mon, Jul 18, 2016 at 4:24 AM, Pravin B Shelar <pshelar@ovn.org> wrote: > Upstream commit: > commit fe3a5f6c795810edb1646a840fec3c8c350c2a4e > Author: Simon Horman <simon.horman@netronome.com> > > openvswitch: allow output of MPLS packets on tunnel vports > > Currently output of MPLS packets on tunnel vports is not allowed by Open > vSwitch. This is because historically encapsulation was done in such a way > that the inner_protocol field of the skb needed to hold the inner protocol > for both MPLS and tunnel encapsulation in order for GSO segmentation to be > performed correctly. > > Since b2acd1dc3949 ("openvswitch: Use regular GRE net_device instead of > vport") Open vSwitch makes use of lwt to output to tunnel netdevs which > perform encapsulation. As no drivers expose support for MPLS offloads this > means that GSO packets are segmented in software by validate_xmit_skb(), > which is called from __dev_queue_xmit(), before tunnel encapsulation occurs. > This means that the inner protocol of MPLS is no longer needed by the time > encapsulation occurs and the contention on the inner_protocol field of the > skb no longer occurs. > > Thus it is now safe to output MPLS to tunnel vports. > > Signed-off-by: Simon Horman <simon.horman@netronome.com> > Reviewed-by: Jesse Gross <jesse@kernel.org> > Signed-off-by: David S. Miller <davem@davemloft.net> > > Signed-off-by: Pravin B Shelar <pshelar@ovn.org> I'm still somewhat concerned about this since it makes USE_UPSTREAM_TUNNEL effectively a user-facing toggle - the effects of it will be visible all the way out to the OpenFlow layer since it will be change what flows can be installed (and work). If we need to continue to bump up the kernel version where we use our own tunneling then it will keep changing whether these flows work. I think the ideal thing to would be to actually run the offload code before transmitting to tunnels. The next best thing would be to enable actually using the device layer on tunnels for kernels that support lightweight tunnels. I guess both of these are probably disproportionally complicated to the actual use case here and this isn't a regression, so for the time being: Acked-by: Jesse Gross <jesse@kernel.org>
On Mon, Jul 18, 2016 at 7:35 AM, Jesse Gross <jesse@kernel.org> wrote: > On Mon, Jul 18, 2016 at 4:24 AM, Pravin B Shelar <pshelar@ovn.org> wrote: >> Upstream commit: >> commit fe3a5f6c795810edb1646a840fec3c8c350c2a4e >> Author: Simon Horman <simon.horman@netronome.com> >> >> openvswitch: allow output of MPLS packets on tunnel vports >> >> Currently output of MPLS packets on tunnel vports is not allowed by Open >> vSwitch. This is because historically encapsulation was done in such a way >> that the inner_protocol field of the skb needed to hold the inner protocol >> for both MPLS and tunnel encapsulation in order for GSO segmentation to be >> performed correctly. >> >> Since b2acd1dc3949 ("openvswitch: Use regular GRE net_device instead of >> vport") Open vSwitch makes use of lwt to output to tunnel netdevs which >> perform encapsulation. As no drivers expose support for MPLS offloads this >> means that GSO packets are segmented in software by validate_xmit_skb(), >> which is called from __dev_queue_xmit(), before tunnel encapsulation occurs. >> This means that the inner protocol of MPLS is no longer needed by the time >> encapsulation occurs and the contention on the inner_protocol field of the >> skb no longer occurs. >> >> Thus it is now safe to output MPLS to tunnel vports. >> >> Signed-off-by: Simon Horman <simon.horman@netronome.com> >> Reviewed-by: Jesse Gross <jesse@kernel.org> >> Signed-off-by: David S. Miller <davem@davemloft.net> >> >> Signed-off-by: Pravin B Shelar <pshelar@ovn.org> > > I'm still somewhat concerned about this since it makes > USE_UPSTREAM_TUNNEL effectively a user-facing toggle - the effects of > it will be visible all the way out to the OpenFlow layer since it will > be change what flows can be installed (and work). If we need to > continue to bump up the kernel version where we use our own tunneling > then it will keep changing whether these flows work. > > I think the ideal thing to would be to actually run the offload code > before transmitting to tunnels. The next best thing would be to enable > actually using the device layer on tunnels for kernels that support > lightweight tunnels. > I thought about running compat-dev-xmit function here, but it would complicate code I am not sure if it is worth it. we can revisit it later on if required. > I guess both of these are probably disproportionally complicated to > the actual use case here and this isn't a regression, so for the time > being: > Acked-by: Jesse Gross <jesse@kernel.org> Thanks for review. I pushed first four patches to master.
diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c index fb41f28..0f32664 100644 --- a/datapath/flow_netlink.c +++ b/datapath/flow_netlink.c @@ -2046,9 +2046,10 @@ static int validate_set(const struct nlattr *a, break; case OVS_KEY_ATTR_TUNNEL: +#ifndef USE_UPSTREAM_TUNNEL if (eth_p_mpls(eth_type)) return -EINVAL; - +#endif if (masked) return -EINVAL; /* Masked tunnel set not supported. */
Upstream commit: commit fe3a5f6c795810edb1646a840fec3c8c350c2a4e Author: Simon Horman <simon.horman@netronome.com> openvswitch: allow output of MPLS packets on tunnel vports Currently output of MPLS packets on tunnel vports is not allowed by Open vSwitch. This is because historically encapsulation was done in such a way that the inner_protocol field of the skb needed to hold the inner protocol for both MPLS and tunnel encapsulation in order for GSO segmentation to be performed correctly. Since b2acd1dc3949 ("openvswitch: Use regular GRE net_device instead of vport") Open vSwitch makes use of lwt to output to tunnel netdevs which perform encapsulation. As no drivers expose support for MPLS offloads this means that GSO packets are segmented in software by validate_xmit_skb(), which is called from __dev_queue_xmit(), before tunnel encapsulation occurs. This means that the inner protocol of MPLS is no longer needed by the time encapsulation occurs and the contention on the inner_protocol field of the skb no longer occurs. Thus it is now safe to output MPLS to tunnel vports. Signed-off-by: Simon Horman <simon.horman@netronome.com> Reviewed-by: Jesse Gross <jesse@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Pravin B Shelar <pshelar@ovn.org> --- datapath/flow_netlink.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)