netdev - Re: [PATCH v2.39 7/7] datapath: Add basic MPLS support to kernel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEP_g=8+osWcHjj6pkGkid6djK9=CDf4U4qC97bY=GP0j=K3iw@mail.gmail.com>
Date:	Thu, 19 Sep 2013 12:31:51 -0500
From:	Jesse Gross <jesse@...ira.com>
To:	Simon Horman <horms@...ge.net.au>
Cc:	Pravin Shelar <pshelar@...ira.com>,
	"dev@...nvswitch.org" <dev@...nvswitch.org>,
	netdev <netdev@...r.kernel.org>, Ravi K <rkerur@...il.com>,
	Isaku Yamahata <yamahata@...inux.co.jp>,
	Joe Stringer <joe@...d.net.nz>
Subject: Re: [PATCH v2.39 7/7] datapath: Add basic MPLS support to kernel

On Wed, Sep 18, 2013 at 5:07 PM, Simon Horman <horms@...ge.net.au> wrote:
> On Tue, Sep 17, 2013 at 11:38:18AM -0700, Pravin Shelar wrote:
>> On Mon, Sep 9, 2013 at 12:20 AM, Simon Horman <horms@...ge.net.au> wrote:
>> > diff --git a/datapath/datapath.h b/datapath/datapath.h
>> > index 5d50dd4..babae3b 100644
>> > --- a/datapath/datapath.h
>> > +++ b/datapath/datapath.h
>> > @@ -36,6 +36,10 @@
>> >
>> >  #define SAMPLE_ACTION_DEPTH 3
>> >
>> > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,11,0)
>> > +#define HAVE_INNER_PROTOCOL
>> > +#endif
>> > +
>> >  /**
>> >   * struct dp_stats_percpu - per-cpu packet processing statistics for a given
>> >   * datapath.
>> > @@ -93,11 +97,16 @@ struct datapath {
>> >   * @pkt_key: The flow information extracted from the packet.  Must be nonnull.
>> >   * @tun_key: Key for the tunnel that encapsulated this packet. NULL if the
>> >   * packet is not being tunneled.
>> > + * @inner_protocol: Provides a substitute for the skb->inner_protocol field on
>> > + * kernels before 3.11.
>> >   */
>> >  struct ovs_skb_cb {
>> >         struct sw_flow          *flow;
>> >         struct sw_flow_key      *pkt_key;
>> >         struct ovs_key_ipv4_tunnel  *tun_key;
>> > +#ifndef HAVE_INNER_PROTOCOL
>> > +       __be16                  inner_protocol;
>> > +#endif
>> >  };
>> >  #define OVS_CB(skb) ((struct ovs_skb_cb *)(skb)->cb)
>> >
>> Can you move this to compat struct ovs_gso_cb {}
>
> I think that you are correct and inner_protocol needs
> to be in struct ovs_gso_cb so that it can
> be accessed via skb_network_protocol() from
> rpl___skb_gso_segment().
>
> However I think it may also need to be present in struct ovs_cb
> so that it can be set correctly.
>
> Currently it is set unconditionally
> in ovs_execute_actions() and Jesse has suggested setting
> it conditionally in pop_mpls() (which is called by do_execute_actions()).
> But regardless it seems to me that the field would need to be available
> in struct ovs_cb.

Since the helper functions are also in the compat code, I think they
should have access to ovs_gso_cb.

>> I think we can simplify code by pushing vlan and then segmenting skb,
>> the way we do it for MPLS.
>
> Are you thinking of something like the following which applies
> prior to the MPLS code.

This is basically what I was thinking about. We might actually be able
to move all of this to compat code by having a replacement for
dev_queue_xmit() similar to what we have for ip_local_out() in the
tunnel code.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html