On Mon, Jun 6, 2016 at 7:46 PM, Simon Horman <simon.hor...@netronome.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.hor...@netronome.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? _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev