On Tue, Nov 21, 2017 at 01:14:53PM +0000, Stokes, Ian wrote: > > On Fri, Nov 17, 2017 at 07:27:51PM +0000, Stokes, Ian wrote: > > > The following changes since commit > > 1ae83bb20677b42d63dbb2140fa8ed3144c6260f: > > > > > > netdev-tc-offloads: Add support for action set (2017-11-16 08:10:29 > > > -0800) > > > > > > are available in the git repository at: > > > > > > https://github.com/istokes/ovs dpdk_merge > > > > > > for you to fetch changes up to af5b0dad30359d14d6412eb80e81783a83a678ce: > > > > > > netdev-dpdk: Fix mempool creation with large MTU. (2017-11-17 > > > 16:26:33 +0000) > > > > Thanks a lot. I merged this branch into master. > > Thanks Ben. > > > > > This yields a new "sparse" warning: > > ../lib/netdev-tc-offloads.c:584:20: warning: Variable length array is > > used. > > because of this declaration in parse_put_flow_set_masked_action(): > > char *set_buff[set_len], *set_data, *set_mask; > > > > So this was introduced in commit 1ae83bb20677b42d63dbb2140fa8ed3144c6260f: > netdev-tc-offloads: Add support for action set. This was pushed just after I > had submitted the pull request (but wasn't actually part of the pull request > patches). > > It's strange though, I ran sparse before submitting the pull request and it > came back clean. Checking after the merge and I don't see the warning you > see. (Normally I would not submit a pull request if a sparse warning > occurred). What version of sparse are you using out of interest? I'm using > sparse-0.5.0-10. > > > It may or may not be worth fixing that. In favor of fixing it is keeping > > OVS sparse warning free, but against it is that the replacement would > > probably be malloc(), which is slower. > > > > However, looking a bit closer (now that I've already done the merge), I > > think there is a bug here. set_buff[] is declared as char > > *set_buff[set_len], which has size (set_len * sizeof(char *)), but only > > set_len bytes of it are used. I think that the right declaration would be > > more like uint8_t set_buff[set_len];, although that would lack the proper > > alignment for struct nl_attr. > > > > Maybe something like this would be the appropriate fix for both issues. > > Will you please review it? > > Sure, will be happy to review, but would also be good to get someone with > tc-offload experience to sign off also. > > Roi has also submitted a patch to fix the issue, maybe Roi could review below > also?
Roi's patch got merged, so that fixes the warning. I still think there is some merit in my additional changes, so I'll submit a revised patch. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
