[ovs-dev] [PATCH v2] packets: use flow protocol when recalculating ipv6 checksums

[Simon Horman]

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 <jrajahalme at nicira.com>
> > Fixes: 6d670e7f0d45 ("lib/odp: Masked set action execution and printing.")
> > Signed-off-by: Simon Horman <simon.horman at 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 <blp at 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.