On Sat, Apr 23, 2016 at 03:12:49PM +1000, Simon Horman wrote:
> On Fri, Apr 22, 2016 at 10:10:38AM -0700, Ben Pfaff wrote:
> > On Fri, Apr 22, 2016 at 10:22:56PM +1000, Simon Horman wrote:
> > > When using masked actions the ipv6_proto field of an action
> > > to set IPv6 fields may be zero rather than the prevailing protocol
> > > which will result in skipping checksum recalculation.
> > > 
> > > This patch resolves the problem by relying on the protocol
> > > in the packet rather than that in the set field action.
> > > 
> > > A similar fix for the kernel datapath has been accepted into David 
> > > Miller's
> > > 'net' tree as b4f70527f052 ("openvswitch: use flow protocol when
> > > recalculating ipv6 checksums").
> > > 
> > > Cc: Jarno Rajahalme <jrajaha...@nicira.com>
> > > Fixes: 6d670e7f0d45 ("lib/odp: Masked set action execution and printing.")
> > > Signed-off-by: Simon Horman <simon.hor...@netronome.com>
> > > ---
> > > While preparing this I noticed that there does seem to be some scope
> > > to consolidate packet_rh_present() and part of miniflow_extract().
> > > 
> > > v2
> > > * Updated changelog to refer to protocol in packet, as this patch does,
> > >   rather than the flow key, which the kernel datapath variant of the patch
> > >   does. This was a copy-paste error in preparing the changelog.
> > 
> > GCC 4.9.1 gives me the following error:
> > 
> >     ../lib/packets.c: In function 'packet_set_ipv6':
> >     ../lib/packets.c:951:9: error: 'proto' may be used uninitialized in 
> > this function [-Werror=maybe-uninitialized]
> >              packet_update_csum128(packet, proto, addr, new_addr);
> >              ^
> >     ../lib/packets.c:1018:13: note: 'proto' was declared here
> >          uint8_t proto;
> >                  ^
> > 
> > Other than that, this seems like a good bug fix, thank you!
> > 
> > Acked-by: Ben Pfaff <b...@ovn.org>
> 
> Thanks.
> 
> * I have resolve the problem you highlighted by initialising proto to zero:
>       uint8_t proto = 0;
> * I have pushed the updated patch to master with your Ack.
> * I have also pushed a backport to branch-2.5 and branch-2.4 as
>   I was able to observe the problem there.
> * I also tested branch-2.3 but the problem does not appear to be present 
> there.

Great, thanks again!
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to