On 7/13/22 16:52, Eelco Chaudron wrote:
>
>
> On 13 Jul 2022, at 16:49, Ilya Maximets wrote:
>
>> On 7/13/22 11:40, Eelco Chaudron wrote:
>>> Sorry Mike/Roi, but for some reason, stgit removed you from the CC list
>>> while sending out this patch.
>>>
>>> //Eelco
>>>
>>>
>>> On 13 Jul 2022, at 10:19, Eelco Chaudron wrote:
>>>
>>>> This series adds support for the datapath action check_pkt_len for
>>>> TC offload. It also includes some offload self-tests.
>>>>
>>>> I had chat offline with Mike, and he is ok with keeping his ACK
>>>> on the changes in patch 3, however, I did remove Roi's.
>>>>
>>>> v2:
>>>> - Add ACKs
>>>> - Unified all the OVS_TRAFFIC_VSWITCHD_START macro's
>>>> - Added section in the NEWS document
>>>> v3:
>>>> - Add ACKs
>>>> - Was using TCA_CSUM_PARMS instead of TCA_POLICE_TBF
>>>> v4:
>>>> - Add ACKs
>>>> - Use the existing dbinit-aux-args argument, rather than
>>>> creating a new pre-vswitchd command option.
>>>> - Removed ACKs for patch 4/5
>>>> v5:
>>>> - Sync with upstream branch.
>>>>
>>>> Eelco Chaudron (5):
>>>> netdev-offload-tc: Move flow_put action handling to isolated
>>>> function.
>>>> netdev-offload-tc: Move flower_to_match action handling to isolated
>>>> function.
>>>> netdev-offload-tc: Handle check_pkt_len datapath action.
>>>> system-offloads-traffic: Properly initialize offload before testing.
>>>> tests: Add check_pkt_len action test to system-offload-traffic.
>>>>
>>>>
>>>> NEWS | 1 +
>>>> lib/netdev-offload-tc.c | 885 +++++++++++++++++++------------
>>>> lib/ofp-actions.c | 2 +-
>>>> lib/tc.c | 443 ++++++++++++++--
>>>> lib/tc.h | 22 +-
>>>> tests/ofproto-macros.at | 3 +-
>>>> tests/system-kmod-macros.at | 4 +-
>>>> tests/system-offloads-traffic.at | 432 ++++++++++++++-
>>>> tests/system-tso-macros.at | 4 +-
>>>> tests/system-userspace-macros.at | 4 +-
>>>> 10 files changed, 1391 insertions(+), 409 deletions(-)
>>
>> Hi, Eelco. Thanks for the rebase!
>>
>> The series looks fine too me and tests are working.
>> I made a few cosmetic changes and fixed a couple of small issues
>> along the review. I can apply the patch set, if the changes looks
>> good to you:
>>
>> 1. s/_parse_tc_flower_to_actions/parse_tc_flower_to_actions__/
>> This is documented in the coding style.
>>
>> 2. s/AT_CHECK_ACTIONS/OVS_CHECK_ACTIONS/
>> We shouldn't create new macros with AT_ prefix, these are for
>> native aototest macros.
>>
>> 3. Following change:
>>
>> diff --git a/lib/tc.c b/lib/tc.c
>> index 090e91671..25e66b5c5 100644
>> --- a/lib/tc.c
>> +++ b/lib/tc.c
>> @@ -2816,7 +2816,7 @@ rewrite_pedits_need_csum_update(struct tc_action
>> *action)
>> &first_word_mask, &mask, &data);
>>
>> for (j = 0; j < cnt; j++, mask++) {
>> - ovs_be32 mask_word = *mask;
>> + ovs_be32 mask_word = get_unaligned_be32(mask);
>>
>> if (j == 0) {
>> mask_word &= first_word_mask;
>> ---
>> See
>> https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/
>> for details.
>>
>> 4. Fixed tso and userspace test macros as they are not fully correct:
>>
>> - _OVS_VSWITCHD_START([--disable-system] [] [$3])
>> + _OVS_VSWITCHD_START([--disable-system], [$3])
>>
>>
>> - _OVS_VSWITCHD_START([--disable-system] [$3])
>> + _OVS_VSWITCHD_START([--disable-system], [$3])
>>
>>
>> What do you think?
>
> Thanks for the additional review, and the changes look good to me.
Thanks! Applied.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
