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?
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev