I was also waiting on this patch to ensure it goes into RHEL. 

Sent from my iPhone

> On Oct 4, 2017, at 14:14, Antony Antony <[email protected]> wrote:
> 
> 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

_______________________________________________
Swan-dev mailing list
[email protected]
https://lists.libreswan.org/mailman/listinfo/swan-dev

Reply via email to