On Mon, Sep 23, 2013 at 06:38:23PM -0700, Jesse Gross wrote: > On Mon, Sep 23, 2013 at 6:32 PM, Simon Horman <ho...@verge.net.au> wrote: > > On Mon, Sep 23, 2013 at 02:17:50PM -0700, Jesse Gross wrote: > >> On Sat, Sep 21, 2013 at 10:34 PM, Simon Horman <ho...@verge.net.au> wrote: > >> > On Thu, Sep 19, 2013 at 12:21:33PM -0500, Jesse Gross wrote: > >> >> On Thu, Sep 19, 2013 at 10:57 AM, Simon Horman <ho...@verge.net.au> > >> >> wrote: > >> >> > On Mon, Sep 16, 2013 at 03:38:21PM -0500, Jesse Gross wrote: > >> >> >> On Mon, Sep 9, 2013 at 12:20 AM, Simon Horman <ho...@verge.net.au> > >> >> >> wrote: > >> >> One other consideration in the OVS case - with recirculation we may > >> >> hit this code multiple times and the difference in behavior could be > >> >> surprising. However, on the other hand, we need to be careful because > >> >> skb->cb is not guaranteed to be initialized to zero. > >> > > >> > Thanks, that is also not something that I had considered. > >> > > >> > I'm not sure, but I think that we can rely on skb->cb > >> > not being clobbered between rounds of recirculation. > >> > Or at the very least I think we could save and restore it > >> > as necessary. > >> > >> Yes, it should be safe to assume this. > >> > >> > So I think if we could be careful to make sure that inner_protocol > >> > is in a sane state the first time we see the skb but not > >> > each time it is recirculated then I think things should work out. > >> > > >> > In my current implementation of recirculation the datapath > >> > side is driven ovs_dp_process_received_packet(). So by my reasoning > >> > above I think it would make sense to reset the inner_protocol there > >> > and in ovs_packet_cmd_execute() rather than in ovs_execute_actions() > >> > which each of those functions call. > >> > >> I think that would work, however, I wonder if it's the right place in > >> general, independent of this compatibility issue. I guess it still > >> seems like the ideal thing to do is to move this as close to where it > >> is necessary as possible, specifically in mpls_push(). Is there a > >> reason to not put it there (again, other than the out-of-tree > >> compatibility issues)? > > > > I agree that should work, out-of-tree compatibility issues aside. > > > > Perhaps a solution is to have a conditional set_inner_protocol call inside > > push_mpls, where the condition is that inner_protocol is zero. > > And a reset_inner_protocol call earlier on, a call that sets inner_protocol > > to zero only if the compatibility code is in use and thus it resides in > > struct ovs_gso_cb. This call could be remove once the compatibility > > code is no longer needed, that is once kernels older than 3.11 are no > > longer supported. > > I agree that's probably the right solution.
Thanks, I will see about making it so. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
