| Message ID | 20240206094043.530335-27-amusil@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Remove most of the hardcoded table numbers | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
Hi Ales Thanks for the series and for making it "review friendly". I only had two comments for patches 1 to 26 - There are a few remaining table numbers in comments which will become wrong if table numbers are changing. Some are oflow tables for lflows, so should we just remove them / replace them by table=?? in the comments ? - The test "action parsing" still has quite a few table numbers. I sent a patch for (trying to) fix it. For patches 1-26: Acked-by: Xavier Simonart <xsimonar@redhat.com> Thanks Xavier On Tue, Feb 6, 2024 at 10:44 AM Ales Musil <amusil@redhat.com> wrote: > Add macro for OFTABLE_MAC_CACHE_USE and replace all table=79 > occurrences in OF with table=OFTABLE_MAC_CACHE_USE. > > Signed-off-by: Ales Musil <amusil@redhat.com> > --- > tests/ovn-macros.at | 1 + > tests/ovn.at | 12 ++++++------ > 2 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at > index 975b70143..84e50d76f 100644 > --- a/tests/ovn-macros.at > +++ b/tests/ovn-macros.at > @@ -1009,3 +1009,4 @@ m4_define([OFTABLE_CHK_OUT_PORT_SEC], [75]) > m4_define([OFTABLE_ECMP_NH_MAC], [76]) > m4_define([OFTABLE_ECMP_NH], [77]) > m4_define([OFTABLE_CHK_LB_AFFINITY], [78]) > +m4_define([OFTABLE_MAC_CACHE_USE], [79]) > diff --git a/tests/ovn.at b/tests/ovn.at > index 2f8aa4840..0bbf3d6da 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -34796,9 +34796,9 @@ wait_row_count mac_binding 1 ip="192.168.10.20" > dp_key=$(printf "0x%x" $(as hv1 fetch_column datapath tunnel_key > external_ids:name=gw)) > port_key=$(printf "0x%x" $(as hv1 fetch_column port_binding tunnel_key > logical_port=gw-public)) > > -AT_CHECK_UNQUOTED([as hv1 ovs-ofctl dump-flows br-int table=79 --no-stats > | strip_cookie | sort], [0], [dnl > - table=79, > priority=100,ip,reg14=${port_key},metadata=${dp_key},dl_src=00:00:00:00:10:10,nw_src=192.168.10.10 > actions=drop > - table=79, > priority=100,ip,reg14=${port_key},metadata=${dp_key},dl_src=00:00:00:00:10:20,nw_src=192.168.10.20 > actions=drop > +AT_CHECK_UNQUOTED([as hv1 ovs-ofctl dump-flows br-int > table=OFTABLE_MAC_CACHE_USE --no-stats | strip_cookie | sort], [0], [dnl > + table=OFTABLE_MAC_CACHE_USE, > priority=100,ip,reg14=${port_key},metadata=${dp_key},dl_src=00:00:00:00:10:10,nw_src=192.168.10.10 > actions=drop > + table=OFTABLE_MAC_CACHE_USE, > priority=100,ip,reg14=${port_key},metadata=${dp_key},dl_src=00:00:00:00:10:20,nw_src=192.168.10.20 > actions=drop > ]) > > timestamp=$(fetch_column mac_binding timestamp ip="192.168.10.20") > @@ -34806,8 +34806,8 @@ timestamp=$(fetch_column mac_binding timestamp > ip="192.168.10.20") > send_udp hv1 ext1 10 > send_udp hv2 ext2 20 > > -OVS_WAIT_UNTIL([as hv1 ovs-ofctl dump-flows br-int table=79 | grep > "192.168.10.10" | grep -q "n_packets=1"]) > -OVS_WAIT_UNTIL([as hv2 ovs-ofctl dump-flows br-int table=79 | grep > "192.168.10.20" | grep -q "n_packets=1"]) > +OVS_WAIT_UNTIL([as hv1 ovs-ofctl dump-flows br-int > table=OFTABLE_MAC_CACHE_USE | grep "192.168.10.10" | grep -q "n_packets=1"]) > +OVS_WAIT_UNTIL([as hv2 ovs-ofctl dump-flows br-int > table=OFTABLE_MAC_CACHE_USE | grep "192.168.10.20" | grep -q "n_packets=1"]) > > # Set the MAC binding aging threshold > AT_CHECK([ovn-nbctl set logical_router gw > options:mac_binding_age_threshold=5]) > @@ -34835,7 +34835,7 @@ OVS_WAIT_UNTIL([ > test "0" = "$(ovn-sbctl list mac_binding | grep -c '192.168.10.20')" > ]) > > -AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=79 --no-stats | > strip_cookie], [0], []) > +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_MAC_CACHE_USE > --no-stats | strip_cookie], [0], []) > > # Test CIDR-based threshold configuration > check ovn-nbctl set logical_router gw options:mac_binding_age_threshold=" > 192.168.10.0/255.255.255.0:2;192.168.10.64/26:0;192.168.10.20:0" > -- > 2.43.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
On Tue, Feb 6, 2024 at 3:55 PM Xavier Simonart <xsimonar@redhat.com> wrote: > Hi Ales > Hi Xavier, > > Thanks for the series and for making it "review friendly". > I only had two comments for patches 1 to 26 > - There are a few remaining table numbers in comments which will become > wrong if table numbers are changing. Some are oflow tables for lflows, so > should we just remove them / replace them by table=?? in the comments ? > I can prepare v3 including the comment changes, if there isn't anything major I will post the diff for them to ML under corresponding patch. > - The test "action parsing" still has quite a few table numbers. I sent a > patch for (trying to) fix it. > Thanks for that! > > For patches 1-26: > Acked-by: Xavier Simonart <xsimonar@redhat.com> > > Thanks > Xavier > > On Tue, Feb 6, 2024 at 10:44 AM Ales Musil <amusil@redhat.com> wrote: > >> Add macro for OFTABLE_MAC_CACHE_USE and replace all table=79 >> occurrences in OF with table=OFTABLE_MAC_CACHE_USE. >> >> Signed-off-by: Ales Musil <amusil@redhat.com> >> --- >> tests/ovn-macros.at | 1 + >> tests/ovn.at | 12 ++++++------ >> 2 files changed, 7 insertions(+), 6 deletions(-) >> >> diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at >> index 975b70143..84e50d76f 100644 >> --- a/tests/ovn-macros.at >> +++ b/tests/ovn-macros.at >> @@ -1009,3 +1009,4 @@ m4_define([OFTABLE_CHK_OUT_PORT_SEC], [75]) >> m4_define([OFTABLE_ECMP_NH_MAC], [76]) >> m4_define([OFTABLE_ECMP_NH], [77]) >> m4_define([OFTABLE_CHK_LB_AFFINITY], [78]) >> +m4_define([OFTABLE_MAC_CACHE_USE], [79]) >> diff --git a/tests/ovn.at b/tests/ovn.at >> index 2f8aa4840..0bbf3d6da 100644 >> --- a/tests/ovn.at >> +++ b/tests/ovn.at >> @@ -34796,9 +34796,9 @@ wait_row_count mac_binding 1 ip="192.168.10.20" >> dp_key=$(printf "0x%x" $(as hv1 fetch_column datapath tunnel_key >> external_ids:name=gw)) >> port_key=$(printf "0x%x" $(as hv1 fetch_column port_binding tunnel_key >> logical_port=gw-public)) >> >> -AT_CHECK_UNQUOTED([as hv1 ovs-ofctl dump-flows br-int table=79 >> --no-stats | strip_cookie | sort], [0], [dnl >> - table=79, >> priority=100,ip,reg14=${port_key},metadata=${dp_key},dl_src=00:00:00:00:10:10,nw_src=192.168.10.10 >> actions=drop >> - table=79, >> priority=100,ip,reg14=${port_key},metadata=${dp_key},dl_src=00:00:00:00:10:20,nw_src=192.168.10.20 >> actions=drop >> +AT_CHECK_UNQUOTED([as hv1 ovs-ofctl dump-flows br-int >> table=OFTABLE_MAC_CACHE_USE --no-stats | strip_cookie | sort], [0], [dnl >> + table=OFTABLE_MAC_CACHE_USE, >> priority=100,ip,reg14=${port_key},metadata=${dp_key},dl_src=00:00:00:00:10:10,nw_src=192.168.10.10 >> actions=drop >> + table=OFTABLE_MAC_CACHE_USE, >> priority=100,ip,reg14=${port_key},metadata=${dp_key},dl_src=00:00:00:00:10:20,nw_src=192.168.10.20 >> actions=drop >> ]) >> >> timestamp=$(fetch_column mac_binding timestamp ip="192.168.10.20") >> @@ -34806,8 +34806,8 @@ timestamp=$(fetch_column mac_binding timestamp >> ip="192.168.10.20") >> send_udp hv1 ext1 10 >> send_udp hv2 ext2 20 >> >> -OVS_WAIT_UNTIL([as hv1 ovs-ofctl dump-flows br-int table=79 | grep >> "192.168.10.10" | grep -q "n_packets=1"]) >> -OVS_WAIT_UNTIL([as hv2 ovs-ofctl dump-flows br-int table=79 | grep >> "192.168.10.20" | grep -q "n_packets=1"]) >> +OVS_WAIT_UNTIL([as hv1 ovs-ofctl dump-flows br-int >> table=OFTABLE_MAC_CACHE_USE | grep "192.168.10.10" | grep -q "n_packets=1"]) >> +OVS_WAIT_UNTIL([as hv2 ovs-ofctl dump-flows br-int >> table=OFTABLE_MAC_CACHE_USE | grep "192.168.10.20" | grep -q "n_packets=1"]) >> >> # Set the MAC binding aging threshold >> AT_CHECK([ovn-nbctl set logical_router gw >> options:mac_binding_age_threshold=5]) >> @@ -34835,7 +34835,7 @@ OVS_WAIT_UNTIL([ >> test "0" = "$(ovn-sbctl list mac_binding | grep -c '192.168.10.20')" >> ]) >> >> -AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=79 --no-stats | >> strip_cookie], [0], []) >> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_MAC_CACHE_USE >> --no-stats | strip_cookie], [0], []) >> >> # Test CIDR-based threshold configuration >> check ovn-nbctl set logical_router gw options:mac_binding_age_threshold=" >> 192.168.10.0/255.255.255.0:2;192.168.10.64/26:0;192.168.10.20:0" >> -- >> 2.43.0 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> Thanks, Ales
Thank you Ales and Xavier. I applied patches 1-26 to main as is. The patches to remove table numbers from comments can be uploaded as a separate series or part of v3 for patches 27-29. Mark Michelson On 2/6/24 10:01, Ales Musil wrote: > On Tue, Feb 6, 2024 at 3:55 PM Xavier Simonart <xsimonar@redhat.com> wrote: > >> Hi Ales >> > > Hi Xavier, > > >> >> Thanks for the series and for making it "review friendly". >> I only had two comments for patches 1 to 26 >> - There are a few remaining table numbers in comments which will become >> wrong if table numbers are changing. Some are oflow tables for lflows, so >> should we just remove them / replace them by table=?? in the comments ? >> > > I can prepare v3 including the comment changes, if there isn't anything > major I will post the diff for them to ML under corresponding patch. > > >> - The test "action parsing" still has quite a few table numbers. I sent a >> patch for (trying to) fix it. >> > > Thanks for that! > > >> >> For patches 1-26: >> Acked-by: Xavier Simonart <xsimonar@redhat.com> >> >> Thanks >> Xavier >> >> On Tue, Feb 6, 2024 at 10:44 AM Ales Musil <amusil@redhat.com> wrote: >> >>> Add macro for OFTABLE_MAC_CACHE_USE and replace all table=79 >>> occurrences in OF with table=OFTABLE_MAC_CACHE_USE. >>> >>> Signed-off-by: Ales Musil <amusil@redhat.com> >>> --- >>> tests/ovn-macros.at | 1 + >>> tests/ovn.at | 12 ++++++------ >>> 2 files changed, 7 insertions(+), 6 deletions(-) >>> >>> diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at >>> index 975b70143..84e50d76f 100644 >>> --- a/tests/ovn-macros.at >>> +++ b/tests/ovn-macros.at >>> @@ -1009,3 +1009,4 @@ m4_define([OFTABLE_CHK_OUT_PORT_SEC], [75]) >>> m4_define([OFTABLE_ECMP_NH_MAC], [76]) >>> m4_define([OFTABLE_ECMP_NH], [77]) >>> m4_define([OFTABLE_CHK_LB_AFFINITY], [78]) >>> +m4_define([OFTABLE_MAC_CACHE_USE], [79]) >>> diff --git a/tests/ovn.at b/tests/ovn.at >>> index 2f8aa4840..0bbf3d6da 100644 >>> --- a/tests/ovn.at >>> +++ b/tests/ovn.at >>> @@ -34796,9 +34796,9 @@ wait_row_count mac_binding 1 ip="192.168.10.20" >>> dp_key=$(printf "0x%x" $(as hv1 fetch_column datapath tunnel_key >>> external_ids:name=gw)) >>> port_key=$(printf "0x%x" $(as hv1 fetch_column port_binding tunnel_key >>> logical_port=gw-public)) >>> >>> -AT_CHECK_UNQUOTED([as hv1 ovs-ofctl dump-flows br-int table=79 >>> --no-stats | strip_cookie | sort], [0], [dnl >>> - table=79, >>> priority=100,ip,reg14=${port_key},metadata=${dp_key},dl_src=00:00:00:00:10:10,nw_src=192.168.10.10 >>> actions=drop >>> - table=79, >>> priority=100,ip,reg14=${port_key},metadata=${dp_key},dl_src=00:00:00:00:10:20,nw_src=192.168.10.20 >>> actions=drop >>> +AT_CHECK_UNQUOTED([as hv1 ovs-ofctl dump-flows br-int >>> table=OFTABLE_MAC_CACHE_USE --no-stats | strip_cookie | sort], [0], [dnl >>> + table=OFTABLE_MAC_CACHE_USE, >>> priority=100,ip,reg14=${port_key},metadata=${dp_key},dl_src=00:00:00:00:10:10,nw_src=192.168.10.10 >>> actions=drop >>> + table=OFTABLE_MAC_CACHE_USE, >>> priority=100,ip,reg14=${port_key},metadata=${dp_key},dl_src=00:00:00:00:10:20,nw_src=192.168.10.20 >>> actions=drop >>> ]) >>> >>> timestamp=$(fetch_column mac_binding timestamp ip="192.168.10.20") >>> @@ -34806,8 +34806,8 @@ timestamp=$(fetch_column mac_binding timestamp >>> ip="192.168.10.20") >>> send_udp hv1 ext1 10 >>> send_udp hv2 ext2 20 >>> >>> -OVS_WAIT_UNTIL([as hv1 ovs-ofctl dump-flows br-int table=79 | grep >>> "192.168.10.10" | grep -q "n_packets=1"]) >>> -OVS_WAIT_UNTIL([as hv2 ovs-ofctl dump-flows br-int table=79 | grep >>> "192.168.10.20" | grep -q "n_packets=1"]) >>> +OVS_WAIT_UNTIL([as hv1 ovs-ofctl dump-flows br-int >>> table=OFTABLE_MAC_CACHE_USE | grep "192.168.10.10" | grep -q "n_packets=1"]) >>> +OVS_WAIT_UNTIL([as hv2 ovs-ofctl dump-flows br-int >>> table=OFTABLE_MAC_CACHE_USE | grep "192.168.10.20" | grep -q "n_packets=1"]) >>> >>> # Set the MAC binding aging threshold >>> AT_CHECK([ovn-nbctl set logical_router gw >>> options:mac_binding_age_threshold=5]) >>> @@ -34835,7 +34835,7 @@ OVS_WAIT_UNTIL([ >>> test "0" = "$(ovn-sbctl list mac_binding | grep -c '192.168.10.20')" >>> ]) >>> >>> -AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=79 --no-stats | >>> strip_cookie], [0], []) >>> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_MAC_CACHE_USE >>> --no-stats | strip_cookie], [0], []) >>> >>> # Test CIDR-based threshold configuration >>> check ovn-nbctl set logical_router gw options:mac_binding_age_threshold=" >>> 192.168.10.0/255.255.255.0:2;192.168.10.64/26:0;192.168.10.20:0" >>> -- >>> 2.43.0 >>> >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> >>> > Thanks, > Ales
diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at index 975b70143..84e50d76f 100644 --- a/tests/ovn-macros.at +++ b/tests/ovn-macros.at @@ -1009,3 +1009,4 @@ m4_define([OFTABLE_CHK_OUT_PORT_SEC], [75]) m4_define([OFTABLE_ECMP_NH_MAC], [76]) m4_define([OFTABLE_ECMP_NH], [77]) m4_define([OFTABLE_CHK_LB_AFFINITY], [78]) +m4_define([OFTABLE_MAC_CACHE_USE], [79]) diff --git a/tests/ovn.at b/tests/ovn.at index 2f8aa4840..0bbf3d6da 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -34796,9 +34796,9 @@ wait_row_count mac_binding 1 ip="192.168.10.20" dp_key=$(printf "0x%x" $(as hv1 fetch_column datapath tunnel_key external_ids:name=gw)) port_key=$(printf "0x%x" $(as hv1 fetch_column port_binding tunnel_key logical_port=gw-public)) -AT_CHECK_UNQUOTED([as hv1 ovs-ofctl dump-flows br-int table=79 --no-stats | strip_cookie | sort], [0], [dnl - table=79, priority=100,ip,reg14=${port_key},metadata=${dp_key},dl_src=00:00:00:00:10:10,nw_src=192.168.10.10 actions=drop - table=79, priority=100,ip,reg14=${port_key},metadata=${dp_key},dl_src=00:00:00:00:10:20,nw_src=192.168.10.20 actions=drop +AT_CHECK_UNQUOTED([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_MAC_CACHE_USE --no-stats | strip_cookie | sort], [0], [dnl + table=OFTABLE_MAC_CACHE_USE, priority=100,ip,reg14=${port_key},metadata=${dp_key},dl_src=00:00:00:00:10:10,nw_src=192.168.10.10 actions=drop + table=OFTABLE_MAC_CACHE_USE, priority=100,ip,reg14=${port_key},metadata=${dp_key},dl_src=00:00:00:00:10:20,nw_src=192.168.10.20 actions=drop ]) timestamp=$(fetch_column mac_binding timestamp ip="192.168.10.20") @@ -34806,8 +34806,8 @@ timestamp=$(fetch_column mac_binding timestamp ip="192.168.10.20") send_udp hv1 ext1 10 send_udp hv2 ext2 20 -OVS_WAIT_UNTIL([as hv1 ovs-ofctl dump-flows br-int table=79 | grep "192.168.10.10" | grep -q "n_packets=1"]) -OVS_WAIT_UNTIL([as hv2 ovs-ofctl dump-flows br-int table=79 | grep "192.168.10.20" | grep -q "n_packets=1"]) +OVS_WAIT_UNTIL([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_MAC_CACHE_USE | grep "192.168.10.10" | grep -q "n_packets=1"]) +OVS_WAIT_UNTIL([as hv2 ovs-ofctl dump-flows br-int table=OFTABLE_MAC_CACHE_USE | grep "192.168.10.20" | grep -q "n_packets=1"]) # Set the MAC binding aging threshold AT_CHECK([ovn-nbctl set logical_router gw options:mac_binding_age_threshold=5]) @@ -34835,7 +34835,7 @@ OVS_WAIT_UNTIL([ test "0" = "$(ovn-sbctl list mac_binding | grep -c '192.168.10.20')" ]) -AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=79 --no-stats | strip_cookie], [0], []) +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_MAC_CACHE_USE --no-stats | strip_cookie], [0], []) # Test CIDR-based threshold configuration check ovn-nbctl set logical_router gw options:mac_binding_age_threshold="192.168.10.0/255.255.255.0:2;192.168.10.64/26:0;192.168.10.20:0"
Add macro for OFTABLE_MAC_CACHE_USE and replace all table=79 occurrences in OF with table=OFTABLE_MAC_CACHE_USE. Signed-off-by: Ales Musil <amusil@redhat.com> --- tests/ovn-macros.at | 1 + tests/ovn.at | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-)