Hi Aviv, On Sun, Sep 24, 2017 at 05:05:42PM +0000, Aviv Heller wrote: > > coverity-detected anomalies are sometimes subtle. So I looked at this > > code and found a couple of bugs. I also did some tidying. But no > > testing! > > > > Aviv, Antony: please have a look at commit > > f7aaa80198851ef71da4be8dc54e816f7064e29a > > Hi Hugh, Antony, > > Hugh, thanks for bringing this up, there are indeed a few bugs in the code. > > > Note: % 31 is almost never correct. I think that in this code, + 31 is > > also wrong. > > The (% 31) is indeed incorrect and should be (% 32). > Also, another bug fixed in your patch is the strneq() being compared to 0 > instead of 1. > > Digging a little into ethtool ioctl code, the size passed (and > correspondingly the array length) in cmd->size to ETHTOOL_GFEATURES should be > the total number of blocks (dwords) possible for all options, or basically, > (sset_len + 31) / 32 [put in the numbers and see that this calculation is > correct]. > Failing to satisfy this will result in failure in case our ESP bit is no > longer located in the last dword. > > > Could someone with real hardware test this code? > It works, with the limitation described in '2' above.
is nic-offload=auto broken now in master? If + 31 is correct f7aaa80198851 need fixing? - unsigned block = netlink_esp_hw_offload / 32; + unsigned block = (netlink_esp_hw_offload + 31) / 32; > > I cannot see that the old code would use NIC offload since the test for > > the feature seemed to be backwards. > > > > Of course I could be backwards. > No, it indeed didn't work. > > I also created a patch with all these fixes based on the original code, in > case it is needed. Did you send a patch? I do not have anything else other than the old one, [Swan-dev] [PATCH libreswan v2], Sep 04, 2017 regards, -antony _______________________________________________ Swan-dev mailing list [email protected] https://lists.libreswan.org/mailman/listinfo/swan-dev
