netdev - Re: [PATCH net-next v10 4/5] openvswitch: add layer 3 flow/port support

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 7 Jun 2016 15:45:58 -0700
From:	pravin shelar <pshelar@....org>
To:	Simon Horman <simon.horman@...ronome.com>
Cc:	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	ovs dev <dev@...nvswitch.org>
Subject: Re: [PATCH net-next v10 4/5] openvswitch: add layer 3 flow/port support

On Mon, Jun 6, 2016 at 7:46 PM, Simon Horman <simon.horman@...ronome.com> wrote:
> On Thu, Jun 02, 2016 at 03:02:18PM -0700, pravin shelar wrote:
>> On Wed, Jun 1, 2016 at 11:24 PM, Simon Horman
>> <simon.horman@...ronome.com> wrote:
>
> [...]
>
>> > diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
>> > index 15f130e4c22b..5567529904fa 100644
>> > --- a/net/openvswitch/actions.c
>> > +++ b/net/openvswitch/actions.c
>> > @@ -300,6 +300,51 @@ static int set_eth_addr(struct sk_buff *skb, struct sw_flow_key *flow_key,
>> >         return 0;
>> >  }
>> >
>> > +static int pop_eth(struct sk_buff *skb, struct sw_flow_key *key)
>> > +{
>> > +       /* Pop outermost VLAN tag to skb metadata unless a VLAN tag
>> > +        * is already present there.
>> > +        */
>> > +       if ((skb->protocol == htons(ETH_P_8021Q) ||
>> > +            skb->protocol == htons(ETH_P_8021AD)) &&
>> > +           !skb_vlan_tag_present(skb)) {
>> > +               int err = skb_vlan_accel(skb);
>> > +               if (unlikely(err))
>> > +                       return err;
>> > +       }
>> > +
>>
>> I do not think we can keep just the vlan tag and pop ethernet header.
>> There are multiple issues with this.
>> First networking stack can not handle suck packet. second issue even
>> after this patch OVS can not parse this type of packet. third this
>> patch does not allow pop-eth action on vlan tagged packet.
>> There is already separate vlan related actions in OVS so lets keep it simple.
>
> I wonder if the best solution is to simply omit handling VLAN tags
> in pop_eth for now. As you mention pop_eth is not permitted on such packets.
>
yes, lets just drop vlan support here.


>> > diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
>> > index 0bb650f4f219..1e1392c3c0ed 100644
>> > --- a/net/openvswitch/flow_netlink.c
>> > +++ b/net/openvswitch/flow_netlink.c
>
> [...]
>
>> > @@ -355,6 +359,7 @@ static const struct ovs_len_tbl ovs_key_lens[OVS_KEY_ATTR_MAX + 1] = {
>> >         [OVS_KEY_ATTR_CT_ZONE]   = { .len = sizeof(u16) },
>> >         [OVS_KEY_ATTR_CT_MARK]   = { .len = sizeof(u32) },
>> >         [OVS_KEY_ATTR_CT_LABELS] = { .len = sizeof(struct ovs_key_ct_labels) },
>> > +       [OVS_KEY_ATTR_PACKET_ETHERTYPE] = { .len = sizeof(__be16) },
>> >  };
>> >
>> I do not see need for OVS_KEY_ATTR_PACKET_ETHERTYPE, we can use
>> existing OVS_KEY_ATTR_ETHERTYPE to serialize the flow key. If there is
>> no OVS_KEY_ATTR_ETHERNET attribute then its l3 packet.
>
> The idea of OVS_KEY_ATTR_PACKET_ETHERTYPE is to allow communication of
> the L2 type of the packet which is not present in an L3 packet. In terms
> of GRE (non-TEB) this correlates to the Protocol Type field in the GRE
> header.

How about using OVS_KEY_ATTR_ETHERTYPE to communicate the protocol type?