> 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. > 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. Thanks, Aviv _______________________________________________ Swan-dev mailing list [email protected] https://lists.libreswan.org/mailman/listinfo/swan-dev
