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
| ||
|
Date: Fri, 4 Oct 2013 09:21:33 -0700 From: Ben Pfaff <blp@...ira.com> To: Simon Horman <horms@...ge.net.au> Cc: dev@...nvswitch.org, netdev@...r.kernel.org, Jesse Gross <jesse@...ira.com>, Pravin B Shelar <pshelar@...ira.com>, Ravi K <rkerur@...il.com>, Isaku Yamahata <yamahata@...inux.co.jp>, Joe Stringer <joe@...d.net.nz> Subject: Re: [PATCH v2.42 1/5] odp: Allow VLAN actions after MPLS actions On Fri, Oct 04, 2013 at 05:09:56PM +0900, Simon Horman wrote: > From: Joe Stringer <joe@...d.net.nz> > > OpenFlow 1.1 and 1.2, and 1.3 differ in their handling of MPLS actions in the > presence of VLAN tags. To allow correct behaviour to be committed in > each situation, this patch adds a second round of VLAN tag action > handling to commit_odp_actions(), which occurs after MPLS actions. This > is implemented with a new field in 'struct xlate_in' called 'vlan_tci'. > > When an push_mpls action is composed, the flow's current VLAN state is > stored into xin->vlan_tci, and flow->vlan_tci is set to 0 (pop_vlan). If > a VLAN tag is present, it is stripped; if not, then there is no change. > Any later modifications to the VLAN state is written to xin->vlan_tci. > When committing the actions, flow->vlan_tci is used before MPLS actions, > and xin->vlan_tci is used afterwards. This retains the current datapath > behaviour, but allows VLAN actions to be applied in a more flexible > manner. > > Both before and after this patch MPLS LSEs are pushed onto a packet after > any VLAN tags that may be present. This is the behaviour described in > OpenFlow 1.1 and 1.2. OpenFlow 1.3 specifies that MPLS LSEs should be > pushed onto a packet before any VLAN tags that are present. Support > for this will be added by a subsequent patch that makes use of > the infrastructure added by this patch. > > Signed-off-by: Joe Stringer <joe@...d.net.nz> > Signed-off-by: Simon Horman <horms@...ge.net.au> I noticed a couple more minor points. First, it seems to me that the "vlan_tci" member that this adds to xlate_in could go in xlate_ctx just as well. I would prefer that, because (as far as I can tell) there is no reason for the client to use any value other than flow->vlan_tci here, and putting it in xlate_ctx hides it from the client. Thanks for rearranging the code and updating the comment in do_xlate_actions(). It makes the operation clearer. But now that it's clear I have an additional question. Does it really make sense to have 'vlan_tci' as only a local variable in do_xlate_actions()? Presumably, MPLS and VLANs should interact the same way regardless of whether they are separated by resubmits or goto_tables. That is, I suspect that this is xlate_ctx state, not local state. Thanks, Ben. -- 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
Powered by blists - more mailing lists