On Dec 12, 2012, at 18:23 , ext Jesse Gross wrote: > On Wed, Dec 12, 2012 at 6:34 AM, Jarno Rajahalme > <jarno.rajaha...@nsn.com> wrote: >> This version adds odp-level support for tunnel metadata. >> >> Currently it seems the datapath expects userspace to specify all the >> tunnel fields, or none of them. IMO, it would be better to use of >> configured values as defaults, allowing flow entries to provide only >> some of the fields (e.g., tun_dst). >> >> This is obviously similar to how any other field changes are viewed >> at the OF level, but tunnel metadata is different since packets don't >> have the metadata when they arrive on a normal port, but the tunnel >> metadata is figured out only when the output port is determined >> (which may be taken from a register if I'm not mistaken). >> >> So, is the design intent that the userspace code digs the missing >> values from the tunnel configuration data? Currently, it seems that >> data is readily available in the datapath when it is needed, but maybe >> doing it "once" on the userspace is better than figuring out which >> values to use on the datapath for each packet separately? > > The configuration information is no longer going to be available at > the kernel level soon. This will allow userspace to be more flexible > about the policies that it implements. > > Please separate out the ODP changes from the OpenFlow changes. I > can't apply the OpenFlow portions until all of the issues that I > mentioned before have been resolved but I could apply the ODP > component now. >
OK. > Is the ODP code based on the patch that I linked to before? If so, it > makes some changes that I don't understand, particularly things like > dropping comments and unit tests. > I hadn't seen your patch, sorry. I was just trying to fill the gaps between the datapath and the Openflow-level patches I sent earlier. You had used memcmp() on struct flow_tnl. Currently that would work fine, given that flow_tnl seems to have no need for compiler generated padding. It might be better still to compare the individual fields to be more explicit and future/comiler proof? > In any case, flags should be converted between userspace and kernel. > The same set of flags happen to be defined now with the same values > but it shouldn't be assumed that is the case. In particular, if there > are different versions of userspace and kernel then new flags that the > other side doesn't know about shouldn't just get carried over. This > can result in not being able to setup flows properly. This should probably be marked with BUILD_ASSERT_DECL(FLOW_WC_SEQ == 18) to remind about updating the flags mapping when they change? _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
