On 13 Jul 2022, at 19:07, Ilya Maximets wrote:
> 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.
Thanks for fixing my errors, and applying it :)
//Eelco
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
