Re: [ovs-dev] [PATCH v7 3/6] ovs-vsctl: Add limit to CT zone.
Bleep bloop. Greetings Ales Musil, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line lacks whitespace around operator WARNING: Line lacks whitespace around operator WARNING: Line lacks whitespace around operator WARNING: Line lacks whitespace around operator #220 FILE: utilities/ovs-vsctl.c:446: set-zone-limit BRIDGE zone_id=ZONE|default limit=LIMIT set CT LIMIT for\ WARNING: Line lacks whitespace around operator #221 FILE: utilities/ovs-vsctl.c:447: ZONE|default on BRIDGE\n\ WARNING: Line lacks whitespace around operator WARNING: Line lacks whitespace around operator WARNING: Line lacks whitespace around operator WARNING: Line lacks whitespace around operator #222 FILE: utilities/ovs-vsctl.c:448: del-zone-limit BRIDGE zone_id=ZONE|default delete CT limit\ WARNING: Line lacks whitespace around operator #223 FILE: utilities/ovs-vsctl.c:449: for ZONE|default on BRIDGE\n\ WARNING: Line lacks whitespace around operator #224 FILE: utilities/ovs-vsctl.c:450: list-zone-limit BRIDGE list all limits\ Lines checked: 471, Warnings: 11, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v7 6/6] tests: Do not use zone 0 for CT limit system test.
The zone 0 is default system zone, do not use this zone for the test because it might contain some entries already which could cause flakiness during the check. In order to still have the zone 0 parsing coverage add simple unit tests for dpctl. Signed-off-by: Ales Musil --- v7: Rebase on top of current master. Revert the unrelated EOL change. v6: Rebase on top of current master. --- tests/dpctl.at | 8 +- tests/system-traffic.at | 59 - 2 files changed, 36 insertions(+), 31 deletions(-) diff --git a/tests/dpctl.at b/tests/dpctl.at index d2f1046f8..a87f67f98 100644 --- a/tests/dpctl.at +++ b/tests/dpctl.at @@ -136,7 +136,7 @@ AT_CHECK([ovs-appctl dpctl/del-dp dummy@br0]) OVS_VSWITCHD_STOP AT_CLEANUP -AT_SETUP([dpctl - ct-get-limits ct-del-limits]) +AT_SETUP([dpctl - ct-set-limits ct-get-limits ct-del-limits]) OVS_VSWITCHD_START AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [default limit=0 ]) @@ -149,5 +149,11 @@ AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=x], [2], [], ovs-appctl: ovs-vswitchd: server returned an error ]) AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=]) +AT_CHECK([ovs-appctl dpctl/ct-set-limits zone=0,limit=0]) +AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=0], [0], [default limit=0 +zone=0,limit=0,count=0 +]) +AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=0]) + OVS_VSWITCHD_STOP AT_CLEANUP \ No newline at end of file diff --git a/tests/system-traffic.at b/tests/system-traffic.at index ac705a0a8..3fed3996a 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -5124,20 +5124,20 @@ ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") AT_DATA([flows.txt], [dnl priority=1,action=drop priority=10,arp,action=normal -priority=100,in_port=1,udp,action=ct(commit),2 +priority=100,in_port=1,udp,action=ct(zone=1,commit),2 priority=100,in_port=2,udp,action=ct(zone=3,commit),1 ]) AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) -AT_CHECK([ovs-appctl dpctl/ct-set-limits default=10 zone=0,limit=5 zone=1,limit=15 zone=2,limit=3 zone=3,limit=3]) -AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=1,2,4]) -AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=0,1,2,3], [],[dnl +AT_CHECK([ovs-appctl dpctl/ct-set-limits default=10 zone=1,limit=5 zone=2,limit=3 zone=3,limit=3 zone=4,limit=15]) +AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=2,4,5]) +AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=1,2,3,4], [],[dnl default limit=10 -zone=0,limit=5,count=0 -zone=1,limit=10,count=0 +zone=1,limit=5,count=0 zone=2,limit=10,count=0 zone=3,limit=3,count=0 +zone=4,limit=10,count=0 ]) dnl Test UDP from port 1 @@ -5151,10 +5151,9 @@ AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=5054000a5 AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=5054000a505400090800451c0011a4cd0a0101010a010102000100090008 actions=resubmit(,0)"]) AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=5054000a505400090800451c0011a4cd0a0101010a0101020001000a0008 actions=resubmit(,0)"]) -AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=0,1,2,3,4,5], [0], [dnl +AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=1,2,3,4,5], [0], [dnl default limit=10 -zone=0,limit=5,count=5 -zone=1,limit=10,count=0 +zone=1,limit=5,count=5 zone=2,limit=10,count=0 zone=3,limit=3,count=0 zone=4,limit=10,count=0 @@ -5164,16 +5163,16 @@ zone=5,limit=10,count=0 dnl Test ct-get-limits for all zones AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl default limit=10 -zone=0,limit=5,count=5 +zone=1,limit=5,count=5 zone=3,limit=3,count=0 ]) AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.1," | sort ], [0], [dnl -udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1) -udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=3),reply=(src=10.1.1.2,dst=10.1.1.1,sport=3,dport=1) -udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=4),reply=(src=10.1.1.2,dst=10.1.1.1,sport=4,dport=1) -udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=5),reply=(src=10.1.1.2,dst=10.1.1.1,sport=5,dport=1) -udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=6),reply=(src=10.1.1.2,dst=10.1.1.1,sport=6,dport=1) +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),zone=1 +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=3),reply=(src=10.1.1.2,dst=10.1.1.1,sport=3,dport=1),zone=1 +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=4),reply=(src=10.1.1.2,dst=10.1.1.1,sport=4,dport=1),zone=1 +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=5),reply=(src=10.1.1.2,dst=10.1.1.1,sport=5,dport=1),zone=1 +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=6),reply=(src=10.1.1.2,dst=10.1.1.1,sport=6,dport=1),zone=1 ]) dnl Test UDP from port 2 @@ -5183,9 +5182,9 @@ AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=5054000a5 AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_
Re: [ovs-dev] [PATCH v6 6/6] tests: Do not use zone 0 for CT limit system test.
On Tue, Nov 28, 2023 at 3:06 PM Ilya Maximets wrote: > On 11/2/23 13:00, Ales Musil wrote: > > The zone 0 is default system zone, do not use this > > zone for the test because it might contain some > > entries already which could cause flakiness during > > the check. > > > > In order to still have the zone 0 parsing coverage > > add simple unit tests for dpctl. > > > > Signed-off-by: Ales Musil > > --- > > v6: Rebase on top of current master. > > --- > > tests/dpctl.at | 10 +-- > > tests/system-traffic.at | 59 - > > 2 files changed, 37 insertions(+), 32 deletions(-) > > > > diff --git a/tests/dpctl.at b/tests/dpctl.at > > index d2f1046f8..bc84b196b 100644 > > --- a/tests/dpctl.at > > +++ b/tests/dpctl.at > > @@ -136,7 +136,7 @@ AT_CHECK([ovs-appctl dpctl/del-dp dummy@br0]) > > OVS_VSWITCHD_STOP > > AT_CLEANUP > > > > -AT_SETUP([dpctl - ct-get-limits ct-del-limits]) > > +AT_SETUP([dpctl - ct-set-limits ct-get-limits ct-del-limits]) > > OVS_VSWITCHD_START > > AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [default limit=0 > > ]) > > @@ -149,5 +149,11 @@ AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=x], > [2], [], > > ovs-appctl: ovs-vswitchd: server returned an error > > ]) > > AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=]) > > +AT_CHECK([ovs-appctl dpctl/ct-set-limits zone=0,limit=0]) > > +AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=0], [0], [default limit=0 > > +zone=0,limit=0,count=0 > > +]) > > +AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=0]) > > + > > OVS_VSWITCHD_STOP > > -AT_CLEANUP > > \ No newline at end of file > > +AT_CLEANUP > > Not sure what nappened here. Is the newline getting added/removed? > It's added because it wasn't there before, it should be fixed in v7. > > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > > index a1d26a06c..375a8aa2f 100644 > > --- a/tests/system-traffic.at > > +++ b/tests/system-traffic.at > > @@ -5124,20 +5124,20 @@ ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") > > AT_DATA([flows.txt], [dnl > > priority=1,action=drop > > priority=10,arp,action=normal > > -priority=100,in_port=1,udp,action=ct(commit),2 > > +priority=100,in_port=1,udp,action=ct(zone=1,commit),2 > > priority=100,in_port=2,udp,action=ct(zone=3,commit),1 > > ]) > > > > AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) > > > > -AT_CHECK([ovs-appctl dpctl/ct-set-limits default=10 zone=0,limit=5 > zone=1,limit=15 zone=2,limit=3 zone=3,limit=3]) > > -AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=1,2,4]) > > -AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=0,1,2,3], [],[dnl > > +AT_CHECK([ovs-appctl dpctl/ct-set-limits default=10 zone=1,limit=5 > zone=2,limit=3 zone=3,limit=3 zone=4,limit=15]) > > +AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=2,4,5]) > > +AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=1,2,3,4], [],[dnl > > default limit=10 > > -zone=0,limit=5,count=0 > > -zone=1,limit=10,count=0 > > +zone=1,limit=5,count=0 > > zone=2,limit=10,count=0 > > zone=3,limit=3,count=0 > > +zone=4,limit=10,count=0 > > ]) > > > > dnl Test UDP from port 1 > > @@ -5151,10 +5151,9 @@ AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 > "in_port=1 packet=5054000a5 > > AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 > packet=5054000a505400090800451c0011a4cd0a0101010a010102000100090008 > actions=resubmit(,0)"]) > > AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 > packet=5054000a505400090800451c0011a4cd0a0101010a0101020001000a0008 > actions=resubmit(,0)"]) > > > > -AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=0,1,2,3,4,5], [0], [dnl > > +AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=1,2,3,4,5], [0], [dnl > > default limit=10 > > -zone=0,limit=5,count=5 > > -zone=1,limit=10,count=0 > > +zone=1,limit=5,count=5 > > zone=2,limit=10,count=0 > > zone=3,limit=3,count=0 > > zone=4,limit=10,count=0 > > @@ -5164,16 +5163,16 @@ zone=5,limit=10,count=0 > > dnl Test ct-get-limits for all zones > > AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl > > default limit=10 > > -zone=0,limit=5,count=5 > > +zone=1,limit=5,count=5 > > zone=3,limit=3,count=0 > > ]) > > > > AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep > "orig=.src=10\.1\.1\.1," | sort ], [0], [dnl > > > -udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1) > > > -udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=3),reply=(src=10.1.1.2,dst=10.1.1.1,sport=3,dport=1) > > > -udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=4),reply=(src=10.1.1.2,dst=10.1.1.1,sport=4,dport=1) > > > -udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=5),reply=(src=10.1.1.2,dst=10.1.1.1,sport=5,dport=1) > > > -udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=6),reply=(src=10.1.1.2,dst=10.1.1.1,sport=6,dport=1) > > > +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),zone=1 > > > +udp,orig=(src=10.1.1.1,dst=1
Re: [ovs-dev] [PATCH v6 5/6] ct-dpif: Enforce CT zone limit protection.
On Tue, Nov 28, 2023 at 3:01 PM Ilya Maximets wrote:
> On 11/2/23 13:00, Ales Musil wrote:
> > Make sure that if any zone limit was set via DB
> > all zones are forced to be set there also. This
> > is done by tracking which datapath has zone limit
> > protection and it is reflected in the dpctl command.
> >
> > If the datapath is protected the dpctl command will
> > return permission error.
> >
> > Signed-off-by: Ales Musil
> > ---
> > v6: Rebase on top of current master.
> > Address comments from Ilya:
> > - Drop the log message about protection.
> > - Make the dpctl error message more user-friendly.
> > - Do not ignore error messages in the system-test.
> > v5: Rebase on top of current master.
> > Address comments from Ilya:
> > - Add more user friendly error message to the dpctl.
> > - Fix style related problems.
> > v4: Rebase on top of current master.
> > Make the protection datapath wide.
> > ---
> > lib/ct-dpif.c | 25 +
> > lib/ct-dpif.h | 2 ++
> > lib/dpctl.c| 14
> > ofproto/ofproto-dpif.c | 13 +++
> > ofproto/ofproto-provider.h | 5 +
> > ofproto/ofproto.c | 11 +
> > ofproto/ofproto.h | 2 ++
> > tests/ofproto-macros.at| 4 ++--
> > tests/system-traffic.at| 46 ++
> > vswitchd/bridge.c | 7 ++
> > 10 files changed, 127 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
> > index 2ee045164..5115c886b 100644
> > --- a/lib/ct-dpif.c
> > +++ b/lib/ct-dpif.c
> > @@ -23,6 +23,7 @@
> > #include "openvswitch/ofp-ct.h"
> > #include "openvswitch/ofp-parse.h"
> > #include "openvswitch/vlog.h"
> > +#include "sset.h"
> >
> > VLOG_DEFINE_THIS_MODULE(ct_dpif);
> >
> > @@ -32,6 +33,10 @@ struct flags {
> > const char *name;
> > };
> >
> > +/* Protection for CT zone limit per datapath. */
> > +static struct sset ct_limit_protection =
> > +SSET_INITIALIZER(&ct_limit_protection);
> > +
> > static void ct_dpif_format_counters(struct ds *,
> > const struct ct_dpif_counters *);
> > static void ct_dpif_format_timestamp(struct ds *,
> > @@ -1064,3 +1069,23 @@ ct_dpif_get_features(struct dpif *dpif, enum
> ct_features *features)
> > ? dpif->dpif_class->ct_get_features(dpif, features)
> > : EOPNOTSUPP);
> > }
> > +
> > +void
> > +ct_dpif_set_zone_limit_protection(struct dpif *dpif, bool protected)
> > +{
> > +if (sset_contains(&ct_limit_protection, dpif->full_name) ==
> protected) {
> > +return;
> > +}
> > +
> > +if (protected) {
> > +sset_add(&ct_limit_protection, dpif->full_name);
> > +} else {
> > +sset_find_and_delete(&ct_limit_protection, dpif->full_name);
> > +}
> > +}
> > +
> > +bool
> > +ct_dpif_is_zone_limit_protected(struct dpif *dpif)
> > +{
> > +return sset_contains(&ct_limit_protection, dpif->full_name);
> > +}
> > diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
> > index c8a7c155e..c3786d5ae 100644
> > --- a/lib/ct-dpif.h
> > +++ b/lib/ct-dpif.h
> > @@ -350,5 +350,7 @@ int ct_dpif_get_timeout_policy_name(struct dpif
> *dpif, uint32_t tp_id,
> > uint16_t dl_type, uint8_t nw_proto,
> > char **tp_name, bool *is_generic);
> > int ct_dpif_get_features(struct dpif *dpif, enum ct_features *features);
> > +void ct_dpif_set_zone_limit_protection(struct dpif *, bool protected);
> > +bool ct_dpif_is_zone_limit_protected(struct dpif *);
> >
> > #endif /* CT_DPIF_H */
> > diff --git a/lib/dpctl.c b/lib/dpctl.c
> > index a8c654747..2a1aac5e5 100644
> > --- a/lib/dpctl.c
> > +++ b/lib/dpctl.c
> > @@ -2234,6 +2234,13 @@ dpctl_ct_set_limits(int argc, const char *argv[],
> > ct_dpif_push_zone_limit(&zone_limits, zone, limit, 0);
> > }
> >
> > +if (ct_dpif_is_zone_limit_protected(dpif)) {
> > +ds_put_cstr(&ds, "the zone limits are set via database, "
> > + "use 'ovs-vsctl set-zone-limit <...>'
> instead.");
> > +error = EPERM;
> > +goto error;
> > +}
> > +
> > error = ct_dpif_set_limits(dpif, &zone_limits);
> > if (!error) {
> > ct_dpif_free_zone_limits(&zone_limits);
> > @@ -2310,6 +2317,13 @@ dpctl_ct_del_limits(int argc, const char *argv[],
> > }
> > }
> >
> > +if (ct_dpif_is_zone_limit_protected(dpif)) {
> > +ds_put_cstr(&ds, "the zone limits are set via database, "
> > + "use 'ovs-vsctl del-zone-limit <...>'
> instead.");
> > +error = EPERM;
> > +goto error;
> > +}
> > +
> > error = ct_dpif_del_limits(dpif, &zone_limits);
> > if (!error) {
> > goto out;
> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index 6a931a806..4cc8e2807 100644
> > --- a/ofproto/ofproto-dpif.c
> > +++ b/ofprot
Re: [ovs-dev] [PATCH v6 1/6] ct-dpif: Handle default zone limit the same way as other limits.
On Tue, Nov 28, 2023 at 2:34 PM Ilya Maximets wrote:
> On 11/2/23 13:00, Ales Musil wrote:
> > Internally handle default CT zone limit as other limits that
> > can be passed via the list with special value -1. Currently,
> > the -1 is treated by both datapaths as default, add static
> > asserts to make sure that this remains the case in the future.
> > This allows us to easily delete the default zone limit.
> >
> > Signed-off-by: Ales Musil
> > ---
> > v6: Rebase on top of current master.
> > Address comments from Ilya:
> > - Add assert to conntrack.h for the zone numbers.
> > - Some minot cosmetic changes.
> > v5: Rebase on top of current master.
> > Address comments from Ilya:
> > - Fix some typos.
> > - Use OVS_ZONE_LIMIT_DEFAULT_ZONE instead of special constant.
> > - Do not relay on DEFAULT_ZONE being -1 for the limit list.
> > - Fix wrong netlink message.
> > ---
> > lib/conntrack.c | 2 +-
> > lib/conntrack.h | 7 +--
> > lib/ct-dpif.c | 28 +++-
> > lib/ct-dpif.h | 14 ++
> > lib/dpctl.c | 15 ---
> > lib/dpif-netdev.c | 21 ++---
> > lib/dpif-netlink.c | 29 ++---
> > lib/dpif-provider.h | 24 +++-
> > 8 files changed, 58 insertions(+), 82 deletions(-)
> >
> > diff --git a/lib/conntrack.c b/lib/conntrack.c
> > index 47a443fba..31f00a127 100644
> > --- a/lib/conntrack.c
> > +++ b/lib/conntrack.c
> > @@ -398,7 +398,7 @@ zone_limit_clean(struct conntrack *ct, struct
> zone_limit *zl)
> > }
> >
> > int
> > -zone_limit_delete(struct conntrack *ct, uint16_t zone)
> > +zone_limit_delete(struct conntrack *ct, int32_t zone)
> > {
> > ovs_mutex_lock(&ct->ct_lock);
> > struct zone_limit *zl = zone_limit_lookup_protected(ct, zone);
> > diff --git a/lib/conntrack.h b/lib/conntrack.h
> > index 57d5159b6..18c182f85 100644
> > --- a/lib/conntrack.h
> > +++ b/lib/conntrack.h
> > @@ -122,11 +122,14 @@ struct timeout_policy {
> >
> > enum {
> > INVALID_ZONE = -2,
> > -DEFAULT_ZONE = -1, /* Default zone for zone limit management. */
> > +DEFAULT_ZONE = OVS_ZONE_LIMIT_DEFAULT_ZONE, /* Default zone for zone
> > + * limit management. */
> > MIN_ZONE = 0,
> > MAX_ZONE = 0x,
> > };
> >
> > +BUILD_ASSERT_DECL(DEFAULT_ZONE > INVALID_ZONE && DEFAULT_ZONE <
> MIN_ZONE);
> > +
> > struct ct_dpif_entry;
> > struct ct_dpif_tuple;
> >
> > @@ -154,6 +157,6 @@ struct ipf *conntrack_ipf_ctx(struct conntrack *ct);
> > struct conntrack_zone_limit zone_limit_get(struct conntrack *ct,
> > int32_t zone);
> > int zone_limit_update(struct conntrack *ct, int32_t zone, uint32_t
> limit);
> > -int zone_limit_delete(struct conntrack *ct, uint16_t zone);
> > +int zone_limit_delete(struct conntrack *ct, int32_t zone);
> >
> > #endif /* conntrack.h */
> > diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
> > index f59c6e560..2ee045164 100644
> > --- a/lib/ct-dpif.c
> > +++ b/lib/ct-dpif.c
> > @@ -398,23 +398,19 @@ ct_dpif_get_tcp_seq_chk(struct dpif *dpif, bool
> *enabled)
> > }
> >
> > int
> > -ct_dpif_set_limits(struct dpif *dpif, const uint32_t *default_limit,
> > - const struct ovs_list *zone_limits)
> > +ct_dpif_set_limits(struct dpif *dpif, const struct ovs_list
> *zone_limits)
> > {
> > return (dpif->dpif_class->ct_set_limits
> > -? dpif->dpif_class->ct_set_limits(dpif, default_limit,
> > - zone_limits)
> > +? dpif->dpif_class->ct_set_limits(dpif, zone_limits)
> > : EOPNOTSUPP);
> > }
> >
> > int
> > -ct_dpif_get_limits(struct dpif *dpif, uint32_t *default_limit,
> > - const struct ovs_list *zone_limits_in,
> > +ct_dpif_get_limits(struct dpif *dpif, const struct ovs_list
> *zone_limits_in,
> > struct ovs_list *zone_limits_out)
> > {
> > return (dpif->dpif_class->ct_get_limits
> > -? dpif->dpif_class->ct_get_limits(dpif, default_limit,
> > - zone_limits_in,
> > +? dpif->dpif_class->ct_get_limits(dpif, zone_limits_in,
> >zone_limits_out)
> > : EOPNOTSUPP);
> > }
> > @@ -854,7 +850,7 @@ ct_dpif_format_tcp_stat(struct ds * ds, int
> tcp_state, int conn_per_state)
> >
> >
> > void
> > -ct_dpif_push_zone_limit(struct ovs_list *zone_limits, uint16_t zone,
> > +ct_dpif_push_zone_limit(struct ovs_list *zone_limits, int32_t zone,
> > uint32_t limit, uint32_t count)
> > {
> > struct ct_dpif_zone_limit *zone_limit = xmalloc(sizeof *zone_limit);
> > @@ -928,15 +924,21 @@ error:
> > }
> >
> > void
> > -ct_dpif_format_zone_limits(uint32_t default_limit,
> > - const struct ovs_list *zone_limits, struct
> ds *d
Re: [ovs-dev] [PATCH v6 3/6] ovs-vsctl: Add limit to CT zone.
On Tue, Nov 28, 2023 at 2:54 PM Ilya Maximets wrote:
> On 11/2/23 13:00, Ales Musil wrote:
> > Add limit to the CT zone DB table with ovs-vsctl
> > helper methods. The limit has two special values
> > besides any number, 0 is unlimited and empty limit
> > is to leave the value untouched in the datapath.
> >
> > This is preparation step and the value is not yet
> > propagated to the datapath.
> >
> > Signed-off-by: Ales Musil
> > ---
> > v6: Rebase on top of current master.
> > Address comments from Ilya:
> > - Update the semantics and documentation of the set command.
> > v5: Rebase on top of current master.
> > Address comments from Ilya:
> > - Use only single command for setting zone and default limit.
> > - Correct the errors in the man page.
> > - Use references for the column description.
> > v4: Rebase on top of current master.
> > Address comments from Ilya:
> > - Make sure that the NEWS is clear on what has been added.
> > - Make the usage of --may-exist and --if-exists more intuitive for
> the new commands.
> > - Some cosmetics.
> > Add command and column for default limit.
> > ---
> > NEWS | 5 ++
> > tests/ovs-vsctl.at | 88 +++-
> > utilities/ovs-vsctl.8.in | 31 +++--
> > utilities/ovs-vsctl.c | 133 +++--
> > vswitchd/vswitch.ovsschema | 14 +++-
> > vswitchd/vswitch.xml | 13
> > 6 files changed, 268 insertions(+), 16 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index 7bc27b687..61b48ff12 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -9,6 +9,11 @@ Post-v3.2.0
> > - ovs-appctl:
> > * Added support removal of default CT zone limit, e.g.
> > "ovs-appctl dpctl/ct-del-limits default".
> > + - ovs-vsctl:
> > + * New commands 'set-zone-limit', 'del-zone-limit' and
> 'list-zone-limit'
> > + to manage the maximum number of connections in conntrack zones
> via
> > + a new 'limit' column in the 'CT_Zone' database table and
> > + 'ct_zone_default_limit' column in the 'Datapath' table.
> >
> >
> > v3.2.0 - 17 Aug 2023
> > diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
> > index a368bff6e..f88e986db 100644
> > --- a/tests/ovs-vsctl.at
> > +++ b/tests/ovs-vsctl.at
> > @@ -975,6 +975,67 @@ AT_CHECK(
> >[0], [stdout])
> > AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:10, Timeout
> Policies: system default
> > ])
> > +AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-tp netdev zone=10])])
> > +
> > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])])
> > +
> > +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev zone=1 limit=1])])
> > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
> > +Zone: 1, Limit: 1
> > +])
> > +
> > +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev zone=1 limit=5])])
> > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
> > +Zone: 1, Limit: 5
> > +])
> > +
> > +AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev zone=1])])
> > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])])
> > +
> > +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev zone=10 limit=5])])
> > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
> > +Zone: 10, Limit: 5
> > +])
> > +
> > +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=10 icmp_first=1
> icmp_reply=2])])
> > +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [dnl
> > +Zone:10, Timeout Policies: icmp_first=1 icmp_reply=2
> > +])
> > +
> > +AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev zone=10])])
> > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])])
> > +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [dnl
> > +Zone:10, Timeout Policies: icmp_first=1 icmp_reply=2
> > +])
> > +
> > +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev zone=10 limit=5])])
> > +AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=10])])
> > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
> > +Zone: 10, Limit: 5
> > +])
> > +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [dnl
> > +Zone:10, Timeout Policies: system default
> > +])
> > +
> > +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev default limit=5])])
> > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
> > +Default limit: 5
> > +Zone: 10, Limit: 5
> > +])
> > +
> > +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev default limit=10])])
> > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
> > +Default limit: 10
> > +Zone: 10, Limit: 5
> > +])
> > +
> > +AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev default])])
> > +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
> > +Zone: 10, Limit: 5
> > +])
> > +
> > +AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-limit netdev default])])
> > +
> >
> > AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0
> 'capabilities={recirc=true}' -- set Open_vSwitch .
> datapaths:"system"=@m])], [0], [stdout])
> > AT_CHECK([RUN_OVS_VSCTL([list-dp-cap system])], [0],
Re: [ovs-dev] [PATCH v6 2/6] dpctl: Allow the default CT zone limit to de deleted.
On Tue, Nov 28, 2023 at 2:47 PM Ilya Maximets wrote:
> On 11/2/23 13:00, Ales Musil wrote:
> > Add optional argument to dpctl ct-del-limits called
> > "default", which allows to remove the default limit
> > making it effectively system default.
> >
> > Signed-off-by: Ales Musil
> > ---
> > v6: Rebase on top of current master.
> > Address comments from Ilya:
> > - Adjust the log message so it doesn't report anything for default
> zone.
> > v5: Rebase on top of current master.
> > Address comments from Ilya:
> > - Correct the NEWS entry.
> > - Fix style related problems.
> > ---
> > NEWS| 3 +++
> > lib/conntrack.c | 13 +++--
> > lib/dpctl.c | 21 +++--
> > tests/system-traffic.at | 26 ++
> > 4 files changed, 51 insertions(+), 12 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index 6b45492f1..7bc27b687 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -6,6 +6,9 @@ Post-v3.2.0
> > from older version is supported but it may trigger more leader
> elections
> > during the process, and error logs complaining unrecognized
> fields may
> > be observed on old nodes.
> > + - ovs-appctl:
> > + * Added support removal of default CT zone limit, e.g.
>
> Added support for ...
>
> > + "ovs-appctl dpctl/ct-del-limits default".
> >
> >
> > v3.2.0 - 17 Aug 2023
> > diff --git a/lib/conntrack.c b/lib/conntrack.c
> > index 31f00a127..b533dd3df 100644
> > --- a/lib/conntrack.c
> > +++ b/lib/conntrack.c
> > @@ -404,13 +404,14 @@ zone_limit_delete(struct conntrack *ct, int32_t
> zone)
> > struct zone_limit *zl = zone_limit_lookup_protected(ct, zone);
> > if (zl) {
> > zone_limit_clean(ct, zl);
> > -ovs_mutex_unlock(&ct->ct_lock);
> > -VLOG_INFO("Deleted zone limit for zone %d", zone);
> > -} else {
> > -ovs_mutex_unlock(&ct->ct_lock);
> > -VLOG_INFO("Attempted delete of non-existent zone limit: zone
> %d",
> > - zone);
> > }
> > +
> > +if (zone != DEFAULT_ZONE) {
> > +VLOG_INFO(zl ? "Deleted zone limit for zone %d" : "Attempted
> delete"
> > + " of non-existent zone limit: zone %d", zone);
>
> Nit: Might be better to not split the string. E.g.:
>
> VLOG_INFO(zl ? "Deleted zone limit for zone %d"
> : "Attempted delete of non-existent zone limit: zone
> %d",
> zone);
>
> > +}
> > +
> > +ovs_mutex_unlock(&ct->ct_lock);
> > return 0;
> > }
> >
> > diff --git a/lib/dpctl.c b/lib/dpctl.c
> > index 76f21a530..a8c654747 100644
> > --- a/lib/dpctl.c
> > +++ b/lib/dpctl.c
> > @@ -2291,14 +2291,23 @@ dpctl_ct_del_limits(int argc, const char *argv[],
> > int i = dp_arg_exists(argc, argv) ? 2 : 1;
> > struct ovs_list zone_limits = OVS_LIST_INITIALIZER(&zone_limits);
> >
> > -error = opt_dpif_open(argc, argv, dpctl_p, 3, &dpif);
> > +error = opt_dpif_open(argc, argv, dpctl_p, 4, &dpif);
> > if (error) {
> > return error;
> > }
> >
> > -error = parse_ct_limit_zones(argv[i], &zone_limits, &ds);
> > -if (error) {
> > -goto error;
> > +/* Parse default limit. */
> > +if (!strcmp(argv[i], "default")) {
> > +ct_dpif_push_zone_limit(&zone_limits,
> OVS_ZONE_LIMIT_DEFAULT_ZONE,
> > +0, 0);
> > +i++;
> > +}
> > +
> > +if (argc > i) {
> > +error = parse_ct_limit_zones(argv[i], &zone_limits, &ds);
> > +if (error) {
> > +goto error;
> > +}
> > }
> >
> > error = ct_dpif_del_limits(dpif, &zone_limits);
> > @@ -3031,8 +3040,8 @@ static const struct dpctl_command all_commands[] =
> {
> > { "ct-get-tcp-seq-chk", "[dp]", 0, 1, dpctl_ct_get_tcp_seq_chk,
> DP_RO },
> > { "ct-set-limits", "[dp] [default=L] [zone=N,limit=L]...", 1,
> INT_MAX,
> > dpctl_ct_set_limits, DP_RO },
> > -{ "ct-del-limits", "[dp] zone=N1[,N2]...", 1, 2,
> dpctl_ct_del_limits,
> > -DP_RO },
> > +{ "ct-del-limits", "[dp] [default] [zone=N1[,N2]...]", 1, 3,
> > +dpctl_ct_del_limits, DP_RO },
> > { "ct-get-limits", "[dp] [zone=N1[,N2]...]", 0, 2,
> dpctl_ct_get_limits,
> > DP_RO },
> > { "ct-get-sweep-interval", "[dp]", 0, 1, dpctl_ct_get_sweep, DP_RO
> },
> > diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> > index 7ea450202..b6c8d7faf 100644
> > --- a/tests/system-traffic.at
> > +++ b/tests/system-traffic.at
> > @@ -5195,6 +5195,32 @@
> udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=3),reply=(src=10.1.1.4,dst=10.
> >
> udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=4),reply=(src=10.1.1.4,dst=10.1.1.3,sport=4,dport=1),zone=3
> > ])
> >
> > +dnl Test ct-del-limits for default zone.
> > +
> > +AT_CHECK([ovs-appctl dpctl/ct-set-limits default=15 zone=4,limit=4])
> > +AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=4], [0], [dnl
> > +default
[ovs-dev] [PATCH v7 3/6] ovs-vsctl: Add limit to CT zone.
Add limit to the CT zone DB table with ovs-vsctl
helper methods. The limit has two special values
besides any number, 0 is unlimited and empty limit
is to leave the value untouched in the datapath.
This is preparation step and the value is not yet
propagated to the datapath.
Signed-off-by: Ales Musil
---
v7: Rebase on top of current master.
Address comments from Ilya:
- Add missing 'a'.
- Slightly change the format for limit listing.
- Add usage string for all the new comamnds.
v6: Rebase on top of current master.
Address comments from Ilya:
- Update the semantics and documentation of the set command.
v5: Rebase on top of current master.
Address comments from Ilya:
- Use only single command for setting zone and default limit.
- Correct the errors in the man page.
- Use references for the column description.
v4: Rebase on top of current master.
Address comments from Ilya:
- Make sure that the NEWS is clear on what has been added.
- Make the usage of --may-exist and --if-exists more intuitive for the new
commands.
- Some cosmetics.
Add command and column for default limit.
---
NEWS | 5 ++
tests/ovs-vsctl.at | 88 ++-
utilities/ovs-vsctl.8.in | 31 ++--
utilities/ovs-vsctl.c | 141 +++--
vswitchd/vswitch.ovsschema | 14 +++-
vswitchd/vswitch.xml | 14
6 files changed, 277 insertions(+), 16 deletions(-)
diff --git a/NEWS b/NEWS
index 474b3a4ef..77117573c 100644
--- a/NEWS
+++ b/NEWS
@@ -17,6 +17,11 @@ Post-v3.2.0
Reported names adjusted accordingly.
* Added support for removal of default CT zone limit, e.g.
"ovs-appctl dpctl/ct-del-limits default".
+ - ovs-vsctl:
+ * New commands 'set-zone-limit', 'del-zone-limit' and 'list-zone-limit'
+ to manage the maximum number of connections in conntrack zones via
+ a new 'limit' column in the 'CT_Zone' database table and
+ 'ct_zone_default_limit' column in the 'Datapath' table.
v3.2.0 - 17 Aug 2023
diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index a368bff6e..c913c5a2d 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -975,6 +975,67 @@ AT_CHECK(
[0], [stdout])
AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:10, Timeout
Policies: system default
])
+AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-tp netdev zone=10])])
+
+AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])])
+
+AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev zone=1 limit=1])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
+Zone: 1, Limit: 1
+])
+
+AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev zone=1 limit=5])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
+Zone: 1, Limit: 5
+])
+
+AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev zone=1])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])])
+
+AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev zone=10 limit=5])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
+Zone: 10, Limit: 5
+])
+
+AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=10 icmp_first=1
icmp_reply=2])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [dnl
+Zone:10, Timeout Policies: icmp_first=1 icmp_reply=2
+])
+
+AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev zone=10])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [dnl
+Zone:10, Timeout Policies: icmp_first=1 icmp_reply=2
+])
+
+AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev zone=10 limit=5])])
+AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=10])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
+Zone: 10, Limit: 5
+])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [dnl
+Zone:10, Timeout Policies: system default
+])
+
+AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev default limit=5])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
+Default, Limit: 5
+Zone: 10, Limit: 5
+])
+
+AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev default limit=10])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
+Default, Limit: 10
+Zone: 10, Limit: 5
+])
+
+AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev default])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
+Zone: 10, Limit: 5
+])
+
+AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-limit netdev default])])
+
AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0
'capabilities={recirc=true}' -- set Open_vSwitch . datapaths:"system"=@m])],
[0], [stdout])
AT_CHECK([RUN_OVS_VSCTL([list-dp-cap system])], [0], [recirc=true
@@ -1113,16 +1174,39 @@ AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdevxx zone=1
icmp_first=1 icmp_reply=2])
])
AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2
icmp_reply=3])])
AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2
icmp_reply=3])],
- [1], [], [ovs-vsctl: zone id 2 already exists
+ [1], [], [
[ovs-dev] [PATCH v7 4/6] vswitchd, ofproto-dpif: Propagate the CT limit from database.
Propagate the CT limit that is present in the DB into
datapath. The limit is currently only propagated on change
and can be overwritten by the dpctl commands.
Signed-off-by: Ales Musil
---
v7: Rebase on top of current master.
v6: Rebase on top of current master.
Address comments from Ilya:
- Update the comments and names.
- Use loop in the system-test.
v5: Rebase on top of current master.
Address comments from Ilya:
- Make sure the zones are always removed.
- Fix style related problems.
- Make sure the limit is initialized to -1.
v4: Rebase on top of current master.
Make sure that the values from DB are propagated only if set. That applies
to both limit and policies.
---
ofproto/ofproto-dpif.c | 39
ofproto/ofproto-dpif.h | 5 +++
ofproto/ofproto-provider.h | 8
ofproto/ofproto.c | 12 ++
ofproto/ofproto.h | 2 +
tests/system-traffic.at| 54 +++
vswitchd/bridge.c | 75 +-
7 files changed, 177 insertions(+), 18 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 54e057d43..bfae28d96 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -220,6 +220,7 @@ static void ofproto_unixctl_init(void);
static void ct_zone_config_init(struct dpif_backer *backer);
static void ct_zone_config_uninit(struct dpif_backer *backer);
static void ct_zone_timeout_policy_sweep(struct dpif_backer *backer);
+static void ct_zone_limits_commit(struct dpif_backer *backer);
static inline struct ofproto_dpif *
ofproto_dpif_cast(const struct ofproto *ofproto)
@@ -513,6 +514,7 @@ type_run(const char *type)
process_dpif_port_changes(backer);
ct_zone_timeout_policy_sweep(backer);
+ct_zone_limits_commit(backer);
return 0;
}
@@ -5532,6 +5534,8 @@ ct_zone_config_init(struct dpif_backer *backer)
cmap_init(&backer->ct_zones);
hmap_init(&backer->ct_tps);
ovs_list_init(&backer->ct_tp_kill_list);
+ovs_list_init(&backer->ct_zone_limits_to_add);
+ovs_list_init(&backer->ct_zone_limits_to_del);
clear_existing_ct_timeout_policies(backer);
}
@@ -,6 +5559,8 @@ ct_zone_config_uninit(struct dpif_backer *backer)
id_pool_destroy(backer->tp_ids);
cmap_destroy(&backer->ct_zones);
hmap_destroy(&backer->ct_tps);
+ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_add);
+ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_del);
}
static void
@@ -5635,6 +5641,38 @@ ct_del_zone_timeout_policy(const char *datapath_type,
uint16_t zone_id)
}
}
+static void
+ct_zone_limit_update(const char *datapath_type, int32_t zone_id,
+ int64_t *limit)
+{
+struct dpif_backer *backer = shash_find_data(&all_dpif_backers,
+ datapath_type);
+if (!backer) {
+return;
+}
+
+if (limit) {
+ct_dpif_push_zone_limit(&backer->ct_zone_limits_to_add, zone_id,
+*limit, 0);
+} else {
+ct_dpif_push_zone_limit(&backer->ct_zone_limits_to_del, zone_id, 0, 0);
+}
+}
+
+static void
+ct_zone_limits_commit(struct dpif_backer *backer)
+{
+if (!ovs_list_is_empty(&backer->ct_zone_limits_to_add)) {
+ct_dpif_set_limits(backer->dpif, &backer->ct_zone_limits_to_add);
+ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_add);
+}
+
+if (!ovs_list_is_empty(&backer->ct_zone_limits_to_del)) {
+ct_dpif_del_limits(backer->dpif, &backer->ct_zone_limits_to_del);
+ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_del);
+}
+}
+
static void
get_datapath_cap(const char *datapath_type, struct smap *cap)
{
@@ -6925,4 +6963,5 @@ const struct ofproto_class ofproto_dpif_class = {
ct_flush, /* ct_flush */
ct_set_zone_timeout_policy,
ct_del_zone_timeout_policy,
+ct_zone_limit_update,
};
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 1fe22ab41..4709200bc 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -285,6 +285,11 @@ struct dpif_backer {
feature than 'bt_support'. */
struct atomic_count tnl_count;
+
+struct ovs_list ct_zone_limits_to_add; /* CT zone limits queued for
+ * addition into datapath. */
+struct ovs_list ct_zone_limits_to_del; /* CT zone limt queued for
+ * deletion from datapath. */
};
/* All existing ofproto_backer instances, indexed by ofproto->up.type. */
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 9f7b8b6e8..face0b574 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1921,6 +1921,14 @@ struct ofproto_class {
/* Deletes the timeout policy associated with 'zone' in datapath type
* 'dp_type'. */
void (*ct_del_zon
[ovs-dev] [PATCH v7 1/6] ct-dpif: Handle default zone limit the same way as other limits.
Internally handle default CT zone limit as other limits that
can be passed via the list with special value -1. Currently,
the -1 is treated by both datapaths as default, add static
asserts to make sure that this remains the case in the future.
This allows us to easily delete the default zone limit.
Signed-off-by: Ales Musil
---
v7: Rebase on top of current master.
Do not populate count for default zone and add note about it to the
dpif_class.
v6: Rebase on top of current master.
Address comments from Ilya:
- Add assert to conntrack.h for the zone numbers.
- Some minot cosmetic changes.
v5: Rebase on top of current master.
Address comments from Ilya:
- Fix some typos.
- Use OVS_ZONE_LIMIT_DEFAULT_ZONE instead of special constant.
- Do not relay on DEFAULT_ZONE being -1 for the limit list.
- Fix wrong netlink message.
---
lib/conntrack.c | 2 +-
lib/conntrack.h | 7 +--
lib/ct-dpif.c | 28 +++-
lib/ct-dpif.h | 14 ++
lib/dpctl.c | 15 ---
lib/dpif-netdev.c | 21 ++---
lib/dpif-netlink.c | 29 ++---
lib/dpif-provider.h | 24 +++-
8 files changed, 58 insertions(+), 82 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 47a443fba..31f00a127 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -398,7 +398,7 @@ zone_limit_clean(struct conntrack *ct, struct zone_limit
*zl)
}
int
-zone_limit_delete(struct conntrack *ct, uint16_t zone)
+zone_limit_delete(struct conntrack *ct, int32_t zone)
{
ovs_mutex_lock(&ct->ct_lock);
struct zone_limit *zl = zone_limit_lookup_protected(ct, zone);
diff --git a/lib/conntrack.h b/lib/conntrack.h
index 57d5159b6..18c182f85 100644
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -122,11 +122,14 @@ struct timeout_policy {
enum {
INVALID_ZONE = -2,
-DEFAULT_ZONE = -1, /* Default zone for zone limit management. */
+DEFAULT_ZONE = OVS_ZONE_LIMIT_DEFAULT_ZONE, /* Default zone for zone
+ * limit management. */
MIN_ZONE = 0,
MAX_ZONE = 0x,
};
+BUILD_ASSERT_DECL(DEFAULT_ZONE > INVALID_ZONE && DEFAULT_ZONE < MIN_ZONE);
+
struct ct_dpif_entry;
struct ct_dpif_tuple;
@@ -154,6 +157,6 @@ struct ipf *conntrack_ipf_ctx(struct conntrack *ct);
struct conntrack_zone_limit zone_limit_get(struct conntrack *ct,
int32_t zone);
int zone_limit_update(struct conntrack *ct, int32_t zone, uint32_t limit);
-int zone_limit_delete(struct conntrack *ct, uint16_t zone);
+int zone_limit_delete(struct conntrack *ct, int32_t zone);
#endif /* conntrack.h */
diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
index f59c6e560..2ee045164 100644
--- a/lib/ct-dpif.c
+++ b/lib/ct-dpif.c
@@ -398,23 +398,19 @@ ct_dpif_get_tcp_seq_chk(struct dpif *dpif, bool *enabled)
}
int
-ct_dpif_set_limits(struct dpif *dpif, const uint32_t *default_limit,
- const struct ovs_list *zone_limits)
+ct_dpif_set_limits(struct dpif *dpif, const struct ovs_list *zone_limits)
{
return (dpif->dpif_class->ct_set_limits
-? dpif->dpif_class->ct_set_limits(dpif, default_limit,
- zone_limits)
+? dpif->dpif_class->ct_set_limits(dpif, zone_limits)
: EOPNOTSUPP);
}
int
-ct_dpif_get_limits(struct dpif *dpif, uint32_t *default_limit,
- const struct ovs_list *zone_limits_in,
+ct_dpif_get_limits(struct dpif *dpif, const struct ovs_list *zone_limits_in,
struct ovs_list *zone_limits_out)
{
return (dpif->dpif_class->ct_get_limits
-? dpif->dpif_class->ct_get_limits(dpif, default_limit,
- zone_limits_in,
+? dpif->dpif_class->ct_get_limits(dpif, zone_limits_in,
zone_limits_out)
: EOPNOTSUPP);
}
@@ -854,7 +850,7 @@ ct_dpif_format_tcp_stat(struct ds * ds, int tcp_state, int
conn_per_state)
void
-ct_dpif_push_zone_limit(struct ovs_list *zone_limits, uint16_t zone,
+ct_dpif_push_zone_limit(struct ovs_list *zone_limits, int32_t zone,
uint32_t limit, uint32_t count)
{
struct ct_dpif_zone_limit *zone_limit = xmalloc(sizeof *zone_limit);
@@ -928,15 +924,21 @@ error:
}
void
-ct_dpif_format_zone_limits(uint32_t default_limit,
- const struct ovs_list *zone_limits, struct ds *ds)
+ct_dpif_format_zone_limits(const struct ovs_list *zone_limits, struct ds *ds)
{
struct ct_dpif_zone_limit *zone_limit;
-ds_put_format(ds, "default limit=%"PRIu32, default_limit);
+LIST_FOR_EACH (zone_limit, node, zone_limits) {
+if (zone_limit->zone == OVS_ZONE_LIMIT_DEFAULT_ZONE) {
+ds_put_format(ds, "default limit=%"PRIu32, zone_limit->limit);
+}
+}
LIST_FOR_EACH (zo
[ovs-dev] [PATCH v7 5/6] ct-dpif: Enforce CT zone limit protection.
Make sure that if any zone limit was set via DB
all zones are forced to be set there also. This
is done by tracking which datapath has zone limit
protection and it is reflected in the dpctl command.
If the datapath is protected the dpctl command will
return permission error.
Signed-off-by: Ales Musil
---
v7: Rebase on top of current master.
Remove leftover comments from testing.
v6: Rebase on top of current master.
Address comments from Ilya:
- Drop the log message about protection.
- Make the dpctl error message more user-friendly.
- Do not ignore error messages in the system-test.
v5: Rebase on top of current master.
Address comments from Ilya:
- Add more user friendly error message to the dpctl.
- Fix style related problems.
v4: Rebase on top of current master.
Make the protection datapath wide.
---
lib/ct-dpif.c | 25 +
lib/ct-dpif.h | 2 ++
lib/dpctl.c| 14
ofproto/ofproto-dpif.c | 13 +++
ofproto/ofproto-provider.h | 5 +
ofproto/ofproto.c | 11 +
ofproto/ofproto.h | 2 ++
tests/system-traffic.at| 46 ++
vswitchd/bridge.c | 7 ++
9 files changed, 125 insertions(+)
diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
index 2ee045164..5115c886b 100644
--- a/lib/ct-dpif.c
+++ b/lib/ct-dpif.c
@@ -23,6 +23,7 @@
#include "openvswitch/ofp-ct.h"
#include "openvswitch/ofp-parse.h"
#include "openvswitch/vlog.h"
+#include "sset.h"
VLOG_DEFINE_THIS_MODULE(ct_dpif);
@@ -32,6 +33,10 @@ struct flags {
const char *name;
};
+/* Protection for CT zone limit per datapath. */
+static struct sset ct_limit_protection =
+SSET_INITIALIZER(&ct_limit_protection);
+
static void ct_dpif_format_counters(struct ds *,
const struct ct_dpif_counters *);
static void ct_dpif_format_timestamp(struct ds *,
@@ -1064,3 +1069,23 @@ ct_dpif_get_features(struct dpif *dpif, enum ct_features
*features)
? dpif->dpif_class->ct_get_features(dpif, features)
: EOPNOTSUPP);
}
+
+void
+ct_dpif_set_zone_limit_protection(struct dpif *dpif, bool protected)
+{
+if (sset_contains(&ct_limit_protection, dpif->full_name) == protected) {
+return;
+}
+
+if (protected) {
+sset_add(&ct_limit_protection, dpif->full_name);
+} else {
+sset_find_and_delete(&ct_limit_protection, dpif->full_name);
+}
+}
+
+bool
+ct_dpif_is_zone_limit_protected(struct dpif *dpif)
+{
+return sset_contains(&ct_limit_protection, dpif->full_name);
+}
diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
index c8a7c155e..c3786d5ae 100644
--- a/lib/ct-dpif.h
+++ b/lib/ct-dpif.h
@@ -350,5 +350,7 @@ int ct_dpif_get_timeout_policy_name(struct dpif *dpif,
uint32_t tp_id,
uint16_t dl_type, uint8_t nw_proto,
char **tp_name, bool *is_generic);
int ct_dpif_get_features(struct dpif *dpif, enum ct_features *features);
+void ct_dpif_set_zone_limit_protection(struct dpif *, bool protected);
+bool ct_dpif_is_zone_limit_protected(struct dpif *);
#endif /* CT_DPIF_H */
diff --git a/lib/dpctl.c b/lib/dpctl.c
index a8c654747..2a1aac5e5 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -2234,6 +2234,13 @@ dpctl_ct_set_limits(int argc, const char *argv[],
ct_dpif_push_zone_limit(&zone_limits, zone, limit, 0);
}
+if (ct_dpif_is_zone_limit_protected(dpif)) {
+ds_put_cstr(&ds, "the zone limits are set via database, "
+ "use 'ovs-vsctl set-zone-limit <...>' instead.");
+error = EPERM;
+goto error;
+}
+
error = ct_dpif_set_limits(dpif, &zone_limits);
if (!error) {
ct_dpif_free_zone_limits(&zone_limits);
@@ -2310,6 +2317,13 @@ dpctl_ct_del_limits(int argc, const char *argv[],
}
}
+if (ct_dpif_is_zone_limit_protected(dpif)) {
+ds_put_cstr(&ds, "the zone limits are set via database, "
+ "use 'ovs-vsctl del-zone-limit <...>' instead.");
+error = EPERM;
+goto error;
+}
+
error = ct_dpif_del_limits(dpif, &zone_limits);
if (!error) {
goto out;
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index bfae28d96..6e62ed1f9 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5673,6 +5673,18 @@ ct_zone_limits_commit(struct dpif_backer *backer)
}
}
+static void
+ct_zone_limit_protection_update(const char *datapath_type, bool protected)
+{
+struct dpif_backer *backer = shash_find_data(&all_dpif_backers,
+ datapath_type);
+if (!backer) {
+return;
+}
+
+ct_dpif_set_zone_limit_protection(backer->dpif, protected);
+}
+
static void
get_datapath_cap(const char *datapath_type, struct smap *cap)
{
@@ -6964,4 +6976,5 @@ const struct ofproto_class ofpr
[ovs-dev] [PATCH v7 2/6] dpctl: Allow the default CT zone limit to de deleted.
Add optional argument to dpctl ct-del-limits called
"default", which allows to remove the default limit
making it effectively system default.
Signed-off-by: Ales Musil
---
v7: Rebase on top of current master.
Address cosmetic comments.
v6: Rebase on top of current master.
Address comments from Ilya:
- Adjust the log message so it doesn't report anything for default zone.
v5: Rebase on top of current master.
Address comments from Ilya:
- Correct the NEWS entry.
- Fix style related problems.
---
NEWS| 2 ++
lib/conntrack.c | 12 +++-
lib/dpctl.c | 21 +++--
tests/system-traffic.at | 26 ++
4 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/NEWS b/NEWS
index 1d9c30533..474b3a4ef 100644
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,8 @@ Post-v3.2.0
a.k.a. 'configured' values, can be found in the 'status' column of
the Interface table, i.e. with 'ovs-vsctl get interface <..> status'.
Reported names adjusted accordingly.
+ * Added support for removal of default CT zone limit, e.g.
+ "ovs-appctl dpctl/ct-del-limits default".
v3.2.0 - 17 Aug 2023
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 31f00a127..71c470661 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -404,13 +404,15 @@ zone_limit_delete(struct conntrack *ct, int32_t zone)
struct zone_limit *zl = zone_limit_lookup_protected(ct, zone);
if (zl) {
zone_limit_clean(ct, zl);
-ovs_mutex_unlock(&ct->ct_lock);
-VLOG_INFO("Deleted zone limit for zone %d", zone);
-} else {
-ovs_mutex_unlock(&ct->ct_lock);
-VLOG_INFO("Attempted delete of non-existent zone limit: zone %d",
+}
+
+if (zone != DEFAULT_ZONE) {
+VLOG_INFO(zl ? "Deleted zone limit for zone %d"
+ : "Attempted delete of non-existent zone limit: zone %d",
zone);
}
+
+ovs_mutex_unlock(&ct->ct_lock);
return 0;
}
diff --git a/lib/dpctl.c b/lib/dpctl.c
index 76f21a530..a8c654747 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -2291,14 +2291,23 @@ dpctl_ct_del_limits(int argc, const char *argv[],
int i = dp_arg_exists(argc, argv) ? 2 : 1;
struct ovs_list zone_limits = OVS_LIST_INITIALIZER(&zone_limits);
-error = opt_dpif_open(argc, argv, dpctl_p, 3, &dpif);
+error = opt_dpif_open(argc, argv, dpctl_p, 4, &dpif);
if (error) {
return error;
}
-error = parse_ct_limit_zones(argv[i], &zone_limits, &ds);
-if (error) {
-goto error;
+/* Parse default limit. */
+if (!strcmp(argv[i], "default")) {
+ct_dpif_push_zone_limit(&zone_limits, OVS_ZONE_LIMIT_DEFAULT_ZONE,
+0, 0);
+i++;
+}
+
+if (argc > i) {
+error = parse_ct_limit_zones(argv[i], &zone_limits, &ds);
+if (error) {
+goto error;
+}
}
error = ct_dpif_del_limits(dpif, &zone_limits);
@@ -3031,8 +3040,8 @@ static const struct dpctl_command all_commands[] = {
{ "ct-get-tcp-seq-chk", "[dp]", 0, 1, dpctl_ct_get_tcp_seq_chk, DP_RO },
{ "ct-set-limits", "[dp] [default=L] [zone=N,limit=L]...", 1, INT_MAX,
dpctl_ct_set_limits, DP_RO },
-{ "ct-del-limits", "[dp] zone=N1[,N2]...", 1, 2, dpctl_ct_del_limits,
-DP_RO },
+{ "ct-del-limits", "[dp] [default] [zone=N1[,N2]...]", 1, 3,
+dpctl_ct_del_limits, DP_RO },
{ "ct-get-limits", "[dp] [zone=N1[,N2]...]", 0, 2, dpctl_ct_get_limits,
DP_RO },
{ "ct-get-sweep-interval", "[dp]", 0, 1, dpctl_ct_get_sweep, DP_RO },
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index a7d4ed83b..7c26adda7 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -5195,6 +5195,32 @@
udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=3),reply=(src=10.1.1.4,dst=10.
udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=4),reply=(src=10.1.1.4,dst=10.1.1.3,sport=4,dport=1),zone=3
])
+dnl Test ct-del-limits for default zone.
+
+AT_CHECK([ovs-appctl dpctl/ct-set-limits default=15 zone=4,limit=4])
+AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=4], [0], [dnl
+default limit=15
+zone=4,limit=4,count=0
+])
+
+AT_CHECK([ovs-appctl dpctl/ct-del-limits default])
+AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=4], [0], [dnl
+default limit=0
+zone=4,limit=4,count=0
+])
+
+AT_CHECK([ovs-appctl dpctl/ct-set-limits default=15])
+AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=4], [0], [dnl
+default limit=15
+zone=4,limit=4,count=0
+])
+
+AT_CHECK([ovs-appctl dpctl/ct-del-limits default zone=4])
+AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=4], [0], [dnl
+default limit=0
+zone=4,limit=0,count=0
+])
+
OVS_TRAFFIC_VSWITCHD_STOP(["dnl
/could not create datapath/d
/(Cannot allocate memory) on packet/d"])
--
2.43.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/l
[ovs-dev] [PATCH v7 0/6] Expose CT limit via DB
The series exposes CT limit via DB, adding user friendly ovs-vsctl interface. The DB value has priority before the dpctl interface, this is achieved by storing which datapath is protected. The dpctl will return an error if the limit is already set in DB for that datapath. Ales Musil (6): ct-dpif: Handle default zone limit the same way as other limits. dpctl: Allow the default CT zone limit to de deleted. ovs-vsctl: Add limit to CT zone. vswitchd, ofproto-dpif: Propagate the CT limit from database. ct-dpif: Enforce CT zone limit protection. tests: Do not use zone 0 for CT limit system test. NEWS | 7 ++ lib/conntrack.c| 14 ++-- lib/conntrack.h| 7 +- lib/ct-dpif.c | 53 ++--- lib/ct-dpif.h | 16 ++-- lib/dpctl.c| 48 --- lib/dpif-netdev.c | 21 ++--- lib/dpif-netlink.c | 29 ++- lib/dpif-provider.h| 24 +++--- ofproto/ofproto-dpif.c | 52 ofproto/ofproto-dpif.h | 5 ++ ofproto/ofproto-provider.h | 13 +++ ofproto/ofproto.c | 23 ++ ofproto/ofproto.h | 4 + tests/dpctl.at | 8 +- tests/ovs-vsctl.at | 88 +++- tests/system-traffic.at| 159 + utilities/ovs-vsctl.8.in | 31 ++-- utilities/ovs-vsctl.c | 141 ++-- vswitchd/bridge.c | 82 ++- vswitchd/vswitch.ovsschema | 14 +++- vswitchd/vswitch.xml | 14 22 files changed, 709 insertions(+), 144 deletions(-) -- 2.43.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH OVN] Add support to make fdb table local to the chassis.
References: <20231129064550.96265-1-naveen.yerramn...@nutanix.com> Bleep bloop. Greetings naveen.yerramneni, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: Author naveen.yerramneni needs to sign off. WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Naveen Yerramneni WARNING: Line is 82 characters long (recommended limit is 79) #100 FILE: lib/actions.c:5268: ovnact_commit_fdb_local_free(struct ovnact_commit_fdb_local *fdb_local OVS_UNUSED) WARNING: Line is 80 characters long (recommended limit is 79) #160 FILE: lib/actions.c:5328: commit_fdb_local_learn_action(fdb_local, ofpacts, ep->lflow_uuid.parts[0]); ERROR: Inappropriate bracing around statement #206 FILE: northd/northd.c:7055: if (ls_is_fdb_local(op->od->nbs)) Lines checked: 386, Warnings: 3, Errors: 2 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH OVN] Add support to make fdb table local to the chassis.
This functionality can be enabled at the logical switch level:
- "other_config:fdb_local" can be used to enable/disable this
functionality, it is disabled by default.
- "other_config:fdb_local_idle_timeout" sepcifies idle timeout
for locally learned fdb flows, default timeout is 300 secs.
If enabled, below lflow is added for each port that has unknown addr set.
- table=2 (ls_in_lookup_fdb), priority=100, match=(inport == ),
action=(commit_fdb_local(timeout=); next;
New OVN action: "commit_fdb_local". This sets following OVS action.
- learn(table=71,idle_timeout=,delete_learned,OXM_OF_METADATA[],
NXM_OF_ETH_DST[]=NXM_OF_ETH_SRC[],load:NXM_NX_REG14[]->NXM_NX_REG15[])
This is useful when OVN is managing VLAN network that has multiple ports
set with unknown addr and localnet_learn_fdb is enabled. With this config,
if there is east-west traffic flowing between VMs part of same VLAN
deployed on different hypervisors then, MAC addrs of the source and
destination VMs keeps flapping between VM port and localnet port in
Southbound FDB table. Enabling fdb_local config makes fdb table local to
the chassis and avoids MAC flapping.
Signed-off-by: Naveen Yerramneni
---
include/ovn/actions.h | 7 +++
lib/actions.c | 94
northd/northd.c | 26 ++
ovn-nb.xml| 14 ++
tests/ovn.at | 108 ++
utilities/ovn-trace.c | 2 +
6 files changed, 251 insertions(+)
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 49cfe0624..85ac92cd3 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -127,6 +127,7 @@ struct collector_set_ids;
OVNACT(CHK_LB_AFF,ovnact_result) \
OVNACT(SAMPLE,ovnact_sample) \
OVNACT(MAC_CACHE_USE, ovnact_null)\
+OVNACT(COMMIT_FDB_LOCAL, ovnact_commit_fdb_local) \
/* enum ovnact_type, with a member OVNACT_ for each action. */
enum OVS_PACKED_ENUM ovnact_type {
@@ -514,6 +515,12 @@ struct ovnact_commit_lb_aff {
uint16_t timeout;
};
+/* OVNACT_COMMIT_FBD_LOCAL. */
+struct ovnact_commit_fdb_local{
+struct ovnact ovnact;
+uint16_t timeout; /* fdb_local flow timeout */
+};
+
/* Internal use by the helpers below. */
void ovnact_init(struct ovnact *, enum ovnact_type, size_t len);
void *ovnact_put(struct ofpbuf *, enum ovnact_type, size_t len);
diff --git a/lib/actions.c b/lib/actions.c
index a73fe1a1e..f5aa78db1 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -5236,6 +5236,98 @@ format_MAC_CACHE_USE(const struct ovnact_null *null
OVS_UNUSED, struct ds *s)
ds_put_cstr(s, "mac_cache_use;");
}
+static void
+parse_commit_fdb_local(struct action_context *ctx,
+ struct ovnact_commit_fdb_local *fdb_local)
+{
+uint16_t timeout = 0;
+lexer_force_match(ctx->lexer, LEX_T_LPAREN); /* Skip '('. */
+if (!lexer_match_id(ctx->lexer, "timeout")) {
+lexer_syntax_error(ctx->lexer, "invalid parameter");
+return;
+}
+if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) {
+lexer_syntax_error(ctx->lexer, "invalid parameter");
+return;
+}
+if (!action_parse_uint16(ctx, &timeout, "fdb_local flow timeout")) {
+return;
+}
+fdb_local->timeout = timeout;
+lexer_force_match(ctx->lexer, LEX_T_RPAREN); /* Skip ')'. */
+}
+
+static void
+format_COMMIT_FDB_LOCAL(const struct ovnact_commit_fdb_local *fdb_local,
+ struct ds *s)
+{
+ds_put_format(s, "commit_fdb_local(timeout=%u);", fdb_local->timeout);
+}
+
+static void
+ovnact_commit_fdb_local_free(struct ovnact_commit_fdb_local *fdb_local
OVS_UNUSED)
+{
+}
+
+static void
+commit_fdb_local_learn_action(struct ovnact_commit_fdb_local *fdb_local,
+struct ofpbuf *ofpacts, uint32_t cookie)
+{
+struct ofpact_learn *ol = ofpact_put_LEARN(ofpacts);
+struct match match = MATCH_CATCHALL_INITIALIZER;
+struct ofpact_learn_spec *ol_spec;
+unsigned int imm_bytes;
+uint8_t *src_imm;
+
+ol->flags = NX_LEARN_F_DELETE_LEARNED;
+ol->idle_timeout = fdb_local->timeout;
+ol->hard_timeout = OFP_FLOW_PERMANENT;
+ol->priority = OFP_DEFAULT_PRIORITY;
+ol->table_id = OFTABLE_GET_FDB;
+ol->cookie = htonll(cookie);
+
+/* Match on metadata of the packet that created the new table. */
+ol_spec = ofpbuf_put_zeros(ofpacts, sizeof *ol_spec);
+ol_spec->dst.field = mf_from_id(MFF_METADATA);
+ol_spec->dst.ofs = 0;
+ol_spec->dst.n_bits = ol_spec->dst.field->n_bits;
+ol_spec->n_bits = ol_spec->dst.n_bits;
+ol_spec->dst_type = NX_LEARN_DST_MATCH;
+ol_spec->src_type = NX_LEARN_SRC_FIELD;
+ol_spec->src.field = mf_from_id(MFF_METADATA);
+
+/* Match on metadata of the packet. */
+ol_spec = ofpbuf_put_zeros(ofpacts, sizeof *ol_spec);
+ol_spec->dst.field = mf_from_id(MFF_ETH_DST);
+ol_spec->dst.ofs = 0;
+ol_spec->d
Re: [ovs-dev] [PATCH v2 1/9] ci: Add make check-ovsdb-cluster tests to GitHub action ci.
On 11/28/23 19:15, Eelco Chaudron wrote:
>
>
> On 28 Nov 2023, at 13:07, Ilya Maximets wrote:
>
>> On 11/28/23 08:39, Eelco Chaudron wrote:
>>>
>>>
>>> On 27 Nov 2023, at 18:58, Ilya Maximets wrote:
>>>
On 11/27/23 13:38, Eelco Chaudron wrote:
> Signed-off-by: Eelco Chaudron
> ---
> .ci/linux-build.sh |9 +
> .github/workflows/build-and-test.yml |3 +++
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index aa2ecc505..e9e1e24b5 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -6,6 +6,7 @@ set -x
> CFLAGS_FOR_OVS="-g -O2"
> SPARSE_FLAGS=""
> EXTRA_OPTS="--enable-Werror"
> +JOBS=${JOBS:-"-j4"}
Is this used anywhere? Seems a little orthogonal to the
purpose of this set.
>>>
>>> Thought rather than adding -j4 in more places, having a seperate definition
>>> for it would allow it to be changed easily if we ever need to (and when
>>> using the script outside of GitHub actions).
>>>
>>> I can make it a seperate patch if that is preferred.
>>
>> Would be better. The only thing that annoys me is that the definition
>> above allows passing JOBS via environment in order to override, but no
>> other variable is defined this way, and the functionality is not actually
>> used.
>
> I’ll make it a seperate patch. I know it’s the only value so far but for a
> reason :) If I want to use this script in my own container for testing I can
> give it more cores, and it will run faster.
Hmm. I didn't consider this scenario. :)
Let's keep it then, no problem.
>
> But if you really do not like it, I’ll do some ‘sed’ in my scripts to make a
> hard-coded change.
>
> function install_dpdk()
> {
> @@ -46,7 +47,7 @@ function build_ovs()
> configure_ovs $OPTS
> make selinux-policy
>
> -make -j4
> +make $JOBS
> }
>
> if [ "$DEB_PACKAGE" ]; then
> @@ -122,8 +123,8 @@ if [ "$TESTSUITE" = 'test' ]; then
> configure_ovs
>
> export DISTCHECK_CONFIGURE_FLAGS="$OPTS"
> -make distcheck -j4 CFLAGS="${CFLAGS_FOR_OVS}" \
> -TESTSUITEFLAGS=-j4 RECHECK=yes
> +make distcheck $JOBS CFLAGS="${CFLAGS_FOR_OVS}" \
> +TESTSUITEFLAGS=$JOBS RECHECK=yes
>>
>> Nit: It's better to use ${JOBS} syntax. Not necessary though.
>
> Will fix those.
>
> else
> build_ovs
> for testsuite in $TESTSUITE; do
> @@ -134,7 +135,7 @@ else
> export DPDK_EAL_OPTIONS="--lcores 0@1,1@1,2@1"
> run_as_root="sudo -E PATH=$PATH"
> fi
> -$run_as_root make $testsuite TESTSUITEFLAGS=-j4 RECHECK=yes
> +$run_as_root make $testsuite TESTSUITEFLAGS=$JOBS RECHECK=yes
>>
>> Ditto.
>>
> done
> fi
>
> diff --git a/.github/workflows/build-and-test.yml
> b/.github/workflows/build-and-test.yml
> index 09654205e..5d441157c 100644
> --- a/.github/workflows/build-and-test.yml
> +++ b/.github/workflows/build-and-test.yml
> @@ -164,6 +164,9 @@ jobs:
> m32: m32
> opts: --disable-ssl
>
> + - compiler: gcc
> +testsuite:check-ovsdb-cluster
FWIW, there is no need to run this testsuite as root.
But I'm not sure it worth changing the scripts in order
to avoid that.
>>>
>>> ACK, will keep it as is.
>>>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 3/9] ci: Add make check-offloads to GitHub actions ci.
On 28 Nov 2023, at 13:10, Ilya Maximets wrote: > On 11/28/23 08:43, Eelco Chaudron wrote: >> >> >> On 27 Nov 2023, at 19:04, Ilya Maximets wrote: >> >>> On 11/27/23 13:39, Eelco Chaudron wrote: This patch also adds the 'CHECK_GITHUB_ACTION' macro to skip tests that won't execute successfully through GitHub actions. We could not use the -k !keyword option, as it can not be combined with a range of tests. Signed-off-by: Eelco Chaudron --- .ci/linux-build.sh |2 +- .github/workflows/build-and-test.yml |7 +++ tests/system-common-macros.at|4 tests/system-offloads-traffic.at |2 ++ 4 files changed, 14 insertions(+), 1 deletion(-) diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index 4f2e36610..85788748f 100755 --- a/.ci/linux-build.sh +++ b/.ci/linux-build.sh @@ -139,7 +139,7 @@ else export DPDK_EAL_OPTIONS="--lcores 0@1,1@1,2@1" fi $run_as_root make $testsuite TESTSUITEFLAGS="$JOBS $TEST_RANGE" \ - RECHECK=yes + RECHECK=yes GITHUB_ACTIONS=$GITHUB_ACTIONS done fi diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 0b881ca91..586b0cdd9 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -176,6 +176,13 @@ jobs: testsuite:check-kernel test_range: "100-" + - compiler: gcc +testsuite:check-offloads +test_range: "-100" + - compiler: gcc +testsuite:check-offloads +test_range: "100-" + steps: - name: checkout uses: actions/checkout@v3 diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at index 0113aae8b..0620be0c7 100644 --- a/tests/system-common-macros.at +++ b/tests/system-common-macros.at @@ -365,3 +365,7 @@ m4_define([OVS_CHECK_IPROUTE_ENCAP], # OVS_CHECK_CT_CLEAR() m4_define([OVS_CHECK_CT_CLEAR], [AT_SKIP_IF([! grep -q "Datapath supports ct_clear action" ovs-vswitchd.log])]) + +# OVS_CHECK_GITHUB_ACTION +m4_define([OVS_CHECK_GITHUB_ACTION], +[AT_SKIP_IF([test "$GITHUB_ACTIONS" = "true"])]) >>> >>> Can we use some pre-defined GHA env variable instead? >>> Or are they not available? >> >> This is a pre-defined GH env. See above we re-export it as we run as root. > > Hmm. I got confused, because you're passing it directly to 'make' > and not as part of 'run_as_root'. Maybe move it there, so it's > obvious that the problem is the same as with PATH? ACK will move it. >> diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at index 0bedee753..6bd49a3ee 100644 --- a/tests/system-offloads-traffic.at +++ b/tests/system-offloads-traffic.at @@ -192,6 +192,7 @@ AT_CLEANUP AT_SETUP([offloads - check interface meter offloading - offloads disabled]) AT_KEYWORDS([dp-meter]) AT_SKIP_IF([test $HAVE_NC = "no"]) +OVS_CHECK_GITHUB_ACTION() OVS_TRAFFIC_VSWITCHD_START() AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br0 'meter=1 pktps bands=type=drop rate=1']) @@ -240,6 +241,7 @@ AT_CLEANUP AT_SETUP([offloads - check interface meter offloading - offloads enabled]) AT_KEYWORDS([offload-meter]) +OVS_CHECK_GITHUB_ACTION() CHECK_TC_INGRESS_PPS() AT_SKIP_IF([test $HAVE_NC = "no"]) OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 1/9] ci: Add make check-ovsdb-cluster tests to GitHub action ci.
On 28 Nov 2023, at 13:07, Ilya Maximets wrote:
> On 11/28/23 08:39, Eelco Chaudron wrote:
>>
>>
>> On 27 Nov 2023, at 18:58, Ilya Maximets wrote:
>>
>>> On 11/27/23 13:38, Eelco Chaudron wrote:
Signed-off-by: Eelco Chaudron
---
.ci/linux-build.sh |9 +
.github/workflows/build-and-test.yml |3 +++
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index aa2ecc505..e9e1e24b5 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -6,6 +6,7 @@ set -x
CFLAGS_FOR_OVS="-g -O2"
SPARSE_FLAGS=""
EXTRA_OPTS="--enable-Werror"
+JOBS=${JOBS:-"-j4"}
>>>
>>> Is this used anywhere? Seems a little orthogonal to the
>>> purpose of this set.
>>
>> Thought rather than adding -j4 in more places, having a seperate definition
>> for it would allow it to be changed easily if we ever need to (and when
>> using the script outside of GitHub actions).
>>
>> I can make it a seperate patch if that is preferred.
>
> Would be better. The only thing that annoys me is that the definition
> above allows passing JOBS via environment in order to override, but no
> other variable is defined this way, and the functionality is not actually
> used.
I’ll make it a seperate patch. I know it’s the only value so far but for a
reason :) If I want to use this script in my own container for testing I can
give it more cores, and it will run faster.
But if you really do not like it, I’ll do some ‘sed’ in my scripts to make a
hard-coded change.
function install_dpdk()
{
@@ -46,7 +47,7 @@ function build_ovs()
configure_ovs $OPTS
make selinux-policy
-make -j4
+make $JOBS
}
if [ "$DEB_PACKAGE" ]; then
@@ -122,8 +123,8 @@ if [ "$TESTSUITE" = 'test' ]; then
configure_ovs
export DISTCHECK_CONFIGURE_FLAGS="$OPTS"
-make distcheck -j4 CFLAGS="${CFLAGS_FOR_OVS}" \
-TESTSUITEFLAGS=-j4 RECHECK=yes
+make distcheck $JOBS CFLAGS="${CFLAGS_FOR_OVS}" \
+TESTSUITEFLAGS=$JOBS RECHECK=yes
>
> Nit: It's better to use ${JOBS} syntax. Not necessary though.
Will fix those.
else
build_ovs
for testsuite in $TESTSUITE; do
@@ -134,7 +135,7 @@ else
export DPDK_EAL_OPTIONS="--lcores 0@1,1@1,2@1"
run_as_root="sudo -E PATH=$PATH"
fi
-$run_as_root make $testsuite TESTSUITEFLAGS=-j4 RECHECK=yes
+$run_as_root make $testsuite TESTSUITEFLAGS=$JOBS RECHECK=yes
>
> Ditto.
>
done
fi
diff --git a/.github/workflows/build-and-test.yml
b/.github/workflows/build-and-test.yml
index 09654205e..5d441157c 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -164,6 +164,9 @@ jobs:
m32: m32
opts: --disable-ssl
+ - compiler: gcc
+testsuite:check-ovsdb-cluster
>>>
>>> FWIW, there is no need to run this testsuite as root.
>>> But I'm not sure it worth changing the scripts in order
>>> to avoid that.
>>
>> ACK, will keep it as is.
>>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] ci: Add clang-analyze to GitHub actions.
On 27 Nov 2023, at 19:18, Ilya Maximets wrote:
> On 11/27/23 18:36, Eelco Chaudron wrote:
>> This patch detects new static analyze issues, and report them.
>> It does this by reporting on the delta for this branch, compared
>> to the previous branch.
>>
>> For example the error might look like this:
>>
>> error level: +0 -0 no changes
>> warning level: +2 +0
>> New issue "deadcode.DeadStores Value stored to 'remote' is never read"
>> (1 occurrence)
>> file:///home/runner/work/ovs/ovs/vswitchd/ovs-vswitchd.c:86
>> New issue "unix.Malloc Potential leak of memory pointed to by 'remote'"
>> (1 occurrence)
>> file:///home/runner/work/ovs/ovs/vswitchd/ovs-vswitchd.c:95
>> note level: +0 -0 no changes
>> all levels: +2 +0
>>
>> Signed-off-by: Eelco Chaudron
>> ---
>>
>> changes in v2:
>> - When it's a new branch, it compares it to the HEAD of the default branch.
>>
>> .ci/linux-build.sh | 29 ++
>> .github/workflows/build-and-test.yml | 96
>> ++
>> 2 files changed, 125 insertions(+)
>>
>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>> index aa2ecc505..fedf1398a 100755
>> --- a/.ci/linux-build.sh
>> +++ b/.ci/linux-build.sh
>> @@ -49,6 +49,30 @@ function build_ovs()
>> make -j4
>> }
>>
>> +function clang_analyze()
>> +{
>> +[ -d "./base-clang-analyzer-results" ] && cache_build=false \
>> + || cache_build=true
>> +if [ "$cache_build" = true ]; then
>> +# If this is a cache build, proceed to the base branch's directory.
>> +cd base_ovs_main
>> +fi;
>> +
>> +configure_ovs $OPTS
>> +make clean
>> +scan-build -o ./clang-analyzer-results -sarif --use-cc=clang make -j4
>> +
>> +if [ "$cache_build" = true ]; then
>> +# Move results, so it will be picked up by the cache.
>> +mv ./clang-analyzer-results ../base-clang-analyzer-results
>> +cd ..
>> +else
>> +# Only do the compare on the none cache builds.
>> +sarif --check note diff ./base-clang-analyzer-results \
>> +./clang-analyzer-results
>> +fi;
>> +}
>> +
>> if [ "$DEB_PACKAGE" ]; then
>> ./boot.sh && ./configure --with-dpdk=$DPDK && make debian
>> mk-build-deps --install --root-cmd sudo --remove debian/control
>> @@ -116,6 +140,11 @@ fi
>>
>> OPTS="${EXTRA_OPTS} ${OPTS} $*"
>>
>> +if [ "$CLANG_ANALYZE" ]; then
>> +clang_analyze
>> +exit 0
>> +fi
>> +
>> if [ "$TESTSUITE" = 'test' ]; then
>> # 'distcheck' will reconfigure with required options.
>> # Now we only need to prepare the Makefile without sparse-wrapped CC.
>> diff --git a/.github/workflows/build-and-test.yml
>> b/.github/workflows/build-and-test.yml
>> index 09654205e..d15105e7d 100644
>> --- a/.github/workflows/build-and-test.yml
>> +++ b/.github/workflows/build-and-test.yml
>> @@ -223,6 +223,102 @@ jobs:
>> name: logs-linux-${{ join(matrix.*, '-') }}
>> path: logs.tgz
>>
>> + build-clang-analyze:
>> +needs: build-dpdk
>> +env:
>> + dependencies: |
>> +automake bc clang-tools libbpf-dev libnuma-dev libpcap-dev \
>> +libunbound-dev libunwind-dev libssl-dev libtool llvm-dev \
>> +python3-unbound
>> + CC: clang
>> + DPDK: dpdk
>> + CLANG_ANALYZE: true
>> +name: clang-analyze
>> +runs-on: ubuntu-22.04
>> +timeout-minutes: 30
>> +
>> +steps:
>> +- name: checkout
>> + uses: actions/checkout@v3
>> +
>> +- name: get base branch sha
>> + id: base_branch
>> + run: |
>> +if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then
>> + echo "sha=$BASE_SHA" >> $GITHUB_OUTPUT
>> +else
>> + if [ "$EVENT_BEFORE" = ""
>> ]; then
>> +echo "sha=$DEFAULT_BRANCH" >> $GITHUB_OUTPUT
>
> How this is going ot work on patches for older branches?
Good question, it’s not :( Took a little bit to figure out how to do this, as
we have no reference branch. The solution I came up with was to figure out all
parent branches, and use the most recent main/master or branch-x.x one. So it
looks like this:
if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then
echo "sha=$BASE_SHA" >> $GITHUB_OUTPUT
else
if [ "$EVENT_BEFORE" = "" ];
then
set sha=$(git log --simplify-by-decoration --decorate=full \
--pretty=format:'%d' | \
grep -oP 'refs/remotes/origin/\K[^, )]+' | \
grep -m1 -E '^master$|^main$|^branch-[0-9]+\.[0-9]+$')
[ -z "$sha" ] && echo "sha=$DEFAULT_BRANCH" >> $GITHUB_OUTPUT \
|| echo "sha=$sha" >> $GITHUB_OUTPUT
else
echo "sha=$EVENT_BEFORE" >> $GITHUB_OUTPUT
fi
fi
>> + else
>> +echo "sha=$EVENT
[ovs-dev] [PATCH ovn v2 4/4] fmt_pkt: make sure scapy-server is started once
When running fmt_pkt in parallel, before the patch, there could be
several attempts to start scapy-server. Using flock on a test case
specific file should guarantee that only one of the subshells will
actually be able to start a daemon, which is designed to be a singleton.
Now that start_scapy_server doesn't necessarily start a server (only the
first lucky subshell will do), wait for server socket file before
proceeding with ovs-appctl call.
Signed-off-by: Ihar Hrachyshka
Acked-by: Mark Michelson
---
tests/ovn-macros.at | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
index 21c314c7c..633ef05f8 100644
--- a/tests/ovn-macros.at
+++ b/tests/ovn-macros.at
@@ -876,6 +876,7 @@ fmt_pkt() {
if [[ ! -S $ctlfile ]]; then
start_scapy_server
fi
+while [[ ! -S $ctlfile ]]; do sleep 0.1; done
ovs-appctl -t $ctlfile payload "$1"
}
@@ -883,10 +884,11 @@ start_scapy_server() {
pidfile=$ovs_base/scapy.pid
ctlfile=$ovs_base/scapy.ctl
logfile=$ovs_base/scapy.log
+lockfile=$ovs_base/scapy.lock
-"$top_srcdir"/tests/scapy-server.py \
---pidfile=$pidfile --unixctl=$ctlfile --log-file=$logfile --detach
-on_exit "test -e \"$pidfile\" && ovs-appctl -t $ctlfile exit"
+flock -n $lockfile "$top_srcdir"/tests/scapy-server.py \
+--pidfile=$pidfile --unixctl=$ctlfile --log-file=$logfile --detach \
+&& on_exit "test -e \"$pidfile\" && ovs-appctl -t $ctlfile exit"
}
sleep_sb() {
--
2.41.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v2 0/4] misc fmt_pkt improvements
This series, combined with ovs series [1], will allow to execute fmt_pkt in parallel. This will allow to revert back to parallel execution of test_ip in a test conversion patch from Mark [2], and speed up the case while at it. (I will post a separate patch to switch back to background test_ip execution after [1], [2] as well as this series are all merged.) --- v1: initial v2: use perf_counter in scapy-server time logging log payload request before any processing Ihar Hrachyshka (4): fmt_pkt: don't subshell when calling ovs-appctl fmt_pkt: use -S check to wait for scapy sock file fmt_pkt: improve scapy-server logging fmt_pkt: make sure scapy-server is started once tests/ovn-macros.at | 13 + tests/scapy-server.py | 16 ++-- 2 files changed, 23 insertions(+), 6 deletions(-) -- 2.41.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v2 1/4] fmt_pkt: don't subshell when calling ovs-appctl
Signed-off-by: Ihar Hrachyshka
Acked-by: Mark Michelson
---
tests/ovn-macros.at | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
index 8dc4ec75c..3191bb6ad 100644
--- a/tests/ovn-macros.at
+++ b/tests/ovn-macros.at
@@ -876,7 +876,7 @@ fmt_pkt() {
if [[ ! -e $ctlfile ]]; then
start_scapy_server
fi
-echo $(ovs-appctl -t $ctlfile payload "$1")
+ovs-appctl -t $ctlfile payload "$1"
}
start_scapy_server() {
--
2.41.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v2 3/4] fmt_pkt: improve scapy-server logging
The daemon will now log to scapy.log file.
Log messages include stats on request processing time as well as any
errors that may happen during processing.
If you'd like to see even more logs (e.g. for debugging purposes), just
pass --verbose to scapy-server.
Signed-off-by: Ihar Hrachyshka
---
tests/ovn-macros.at | 5 -
tests/scapy-server.py | 16 ++--
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
index 0b15bcc80..21c314c7c 100644
--- a/tests/ovn-macros.at
+++ b/tests/ovn-macros.at
@@ -882,7 +882,10 @@ fmt_pkt() {
start_scapy_server() {
pidfile=$ovs_base/scapy.pid
ctlfile=$ovs_base/scapy.ctl
-"$top_srcdir"/tests/scapy-server.py --pidfile=$pidfile --unixctl=$ctlfile
--detach
+logfile=$ovs_base/scapy.log
+
+"$top_srcdir"/tests/scapy-server.py \
+--pidfile=$pidfile --unixctl=$ctlfile --log-file=$logfile --detach
on_exit "test -e \"$pidfile\" && ovs-appctl -t $ctlfile exit"
}
diff --git a/tests/scapy-server.py b/tests/scapy-server.py
index a7255c84d..1cc616f70 100755
--- a/tests/scapy-server.py
+++ b/tests/scapy-server.py
@@ -1,6 +1,7 @@
#!/usr/bin/env python3
import argparse
+import time
import ovs.daemon
import ovs.unixctl
@@ -23,15 +24,24 @@ def exit(conn, argv, aux):
def process(data):
+start_time = time.perf_counter()
+vlog.info(f"received payload request: {data}")
try:
data = data.replace('\n', '')
return binascii.hexlify(raw(eval(data))).decode()
-except Exception:
+except Exception as e:
+vlog.exception(f"failed to process payload request: {e}")
return ""
+finally:
+total_time = (time.perf_counter() - start_time) * 1000
+vlog.info(f"took {total_time:.2f}ms to process payload request")
def payload(conn, argv, aux):
-conn.reply(process(argv[0]))
+try:
+conn.reply(process(argv[0]))
+except Exception as e:
+vlog.exception(f"failed to reply to payload request: {e}")
def main():
@@ -55,6 +65,8 @@ def main():
ovs.unixctl.command_register("payload", "", 1, 1, payload, None)
ovs.daemon.daemonize_complete()
+vlog.info("scapy server ready")
+
poller = ovs.poller.Poller()
while not exiting:
server.run()
--
2.41.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v2 2/4] fmt_pkt: use -S check to wait for scapy sock file
While either check works, it's better to use a more explicit check.
Signed-off-by: Ihar Hrachyshka
Acked-by: Mark Michelson
---
tests/ovn-macros.at | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
index 3191bb6ad..0b15bcc80 100644
--- a/tests/ovn-macros.at
+++ b/tests/ovn-macros.at
@@ -873,7 +873,7 @@ ovn_trace_client() {
#
fmt_pkt() {
ctlfile=$ovs_base/scapy.ctl
-if [[ ! -e $ctlfile ]]; then
+if [[ ! -S $ctlfile ]]; then
start_scapy_server
fi
ovs-appctl -t $ctlfile payload "$1"
--
2.41.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v2 00/18] northd lflow incremental processing
On 11/24/23 16:41, Numan Siddique wrote: > On Fri, Nov 24, 2023 at 10:38 AM Dumitru Ceara wrote: >> >> On 11/16/23 22:05, Numan Siddique wrote: >>> On Thu, Nov 16, 2023 at 2:54 PM Han Zhou wrote: On Wed, Nov 15, 2023 at 7:32 PM Numan Siddique wrote: > > On Wed, Nov 15, 2023 at 2:59 AM Han Zhou wrote: >> >> On Thu, Oct 26, 2023 at 11:12 AM wrote: >>> >>> From: Numan Siddique >>> >>> This patch series adds incremental processing in the lflow engine >>> node to handle changes to northd and other engine nodes. >>> Changed related to load balancers and NAT are mainly handled in >>> this patch series. >>> >>> This patch series can also be found here - >> https://github.com/numansiddique/ovn/tree/northd_lflow_ip/v1 >> >> Thanks Numan for the great improvement! > > Hi Han, > > Thanks for the review comments. I understand it is hard to review > somany patches, especially related to I-P. > I appreciate the time spent on it and the feedback. > >> >> I spent some time these days to review the series and now at patch 10. I >> need to take a break and so I just sent the comments I had so far. >> >> I also did scale testing for northd with >> https://github.com/hzhou8/ovn-test-script for a 500 chassis, 50 lsp / >> chassis setup, and noticed that the recompute latency has increased 20% >> after the series. I think in general it is expected to have some >> performance penalty for the recompute case because of the new indexes >> created for I-P. However, the patch 10 "northd: Refactor lflow management >> into a separate module." alone introduces a 10% latency increase, which >> necessitates more investigation. > > Before sending out the series I did some testing on recomputes with a > large OVN NB DB and SB DB > (from a 500 node ovn-heater density heavy run). > I'm aware of the increase in recomputes. And I did some more testing > today as well. > > In my testing, I can see that the increase in latency is due to the > new engine nodes added (lr_lbnat mainly) > and due to housekeeping of the lflow references. I do not see any > increase due to the new lflow-mgr.c added in patch 10. > > I compared patch 9 and patch 10 separately and there is no difference > in lflow engine node recompute time between patch 9 and 10. > My results were different. My test profile simulates the ovn-k8s topology (central mode, not IC) with 500 nodes, 50 LSP/node, with no LB, small amount of ACLs and PGs. ( https://github.com/hzhou8/ovn-test-script/blob/main/args.500ch_50lsp_1pg ) The test results for ovn-northd recompute time are: main: 1118.3 p9: 1129.5 p10: 1243.4 ==> 10% more than p9 p18: 1357.6 I am not sure if the p10 increase is related to the hash change or something else. >> >> I didn't review p10 in detail yet but I did run some tests and (with gcc >> and CFLAGS="-O2 -fno-omit-frame-pointer -fno-common -g") I see no >> significant difference between p9 and p10 when running Han's scale test >> profile. >> >> Then I also tried the same thing using the 500 nodes + 50 LSP/node perf >> test we already have in the repo: >> >> https://github.com/ovn-org/ovn/blob/5ef2a08ade01d698f84e197987ea679d8978d2b9/tests/perf-northd.at#L185 >> >> Again, I didn't see any significant change between p9 and p10 on my >> laptop. I wonder what's different in our setups. >> >> Kind of related, I posted a series to improve the in-tree perf testing >> and allow us to also gather recompute stats (build the DB, reset >> stopwatches and trigger a recompute, then collect stopwatches): >> >> https://patchwork.ozlabs.org/project/ovn/list/?series=383727&state=* >> >> Would it be an idea to try to merge both scenarios you guys used into >> something that's defined as a new test in-tree? Like that it's easier >> for anyone to just run the same test. > > That sounds good to me. I'll take a look at your patches soon. > > Thanks for testing them out. Can you also please compare OVN main vs > p10 of this series ? > Sorry for the delay in replying; I ran some more tests both with Han's benchmark and also with the in-tree check-perf 500 node + 50 LSP/node test. I did this with debug logs disabled and ran them multiple times to collect some more relevant averages (the results did seem more stable). This is what I measured (where "pX" == "patch X of this series" and values are averages in milliseconds): --- | Benchmark | Recompute "main" | Recompute p9 | Recompute p10 | Recompute p16 | |-- | Han's | 1552|1560 | 1676 | 1704 | ---
Re: [ovs-dev] Next OVN technical community meeting
On 11/26/23 18:59, Dumitru Ceara wrote: > On 10/24/23 09:34, Dumitru Ceara wrote: >> On 10/23/23 11:58, Dumitru Ceara wrote: In the meantime, as discussed, I sent out an invite to the people in this thread but I'm also sharing it here for anyone from the community to join if they like it. Date/Time: Monday October 23rd 15:00 UTC Video Link: https://meet.google.com/zns-gqsd-jdn Meeting notes: https://docs.google.com/document/d/1N61LY_QHEALD-KOfymqrceQrlyHMpMDwOJiqw3MdIx0/edit >>> Updated link to meeting notes (with public commenter access): >>> >>> https://docs.google.com/document/d/1dG4GwcYOSs4uArPGtOoaP5tH4KCto-GH_C3tIXSnZZ8 >>> Agenda (until now): 1. What new features or relevant changes does the community think are important to have in the next release (24.03.0)? 2. Decide on a cadence for the technical meetings. >> >> Thanks to everyone who attended! And for people who couldn't make it >> but are interested in what was discussed please find below the meeting >> notes [0] and recording [1]. >> >> [0] >> https://docs.google.com/document/d/1dG4GwcYOSs4uArPGtOoaP5tH4KCto-GH_C3tIXSnZZ8/edit#heading=h.4lj9zlqqdtzm >> >> [1] https://drive.google.com/file/d/1sK3-KVT726FXc0kLrKcS83jwh5gPYAuc/view >> >> As discussed, I went ahead and scheduled another instance: >> >> Date/Time: Monday November 27th 15:00 UTC >> Video Link: meet.google.com/zns-gqsd-jdn >> Meeting notes: >> https://docs.google.com/document/d/1dG4GwcYOSs4uArPGtOoaP5tH4KCto-GH_C3tIXSnZZ8 >> > > Hi all, > > A reminder for those who wish to attend: the next instance of the OVN > technical community meeting (audio/video) is scheduled to happen tomorrow: > > Date/Time: Monday November 27th 15:00 UTC > Video Link: meet.google.com/zns-gqsd-jdn > Meeting notes: > https://docs.google.com/document/d/1dG4GwcYOSs4uArPGtOoaP5tH4KCto-GH_C3tIXSnZZ8 > > Feel free to already add items you might want to discuss to the agenda > in the document linked above. > Thanks to everyone who attended! And for people who couldn't make it but are interested in what was discussed please find below the meeting notes [0], recording [1] and transcript [2]. [0] https://docs.google.com/document/d/1dG4GwcYOSs4uArPGtOoaP5tH4KCto-GH_C3tIXSnZZ8/edit#heading=h.46qhcmjkgp4m [1] https://drive.google.com/file/d/1Wlz57QPTwFxuEz8krxoty5oivzV6C1TT/view?usp=drive_link [2] https://drive.google.com/file/d/1iyRBbhae2lD98St22o58kzQAHL0X5thE/view?usp=drive_link I went ahead and scheduled another instance (please let me know if the date/time doesn't work for you): Date/Time: Monday January 15th 15:00 UTC Video Link: meet.google.com/zns-gqsd-jdn Meeting notes: https://docs.google.com/document/d/1dG4GwcYOSs4uArPGtOoaP5tH4KCto-GH_C3tIXSnZZ8 Best regards, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH branch-2.17] dpdk: Use DPDK 21.11.5 release for OVS 2.17.
On 27/11/2023 11:34, Ilya Maximets wrote: On 11/24/23 11:32, David Marchand wrote: On Thu, Nov 23, 2023 at 12:50 PM Kevin Traynor wrote: Update the CI and docs to use DPDK 21.11.5. Signed-off-by: Kevin Traynor --- .ci/linux-build.sh | 2 +- Documentation/faq/releases.rst | 2 +- Documentation/intro/install/dpdk.rst | 8 NEWS | 3 +++ 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index f5021e1a8..9464ea49c 100755 --- a/.ci/linux-build.sh +++ b/.ci/linux-build.sh @@ -221,5 +221,5 @@ fi if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then if [ -z "$DPDK_VER" ]; then -DPDK_VER="21.11.2" +DPDK_VER="21.11.5" fi install_dpdk $DPDK_VER diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst index 49895c595..0e0c589a3 100644 --- a/Documentation/faq/releases.rst +++ b/Documentation/faq/releases.rst @@ -211,5 +211,5 @@ Q: What DPDK version does each Open vSwitch release work with? 2.15.x 20.11.6 2.16.x 20.11.6 -2.17.x 21.11.2 +2.17.x 21.11.5 diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst index a284e6851..559e8eb1f 100644 --- a/Documentation/intro/install/dpdk.rst +++ b/Documentation/intro/install/dpdk.rst @@ -43,5 +43,5 @@ In addition to the requirements described in :doc:`general`, building Open vSwitch with DPDK will require the following: -- DPDK 21.11.2 +- DPDK 21.11.5 - A `DPDK supported NIC`_ @@ -74,7 +74,7 @@ Install DPDK $ cd /usr/src/ - $ wget https://fast.dpdk.org/rel/dpdk-21.11.2.tar.xz - $ tar xf dpdk-21.11.2.tar.xz - $ export DPDK_DIR=/usr/src/dpdk-stable-21.11.2 + $ wget https://fast.dpdk.org/rel/dpdk-21.11.5.tar.xz + $ tar xf dpdk-21.11.5.tar.xz + $ export DPDK_DIR=/usr/src/dpdk-stable-21.11.5 $ cd $DPDK_DIR diff --git a/NEWS b/NEWS index 7d4a8c081..642beb45b 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,7 @@ v2.17.9 - xx xxx - + - Bug fixes I see in the history that the "Bug fixes" characterization is usually added when releasing a version. So I am not sure it should be added in this patch. Good point. I just added it as i was updating the section, but yes it is unrelated and maybe it will interfere with some release script. It should not. Kind of unrelated change for this patch. Kevin, could you re-post removing these extra lines, please? Ack - I just sent a v2. thanks, Kevin. Best regards, Ilya Maximets. + - DPDK: + * OVS validated with DPDK 21.11.5 v2.17.8 - 17 Oct 2023 Otherwise, it lgtm. Reviewed-by: David Marchand ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 branch-3.2] dpdk: Use DPDK 22.11.3 release for OVS 3.2.
Update the CI and docs to use DPDK 22.11.3. Signed-off-by: Kevin Traynor Acked-by: Simon Horman Reviewed-by: David Marchand --- .github/workflows/build-and-test.yml | 2 +- Documentation/faq/releases.rst | 8 Documentation/intro/install/dpdk.rst | 8 NEWS | 2 ++ 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index bc5494e86..fd911c110 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -9,5 +9,5 @@ jobs: CC: gcc DPDK_GIT: https://dpdk.org/git/dpdk-stable - DPDK_VER: 22.11.1 + DPDK_VER: 22.11.3 name: dpdk gcc outputs: diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst index e6bda14e7..362bf4ec7 100644 --- a/Documentation/faq/releases.rst +++ b/Documentation/faq/releases.rst @@ -216,8 +216,8 @@ Q: What DPDK version does each Open vSwitch release work with? 2.15.x 20.11.6 2.16.x 20.11.6 -2.17.x 21.11.2 -3.0.x21.11.2 -3.1.x22.11.1 -3.2.x22.11.1 +2.17.x 21.11.5 +3.0.x21.11.5 +3.1.x22.11.3 +3.2.x22.11.3 diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst index 63a0ebb23..02eaf8b10 100644 --- a/Documentation/intro/install/dpdk.rst +++ b/Documentation/intro/install/dpdk.rst @@ -43,5 +43,5 @@ In addition to the requirements described in :doc:`general`, building Open vSwitch with DPDK will require the following: -- DPDK 22.11.1 +- DPDK 22.11.3 - A `DPDK supported NIC`_ @@ -74,7 +74,7 @@ Install DPDK $ cd /usr/src/ - $ wget https://fast.dpdk.org/rel/dpdk-22.11.1.tar.xz - $ tar xf dpdk-22.11.1.tar.xz - $ export DPDK_DIR=/usr/src/dpdk-stable-22.11.1 + $ wget https://fast.dpdk.org/rel/dpdk-22.11.3.tar.xz + $ tar xf dpdk-22.11.3.tar.xz + $ export DPDK_DIR=/usr/src/dpdk-stable-22.11.3 $ cd $DPDK_DIR diff --git a/NEWS b/NEWS index eb7a9b1ba..6e1f175c5 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ v3.2.2 - xx xxx + - DPDK: + * OVS validated with DPDK 22.11.3 v3.2.1 - 17 Oct 2023 -- 2.42.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 branch-3.1] dpdk: Use DPDK 22.11.3 release for OVS 3.1.
Update the CI and docs to use DPDK 22.11.3. Signed-off-by: Kevin Traynor Acked-by: Simon Horman Reviewed-by: David Marchand --- .github/workflows/build-and-test.yml | 2 +- Documentation/faq/releases.rst | 6 +++--- Documentation/intro/install/dpdk.rst | 8 NEWS | 2 ++ 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 80c449336..2a001f9ed 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -9,5 +9,5 @@ jobs: CC: gcc DPDK_GIT: https://dpdk.org/git/dpdk-stable - DPDK_VER: 22.11.1 + DPDK_VER: 22.11.3 name: dpdk gcc outputs: diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst index 9e1b42262..f956c7e10 100644 --- a/Documentation/faq/releases.rst +++ b/Documentation/faq/releases.rst @@ -215,7 +215,7 @@ Q: What DPDK version does each Open vSwitch release work with? 2.15.x 20.11.6 2.16.x 20.11.6 -2.17.x 21.11.2 -3.0.x21.11.2 -3.1.x22.11.1 +2.17.x 21.11.5 +3.0.x21.11.5 +3.1.x22.11.3 diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst index 63a0ebb23..02eaf8b10 100644 --- a/Documentation/intro/install/dpdk.rst +++ b/Documentation/intro/install/dpdk.rst @@ -43,5 +43,5 @@ In addition to the requirements described in :doc:`general`, building Open vSwitch with DPDK will require the following: -- DPDK 22.11.1 +- DPDK 22.11.3 - A `DPDK supported NIC`_ @@ -74,7 +74,7 @@ Install DPDK $ cd /usr/src/ - $ wget https://fast.dpdk.org/rel/dpdk-22.11.1.tar.xz - $ tar xf dpdk-22.11.1.tar.xz - $ export DPDK_DIR=/usr/src/dpdk-stable-22.11.1 + $ wget https://fast.dpdk.org/rel/dpdk-22.11.3.tar.xz + $ tar xf dpdk-22.11.3.tar.xz + $ export DPDK_DIR=/usr/src/dpdk-stable-22.11.3 $ cd $DPDK_DIR diff --git a/NEWS b/NEWS index 13229cb3a..70d9f5ade 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ v3.1.4 - xx xxx + - DPDK: + * OVS validated with DPDK 22.11.3 v3.1.3 - 17 Oct 2023 -- 2.42.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 branch-3.0] dpdk: Use DPDK 21.11.5 release for OVS 3.0.
Update the CI and docs to use DPDK 21.11.5. Signed-off-by: Kevin Traynor Acked-by: Simon Horman Reviewed-by: David Marchand --- .ci/linux-build.sh | 2 +- Documentation/faq/releases.rst | 4 ++-- Documentation/intro/install/dpdk.rst | 8 NEWS | 2 ++ 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index a0d780101..2df466b0c 100755 --- a/.ci/linux-build.sh +++ b/.ci/linux-build.sh @@ -229,5 +229,5 @@ fi if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then if [ -z "$DPDK_VER" ]; then -DPDK_VER="21.11.2" +DPDK_VER="21.11.5" fi install_dpdk $DPDK_VER diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst index b19d9f556..3825e2695 100644 --- a/Documentation/faq/releases.rst +++ b/Documentation/faq/releases.rst @@ -214,6 +214,6 @@ Q: What DPDK version does each Open vSwitch release work with? 2.15.x 20.11.6 2.16.x 20.11.6 -2.17.x 21.11.2 -3.0.x21.11.2 +2.17.x 21.11.5 +3.0.x21.11.5 diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst index a284e6851..559e8eb1f 100644 --- a/Documentation/intro/install/dpdk.rst +++ b/Documentation/intro/install/dpdk.rst @@ -43,5 +43,5 @@ In addition to the requirements described in :doc:`general`, building Open vSwitch with DPDK will require the following: -- DPDK 21.11.2 +- DPDK 21.11.5 - A `DPDK supported NIC`_ @@ -74,7 +74,7 @@ Install DPDK $ cd /usr/src/ - $ wget https://fast.dpdk.org/rel/dpdk-21.11.2.tar.xz - $ tar xf dpdk-21.11.2.tar.xz - $ export DPDK_DIR=/usr/src/dpdk-stable-21.11.2 + $ wget https://fast.dpdk.org/rel/dpdk-21.11.5.tar.xz + $ tar xf dpdk-21.11.5.tar.xz + $ export DPDK_DIR=/usr/src/dpdk-stable-21.11.5 $ cd $DPDK_DIR diff --git a/NEWS b/NEWS index 33b982886..23327927b 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ v3.0.6 - xx xxx + - DPDK: + * OVS validated with DPDK 21.11.5 v3.0.5 - 17 Oct 2023 -- 2.42.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 branch-2.17] dpdk: Use DPDK 21.11.5 release for OVS 2.17.
Update the CI and docs to use DPDK 21.11.5. Signed-off-by: Kevin Traynor Acked-by: Simon Horman Reviewed-by: David Marchand --- .ci/linux-build.sh | 2 +- Documentation/faq/releases.rst | 2 +- Documentation/intro/install/dpdk.rst | 8 NEWS | 2 ++ 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index f5021e1a8..9464ea49c 100755 --- a/.ci/linux-build.sh +++ b/.ci/linux-build.sh @@ -221,5 +221,5 @@ fi if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then if [ -z "$DPDK_VER" ]; then -DPDK_VER="21.11.2" +DPDK_VER="21.11.5" fi install_dpdk $DPDK_VER diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst index 49895c595..0e0c589a3 100644 --- a/Documentation/faq/releases.rst +++ b/Documentation/faq/releases.rst @@ -211,5 +211,5 @@ Q: What DPDK version does each Open vSwitch release work with? 2.15.x 20.11.6 2.16.x 20.11.6 -2.17.x 21.11.2 +2.17.x 21.11.5 diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst index a284e6851..559e8eb1f 100644 --- a/Documentation/intro/install/dpdk.rst +++ b/Documentation/intro/install/dpdk.rst @@ -43,5 +43,5 @@ In addition to the requirements described in :doc:`general`, building Open vSwitch with DPDK will require the following: -- DPDK 21.11.2 +- DPDK 21.11.5 - A `DPDK supported NIC`_ @@ -74,7 +74,7 @@ Install DPDK $ cd /usr/src/ - $ wget https://fast.dpdk.org/rel/dpdk-21.11.2.tar.xz - $ tar xf dpdk-21.11.2.tar.xz - $ export DPDK_DIR=/usr/src/dpdk-stable-21.11.2 + $ wget https://fast.dpdk.org/rel/dpdk-21.11.5.tar.xz + $ tar xf dpdk-21.11.5.tar.xz + $ export DPDK_DIR=/usr/src/dpdk-stable-21.11.5 $ cd $DPDK_DIR diff --git a/NEWS b/NEWS index 7d4a8c081..ceecc25da 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ v2.17.9 - xx xxx - + - DPDK: + * OVS validated with DPDK 21.11.5 v2.17.8 - 17 Oct 2023 -- 2.42.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6 6/6] tests: Do not use zone 0 for CT limit system test.
On 11/2/23 13:00, Ales Musil wrote: > The zone 0 is default system zone, do not use this > zone for the test because it might contain some > entries already which could cause flakiness during > the check. > > In order to still have the zone 0 parsing coverage > add simple unit tests for dpctl. > > Signed-off-by: Ales Musil > --- > v6: Rebase on top of current master. > --- > tests/dpctl.at | 10 +-- > tests/system-traffic.at | 59 - > 2 files changed, 37 insertions(+), 32 deletions(-) > > diff --git a/tests/dpctl.at b/tests/dpctl.at > index d2f1046f8..bc84b196b 100644 > --- a/tests/dpctl.at > +++ b/tests/dpctl.at > @@ -136,7 +136,7 @@ AT_CHECK([ovs-appctl dpctl/del-dp dummy@br0]) > OVS_VSWITCHD_STOP > AT_CLEANUP > > -AT_SETUP([dpctl - ct-get-limits ct-del-limits]) > +AT_SETUP([dpctl - ct-set-limits ct-get-limits ct-del-limits]) > OVS_VSWITCHD_START > AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [default limit=0 > ]) > @@ -149,5 +149,11 @@ AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=x], [2], > [], > ovs-appctl: ovs-vswitchd: server returned an error > ]) > AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=]) > +AT_CHECK([ovs-appctl dpctl/ct-set-limits zone=0,limit=0]) > +AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=0], [0], [default limit=0 > +zone=0,limit=0,count=0 > +]) > +AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=0]) > + > OVS_VSWITCHD_STOP > -AT_CLEANUP > \ No newline at end of file > +AT_CLEANUP Not sure what nappened here. Is the newline getting added/removed? > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index a1d26a06c..375a8aa2f 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -5124,20 +5124,20 @@ ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") > AT_DATA([flows.txt], [dnl > priority=1,action=drop > priority=10,arp,action=normal > -priority=100,in_port=1,udp,action=ct(commit),2 > +priority=100,in_port=1,udp,action=ct(zone=1,commit),2 > priority=100,in_port=2,udp,action=ct(zone=3,commit),1 > ]) > > AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) > > -AT_CHECK([ovs-appctl dpctl/ct-set-limits default=10 zone=0,limit=5 > zone=1,limit=15 zone=2,limit=3 zone=3,limit=3]) > -AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=1,2,4]) > -AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=0,1,2,3], [],[dnl > +AT_CHECK([ovs-appctl dpctl/ct-set-limits default=10 zone=1,limit=5 > zone=2,limit=3 zone=3,limit=3 zone=4,limit=15]) > +AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=2,4,5]) > +AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=1,2,3,4], [],[dnl > default limit=10 > -zone=0,limit=5,count=0 > -zone=1,limit=10,count=0 > +zone=1,limit=5,count=0 > zone=2,limit=10,count=0 > zone=3,limit=3,count=0 > +zone=4,limit=10,count=0 > ]) > > dnl Test UDP from port 1 > @@ -5151,10 +5151,9 @@ AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 > "in_port=1 packet=5054000a5 > AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 > packet=5054000a505400090800451c0011a4cd0a0101010a010102000100090008 > actions=resubmit(,0)"]) > AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 > packet=5054000a505400090800451c0011a4cd0a0101010a0101020001000a0008 > actions=resubmit(,0)"]) > > -AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=0,1,2,3,4,5], [0], [dnl > +AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=1,2,3,4,5], [0], [dnl > default limit=10 > -zone=0,limit=5,count=5 > -zone=1,limit=10,count=0 > +zone=1,limit=5,count=5 > zone=2,limit=10,count=0 > zone=3,limit=3,count=0 > zone=4,limit=10,count=0 > @@ -5164,16 +5163,16 @@ zone=5,limit=10,count=0 > dnl Test ct-get-limits for all zones > AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl > default limit=10 > -zone=0,limit=5,count=5 > +zone=1,limit=5,count=5 > zone=3,limit=3,count=0 > ]) > > AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.1," | > sort ], [0], [dnl > -udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1) > -udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=3),reply=(src=10.1.1.2,dst=10.1.1.1,sport=3,dport=1) > -udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=4),reply=(src=10.1.1.2,dst=10.1.1.1,sport=4,dport=1) > -udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=5),reply=(src=10.1.1.2,dst=10.1.1.1,sport=5,dport=1) > -udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=6),reply=(src=10.1.1.2,dst=10.1.1.1,sport=6,dport=1) > +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),zone=1 > +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=3),reply=(src=10.1.1.2,dst=10.1.1.1,sport=3,dport=1),zone=1 > +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=4),reply=(src=10.1.1.2,dst=10.1.1.1,sport=4,dport=1),zone=1 > +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=5),reply=(src=10.1.1.2,dst=10.1.1.1,sport=5,dport=1),zone=1 > +udp,orig=(src=10.1.1.1
Re: [ovs-dev] [PATCH v6 5/6] ct-dpif: Enforce CT zone limit protection.
On 11/2/23 13:00, Ales Musil wrote:
> Make sure that if any zone limit was set via DB
> all zones are forced to be set there also. This
> is done by tracking which datapath has zone limit
> protection and it is reflected in the dpctl command.
>
> If the datapath is protected the dpctl command will
> return permission error.
>
> Signed-off-by: Ales Musil
> ---
> v6: Rebase on top of current master.
> Address comments from Ilya:
> - Drop the log message about protection.
> - Make the dpctl error message more user-friendly.
> - Do not ignore error messages in the system-test.
> v5: Rebase on top of current master.
> Address comments from Ilya:
> - Add more user friendly error message to the dpctl.
> - Fix style related problems.
> v4: Rebase on top of current master.
> Make the protection datapath wide.
> ---
> lib/ct-dpif.c | 25 +
> lib/ct-dpif.h | 2 ++
> lib/dpctl.c| 14
> ofproto/ofproto-dpif.c | 13 +++
> ofproto/ofproto-provider.h | 5 +
> ofproto/ofproto.c | 11 +
> ofproto/ofproto.h | 2 ++
> tests/ofproto-macros.at| 4 ++--
> tests/system-traffic.at| 46 ++
> vswitchd/bridge.c | 7 ++
> 10 files changed, 127 insertions(+), 2 deletions(-)
>
> diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
> index 2ee045164..5115c886b 100644
> --- a/lib/ct-dpif.c
> +++ b/lib/ct-dpif.c
> @@ -23,6 +23,7 @@
> #include "openvswitch/ofp-ct.h"
> #include "openvswitch/ofp-parse.h"
> #include "openvswitch/vlog.h"
> +#include "sset.h"
>
> VLOG_DEFINE_THIS_MODULE(ct_dpif);
>
> @@ -32,6 +33,10 @@ struct flags {
> const char *name;
> };
>
> +/* Protection for CT zone limit per datapath. */
> +static struct sset ct_limit_protection =
> +SSET_INITIALIZER(&ct_limit_protection);
> +
> static void ct_dpif_format_counters(struct ds *,
> const struct ct_dpif_counters *);
> static void ct_dpif_format_timestamp(struct ds *,
> @@ -1064,3 +1069,23 @@ ct_dpif_get_features(struct dpif *dpif, enum
> ct_features *features)
> ? dpif->dpif_class->ct_get_features(dpif, features)
> : EOPNOTSUPP);
> }
> +
> +void
> +ct_dpif_set_zone_limit_protection(struct dpif *dpif, bool protected)
> +{
> +if (sset_contains(&ct_limit_protection, dpif->full_name) == protected) {
> +return;
> +}
> +
> +if (protected) {
> +sset_add(&ct_limit_protection, dpif->full_name);
> +} else {
> +sset_find_and_delete(&ct_limit_protection, dpif->full_name);
> +}
> +}
> +
> +bool
> +ct_dpif_is_zone_limit_protected(struct dpif *dpif)
> +{
> +return sset_contains(&ct_limit_protection, dpif->full_name);
> +}
> diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
> index c8a7c155e..c3786d5ae 100644
> --- a/lib/ct-dpif.h
> +++ b/lib/ct-dpif.h
> @@ -350,5 +350,7 @@ int ct_dpif_get_timeout_policy_name(struct dpif *dpif,
> uint32_t tp_id,
> uint16_t dl_type, uint8_t nw_proto,
> char **tp_name, bool *is_generic);
> int ct_dpif_get_features(struct dpif *dpif, enum ct_features *features);
> +void ct_dpif_set_zone_limit_protection(struct dpif *, bool protected);
> +bool ct_dpif_is_zone_limit_protected(struct dpif *);
>
> #endif /* CT_DPIF_H */
> diff --git a/lib/dpctl.c b/lib/dpctl.c
> index a8c654747..2a1aac5e5 100644
> --- a/lib/dpctl.c
> +++ b/lib/dpctl.c
> @@ -2234,6 +2234,13 @@ dpctl_ct_set_limits(int argc, const char *argv[],
> ct_dpif_push_zone_limit(&zone_limits, zone, limit, 0);
> }
>
> +if (ct_dpif_is_zone_limit_protected(dpif)) {
> +ds_put_cstr(&ds, "the zone limits are set via database, "
> + "use 'ovs-vsctl set-zone-limit <...>' instead.");
> +error = EPERM;
> +goto error;
> +}
> +
> error = ct_dpif_set_limits(dpif, &zone_limits);
> if (!error) {
> ct_dpif_free_zone_limits(&zone_limits);
> @@ -2310,6 +2317,13 @@ dpctl_ct_del_limits(int argc, const char *argv[],
> }
> }
>
> +if (ct_dpif_is_zone_limit_protected(dpif)) {
> +ds_put_cstr(&ds, "the zone limits are set via database, "
> + "use 'ovs-vsctl del-zone-limit <...>' instead.");
> +error = EPERM;
> +goto error;
> +}
> +
> error = ct_dpif_del_limits(dpif, &zone_limits);
> if (!error) {
> goto out;
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 6a931a806..4cc8e2807 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -5663,6 +5663,18 @@ ct_zone_limits_commit(struct dpif_backer *backer)
> }
> }
>
> +static void
> +ct_zone_limit_protection_update(const char *datapath_type, bool protected)
> +{
> +struct dpif_backer *backer = shash_find_data(&all_dpif_backers,
> +
Re: [ovs-dev] [PATCH v6 3/6] ovs-vsctl: Add limit to CT zone.
On 11/2/23 13:00, Ales Musil wrote:
> Add limit to the CT zone DB table with ovs-vsctl
> helper methods. The limit has two special values
> besides any number, 0 is unlimited and empty limit
> is to leave the value untouched in the datapath.
>
> This is preparation step and the value is not yet
> propagated to the datapath.
>
> Signed-off-by: Ales Musil
> ---
> v6: Rebase on top of current master.
> Address comments from Ilya:
> - Update the semantics and documentation of the set command.
> v5: Rebase on top of current master.
> Address comments from Ilya:
> - Use only single command for setting zone and default limit.
> - Correct the errors in the man page.
> - Use references for the column description.
> v4: Rebase on top of current master.
> Address comments from Ilya:
> - Make sure that the NEWS is clear on what has been added.
> - Make the usage of --may-exist and --if-exists more intuitive for the
> new commands.
> - Some cosmetics.
> Add command and column for default limit.
> ---
> NEWS | 5 ++
> tests/ovs-vsctl.at | 88 +++-
> utilities/ovs-vsctl.8.in | 31 +++--
> utilities/ovs-vsctl.c | 133 +++--
> vswitchd/vswitch.ovsschema | 14 +++-
> vswitchd/vswitch.xml | 13
> 6 files changed, 268 insertions(+), 16 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 7bc27b687..61b48ff12 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -9,6 +9,11 @@ Post-v3.2.0
> - ovs-appctl:
> * Added support removal of default CT zone limit, e.g.
> "ovs-appctl dpctl/ct-del-limits default".
> + - ovs-vsctl:
> + * New commands 'set-zone-limit', 'del-zone-limit' and 'list-zone-limit'
> + to manage the maximum number of connections in conntrack zones via
> + a new 'limit' column in the 'CT_Zone' database table and
> + 'ct_zone_default_limit' column in the 'Datapath' table.
>
>
> v3.2.0 - 17 Aug 2023
> diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
> index a368bff6e..f88e986db 100644
> --- a/tests/ovs-vsctl.at
> +++ b/tests/ovs-vsctl.at
> @@ -975,6 +975,67 @@ AT_CHECK(
>[0], [stdout])
> AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:10, Timeout
> Policies: system default
> ])
> +AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-tp netdev zone=10])])
> +
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])])
> +
> +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev zone=1 limit=1])])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
> +Zone: 1, Limit: 1
> +])
> +
> +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev zone=1 limit=5])])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
> +Zone: 1, Limit: 5
> +])
> +
> +AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev zone=1])])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])])
> +
> +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev zone=10 limit=5])])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
> +Zone: 10, Limit: 5
> +])
> +
> +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=10 icmp_first=1
> icmp_reply=2])])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [dnl
> +Zone:10, Timeout Policies: icmp_first=1 icmp_reply=2
> +])
> +
> +AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev zone=10])])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [dnl
> +Zone:10, Timeout Policies: icmp_first=1 icmp_reply=2
> +])
> +
> +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev zone=10 limit=5])])
> +AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=10])])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
> +Zone: 10, Limit: 5
> +])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [dnl
> +Zone:10, Timeout Policies: system default
> +])
> +
> +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev default limit=5])])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
> +Default limit: 5
> +Zone: 10, Limit: 5
> +])
> +
> +AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev default limit=10])])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
> +Default limit: 10
> +Zone: 10, Limit: 5
> +])
> +
> +AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev default])])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-limit netdev])], [0], [dnl
> +Zone: 10, Limit: 5
> +])
> +
> +AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-limit netdev default])])
> +
>
> AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0
> 'capabilities={recirc=true}' -- set Open_vSwitch . datapaths:"system"=@m])],
> [0], [stdout])
> AT_CHECK([RUN_OVS_VSCTL([list-dp-cap system])], [0], [recirc=true
> @@ -1113,16 +1174,39 @@ AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdevxx zone=1
> icmp_first=1 icmp_reply=2])
> ])
> AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2
> icmp_reply=3])])
> AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2
> icm
Re: [ovs-dev] [PATCH v6 2/6] dpctl: Allow the default CT zone limit to de deleted.
On 11/2/23 13:00, Ales Musil wrote:
> Add optional argument to dpctl ct-del-limits called
> "default", which allows to remove the default limit
> making it effectively system default.
>
> Signed-off-by: Ales Musil
> ---
> v6: Rebase on top of current master.
> Address comments from Ilya:
> - Adjust the log message so it doesn't report anything for default zone.
> v5: Rebase on top of current master.
> Address comments from Ilya:
> - Correct the NEWS entry.
> - Fix style related problems.
> ---
> NEWS| 3 +++
> lib/conntrack.c | 13 +++--
> lib/dpctl.c | 21 +++--
> tests/system-traffic.at | 26 ++
> 4 files changed, 51 insertions(+), 12 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 6b45492f1..7bc27b687 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -6,6 +6,9 @@ Post-v3.2.0
> from older version is supported but it may trigger more leader
> elections
> during the process, and error logs complaining unrecognized fields may
> be observed on old nodes.
> + - ovs-appctl:
> + * Added support removal of default CT zone limit, e.g.
Added support for ...
> + "ovs-appctl dpctl/ct-del-limits default".
>
>
> v3.2.0 - 17 Aug 2023
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 31f00a127..b533dd3df 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -404,13 +404,14 @@ zone_limit_delete(struct conntrack *ct, int32_t zone)
> struct zone_limit *zl = zone_limit_lookup_protected(ct, zone);
> if (zl) {
> zone_limit_clean(ct, zl);
> -ovs_mutex_unlock(&ct->ct_lock);
> -VLOG_INFO("Deleted zone limit for zone %d", zone);
> -} else {
> -ovs_mutex_unlock(&ct->ct_lock);
> -VLOG_INFO("Attempted delete of non-existent zone limit: zone %d",
> - zone);
> }
> +
> +if (zone != DEFAULT_ZONE) {
> +VLOG_INFO(zl ? "Deleted zone limit for zone %d" : "Attempted delete"
> + " of non-existent zone limit: zone %d", zone);
Nit: Might be better to not split the string. E.g.:
VLOG_INFO(zl ? "Deleted zone limit for zone %d"
: "Attempted delete of non-existent zone limit: zone %d",
zone);
> +}
> +
> +ovs_mutex_unlock(&ct->ct_lock);
> return 0;
> }
>
> diff --git a/lib/dpctl.c b/lib/dpctl.c
> index 76f21a530..a8c654747 100644
> --- a/lib/dpctl.c
> +++ b/lib/dpctl.c
> @@ -2291,14 +2291,23 @@ dpctl_ct_del_limits(int argc, const char *argv[],
> int i = dp_arg_exists(argc, argv) ? 2 : 1;
> struct ovs_list zone_limits = OVS_LIST_INITIALIZER(&zone_limits);
>
> -error = opt_dpif_open(argc, argv, dpctl_p, 3, &dpif);
> +error = opt_dpif_open(argc, argv, dpctl_p, 4, &dpif);
> if (error) {
> return error;
> }
>
> -error = parse_ct_limit_zones(argv[i], &zone_limits, &ds);
> -if (error) {
> -goto error;
> +/* Parse default limit. */
> +if (!strcmp(argv[i], "default")) {
> +ct_dpif_push_zone_limit(&zone_limits, OVS_ZONE_LIMIT_DEFAULT_ZONE,
> +0, 0);
> +i++;
> +}
> +
> +if (argc > i) {
> +error = parse_ct_limit_zones(argv[i], &zone_limits, &ds);
> +if (error) {
> +goto error;
> +}
> }
>
> error = ct_dpif_del_limits(dpif, &zone_limits);
> @@ -3031,8 +3040,8 @@ static const struct dpctl_command all_commands[] = {
> { "ct-get-tcp-seq-chk", "[dp]", 0, 1, dpctl_ct_get_tcp_seq_chk, DP_RO },
> { "ct-set-limits", "[dp] [default=L] [zone=N,limit=L]...", 1, INT_MAX,
> dpctl_ct_set_limits, DP_RO },
> -{ "ct-del-limits", "[dp] zone=N1[,N2]...", 1, 2, dpctl_ct_del_limits,
> -DP_RO },
> +{ "ct-del-limits", "[dp] [default] [zone=N1[,N2]...]", 1, 3,
> +dpctl_ct_del_limits, DP_RO },
> { "ct-get-limits", "[dp] [zone=N1[,N2]...]", 0, 2, dpctl_ct_get_limits,
> DP_RO },
> { "ct-get-sweep-interval", "[dp]", 0, 1, dpctl_ct_get_sweep, DP_RO },
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 7ea450202..b6c8d7faf 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -5195,6 +5195,32 @@
> udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=3),reply=(src=10.1.1.4,dst=10.
>
> udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=4),reply=(src=10.1.1.4,dst=10.1.1.3,sport=4,dport=1),zone=3
> ])
>
> +dnl Test ct-del-limits for default zone.
> +
> +AT_CHECK([ovs-appctl dpctl/ct-set-limits default=15 zone=4,limit=4])
> +AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=4], [0], [dnl
> +default limit=15
> +zone=4,limit=4,count=0
> +])
> +
> +AT_CHECK([ovs-appctl dpctl/ct-del-limits default])
> +AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=4], [0], [dnl
> +default limit=0
> +zone=4,limit=4,count=0
> +])
> +
> +AT_CHECK([ovs-appctl dpctl/ct-set-limits default=15])
> +AT_CHECK([ovs-appctl dpctl/ct-g
Re: [ovs-dev] [PATCH v6 1/6] ct-dpif: Handle default zone limit the same way as other limits.
On 11/2/23 13:00, Ales Musil wrote:
> Internally handle default CT zone limit as other limits that
> can be passed via the list with special value -1. Currently,
> the -1 is treated by both datapaths as default, add static
> asserts to make sure that this remains the case in the future.
> This allows us to easily delete the default zone limit.
>
> Signed-off-by: Ales Musil
> ---
> v6: Rebase on top of current master.
> Address comments from Ilya:
> - Add assert to conntrack.h for the zone numbers.
> - Some minot cosmetic changes.
> v5: Rebase on top of current master.
> Address comments from Ilya:
> - Fix some typos.
> - Use OVS_ZONE_LIMIT_DEFAULT_ZONE instead of special constant.
> - Do not relay on DEFAULT_ZONE being -1 for the limit list.
> - Fix wrong netlink message.
> ---
> lib/conntrack.c | 2 +-
> lib/conntrack.h | 7 +--
> lib/ct-dpif.c | 28 +++-
> lib/ct-dpif.h | 14 ++
> lib/dpctl.c | 15 ---
> lib/dpif-netdev.c | 21 ++---
> lib/dpif-netlink.c | 29 ++---
> lib/dpif-provider.h | 24 +++-
> 8 files changed, 58 insertions(+), 82 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 47a443fba..31f00a127 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -398,7 +398,7 @@ zone_limit_clean(struct conntrack *ct, struct zone_limit
> *zl)
> }
>
> int
> -zone_limit_delete(struct conntrack *ct, uint16_t zone)
> +zone_limit_delete(struct conntrack *ct, int32_t zone)
> {
> ovs_mutex_lock(&ct->ct_lock);
> struct zone_limit *zl = zone_limit_lookup_protected(ct, zone);
> diff --git a/lib/conntrack.h b/lib/conntrack.h
> index 57d5159b6..18c182f85 100644
> --- a/lib/conntrack.h
> +++ b/lib/conntrack.h
> @@ -122,11 +122,14 @@ struct timeout_policy {
>
> enum {
> INVALID_ZONE = -2,
> -DEFAULT_ZONE = -1, /* Default zone for zone limit management. */
> +DEFAULT_ZONE = OVS_ZONE_LIMIT_DEFAULT_ZONE, /* Default zone for zone
> + * limit management. */
> MIN_ZONE = 0,
> MAX_ZONE = 0x,
> };
>
> +BUILD_ASSERT_DECL(DEFAULT_ZONE > INVALID_ZONE && DEFAULT_ZONE < MIN_ZONE);
> +
> struct ct_dpif_entry;
> struct ct_dpif_tuple;
>
> @@ -154,6 +157,6 @@ struct ipf *conntrack_ipf_ctx(struct conntrack *ct);
> struct conntrack_zone_limit zone_limit_get(struct conntrack *ct,
> int32_t zone);
> int zone_limit_update(struct conntrack *ct, int32_t zone, uint32_t limit);
> -int zone_limit_delete(struct conntrack *ct, uint16_t zone);
> +int zone_limit_delete(struct conntrack *ct, int32_t zone);
>
> #endif /* conntrack.h */
> diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
> index f59c6e560..2ee045164 100644
> --- a/lib/ct-dpif.c
> +++ b/lib/ct-dpif.c
> @@ -398,23 +398,19 @@ ct_dpif_get_tcp_seq_chk(struct dpif *dpif, bool
> *enabled)
> }
>
> int
> -ct_dpif_set_limits(struct dpif *dpif, const uint32_t *default_limit,
> - const struct ovs_list *zone_limits)
> +ct_dpif_set_limits(struct dpif *dpif, const struct ovs_list *zone_limits)
> {
> return (dpif->dpif_class->ct_set_limits
> -? dpif->dpif_class->ct_set_limits(dpif, default_limit,
> - zone_limits)
> +? dpif->dpif_class->ct_set_limits(dpif, zone_limits)
> : EOPNOTSUPP);
> }
>
> int
> -ct_dpif_get_limits(struct dpif *dpif, uint32_t *default_limit,
> - const struct ovs_list *zone_limits_in,
> +ct_dpif_get_limits(struct dpif *dpif, const struct ovs_list *zone_limits_in,
> struct ovs_list *zone_limits_out)
> {
> return (dpif->dpif_class->ct_get_limits
> -? dpif->dpif_class->ct_get_limits(dpif, default_limit,
> - zone_limits_in,
> +? dpif->dpif_class->ct_get_limits(dpif, zone_limits_in,
>zone_limits_out)
> : EOPNOTSUPP);
> }
> @@ -854,7 +850,7 @@ ct_dpif_format_tcp_stat(struct ds * ds, int tcp_state,
> int conn_per_state)
>
>
> void
> -ct_dpif_push_zone_limit(struct ovs_list *zone_limits, uint16_t zone,
> +ct_dpif_push_zone_limit(struct ovs_list *zone_limits, int32_t zone,
> uint32_t limit, uint32_t count)
> {
> struct ct_dpif_zone_limit *zone_limit = xmalloc(sizeof *zone_limit);
> @@ -928,15 +924,21 @@ error:
> }
>
> void
> -ct_dpif_format_zone_limits(uint32_t default_limit,
> - const struct ovs_list *zone_limits, struct ds *ds)
> +ct_dpif_format_zone_limits(const struct ovs_list *zone_limits, struct ds *ds)
> {
> struct ct_dpif_zone_limit *zone_limit;
>
> -ds_put_format(ds, "default limit=%"PRIu32, default_limit);
> +LIST_FOR_EACH (zone_limit, node, zone_limits) {
> +if (
[ovs-dev] 答复: [PATCH v8] userspace: Support vxlan and geneve tso.
On Fri, Nov 3, 2023 at 12:01 AM Dexia Li via dev
wrote:
>
> For userspace datapath, this patch provides vxlan and geneve tunnel tso.
> Only support userspace vxlan or geneve tunnel, meanwhile support
> tunnel outter and inner csum offload. If netdev do not support offload
> features, there is a software fallback.If netdev do not support vxlan
> and geneve tso,packets will drop. Front-end devices can close offload
> features by ethtool also.
>
> Signed-off-by: Dexia Li
Hello Dexia,
Thanks for getting back to this patch, I see that it's now passing all the unit
tests, that's great! I've left comments inline with the code.
-M
Hi Mike,
Thanks for your comments. I've gone through the comments quickly and
will check them in next few weeks, busy recently.
Best regards,
Dexia
> ---
> lib/dp-packet.c | 41 +++-
> lib/dp-packet.h | 216
> lib/dpif-netdev.c | 4 +-
> lib/flow.c | 2 +-
> lib/netdev-dpdk.c | 88 ++--
> lib/netdev-dummy.c | 2 +-
> lib/netdev-native-tnl.c | 106 ++--
> lib/netdev-provider.h | 4 +
> lib/netdev.c| 35 +--
> lib/packets.c | 12 +--
> lib/packets.h | 6 +-
> tests/dpif-netdev.at| 4 +-
> 12 files changed, 461 insertions(+), 59 deletions(-)
>
> diff --git a/lib/dp-packet.c b/lib/dp-packet.c index
> ed004c3b9..b5013da9f 100644
> --- a/lib/dp-packet.c
> +++ b/lib/dp-packet.c
> @@ -543,16 +543,47 @@ dp_packet_compare_offsets(struct dp_packet *b1, struct
> dp_packet *b2,
> return true;
> }
>
> +void
> +dp_packet_tnl_outer_ol_send_prepare(struct dp_packet *p,
> +uint64_t flags) {
> +if (dp_packet_hwol_is_outer_ipv4_cksum(p)) {
> +if (!(flags & NETDEV_TX_OFFLOAD_OUTER_IP_CKSUM)) {
> +dp_packet_ip_set_header_csum(p, false);
> +dp_packet_ol_set_ip_csum_good(p);
> +dp_packet_hwol_reset_outer_ipv4_csum(p);
> +}
> +}
> +
> +if (!dp_packet_hwol_is_outer_UDP_cksum(p)) {
> +return;
> +}
> +
> +if (!(flags & NETDEV_TX_OFFLOAD_OUTER_UDP_CKSUM)) {
> +packet_udp_complete_csum(p, false);
> +dp_packet_ol_set_l4_csum_good(p);
> +dp_packet_hwol_reset_outer_udp_csum(p);
> +}
> +}
> +
> /* Checks if the packet 'p' is compatible with netdev_ol_flags 'flags'
> * and if not, updates the packet with the software fall back. */
> void dp_packet_ol_send_prepare(struct dp_packet *p, uint64_t flags)
> {
> +bool tnl_inner = false;
> +
> +if (dp_packet_hwol_is_tunnel_geneve(p) ||
> +dp_packet_hwol_is_tunnel_vxlan(p)) {
> +dp_packet_tnl_outer_ol_send_prepare(p, flags);
> +tnl_inner = true;
> +}
> +
> if (dp_packet_hwol_tx_ip_csum(p)) {
> if (dp_packet_ip_checksum_good(p)) {
> dp_packet_hwol_reset_tx_ip_csum(p);
> } else if (!(flags & NETDEV_TX_OFFLOAD_IPV4_CKSUM)) {
> -dp_packet_ip_set_header_csum(p);
> +dp_packet_ip_set_header_csum(p, tnl_inner);
> dp_packet_ol_set_ip_csum_good(p);
> dp_packet_hwol_reset_tx_ip_csum(p);
> }
> @@ -562,24 +593,24 @@ dp_packet_ol_send_prepare(struct dp_packet *p, uint64_t
> flags)
> return;
> }
>
> -if (dp_packet_l4_checksum_good(p)) {
> +if (dp_packet_l4_checksum_good(p) && (!tnl_inner)) {
> dp_packet_hwol_reset_tx_l4_csum(p);
> return;
> }
>
> if (dp_packet_hwol_l4_is_tcp(p)
> && !(flags & NETDEV_TX_OFFLOAD_TCP_CKSUM)) {
> -packet_tcp_complete_csum(p);
> +packet_tcp_complete_csum(p, tnl_inner);
> dp_packet_ol_set_l4_csum_good(p);
> dp_packet_hwol_reset_tx_l4_csum(p);
> } else if (dp_packet_hwol_l4_is_udp(p)
> && !(flags & NETDEV_TX_OFFLOAD_UDP_CKSUM)) {
> -packet_udp_complete_csum(p);
> +packet_udp_complete_csum(p, tnl_inner);
> dp_packet_ol_set_l4_csum_good(p);
> dp_packet_hwol_reset_tx_l4_csum(p);
> } else if (!(flags & NETDEV_TX_OFFLOAD_SCTP_CKSUM)
> && dp_packet_hwol_l4_is_sctp(p)) {
> -packet_sctp_complete_csum(p);
> +packet_sctp_complete_csum(p, tnl_inner);
> dp_packet_ol_set_l4_csum_good(p);
> dp_packet_hwol_reset_tx_l4_csum(p);
> }
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h index
> 70ddf8aa4..80c7ab961 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -86,22 +86,47 @@ enum dp_packet_offload_mask {
> DEF_OL_FLAG(DP_PACKET_OL_TX_SCTP_CKSUM, RTE_MBUF_F_TX_SCTP_CKSUM, 0x800),
> /* Offload IP checksum. */
> DEF_OL_FLAG(DP_PACKET_OL_TX_IP_CKSUM, RTE_MBUF_F_TX_IP_CKSUM,
> 0x1000),
> +/* Offload packet is tunnel GENEVE. */
> +DEF_OL_FLAG(DP_PACKET_OL_TX_TUNNEL_GENEVE,
> +RTE_MBUF_F_TX_TUNNEL_GENEVE, 0x2000),
> +/* Offload packet is tunnel VXLAN. *
Re: [ovs-dev] [PATCH v2 7/9] ci: Allow make check-dpdk to run more tests.
On 11/28/23 08:49, Eelco Chaudron wrote: > > > On 27 Nov 2023, at 19:14, Ilya Maximets wrote: > >> On 11/27/23 13:40, Eelco Chaudron wrote: >>> Install additional packages and drivers required by >>> make check-dpdk. >>> >>> Signed-off-by: Eelco Chaudron >>> --- >>> .ci/dpdk-build.sh|2 +- >>> .github/workflows/build-and-test.yml |2 +- >>> python/test_requirements.txt |1 + >>> 3 files changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/.ci/dpdk-build.sh b/.ci/dpdk-build.sh >>> index aa83e4464..d4c178ee0 100755 >>> --- a/.ci/dpdk-build.sh >>> +++ b/.ci/dpdk-build.sh >>> @@ -38,7 +38,7 @@ function build_dpdk() >>> # any DPDK driver. >>> # check-dpdk unit tests requires testpmd and some net/ driver. >>> DPDK_OPTS="$DPDK_OPTS -Denable_apps=test-pmd" >>> -enable_drivers="net/null,net/af_xdp,net/tap,net/virtio" >>> +enable_drivers="net/null,net/af_xdp,net/tap,net/virtio,net/pcap" >>> DPDK_OPTS="$DPDK_OPTS -Denable_drivers=$enable_drivers" >>> >>> # Install DPDK using prefix. >>> diff --git a/.github/workflows/build-and-test.yml >>> b/.github/workflows/build-and-test.yml >>> index e9a2714fb..1e92a0e2b 100644 >>> --- a/.github/workflows/build-and-test.yml >>> +++ b/.github/workflows/build-and-test.yml >>> @@ -5,7 +5,7 @@ on: [push, pull_request] >>> jobs: >>>build-dpdk: >>> env: >>> - dependencies: gcc libbpf-dev libnuma-dev ninja-build pkgconf >>> + dependencies: gcc libbpf-dev libnuma-dev libpcap-dev ninja-build >>> pkgconf >>>CC: gcc >>>DPDK_GIT: https://dpdk.org/git/dpdk-stable >>>DPDK_VER: 22.11.1 >>> diff --git a/python/test_requirements.txt b/python/test_requirements.txt >>> index c85ce41ad..5043c71e2 100644 >>> --- a/python/test_requirements.txt >>> +++ b/python/test_requirements.txt >>> @@ -2,4 +2,5 @@ netaddr >>> pyftpdlib >>> pyparsing >>> pytest >>> +scapy >> >> I'd vote against enabling scapy-based tests. They are mainly randomized >> autovalidator tests that we cannot meaningfully test in GHA anyway. The >> one that is about configuration is also questionable, it should not really >> need any real traffic. >> >> In addition, these tests consume too much CPU on a very resource-limited >> system like GHA. > > What about I keep this patch (with a more clear commit message), and add > OVS_CHECK_GITHUB_ACTION() for the scapy tests. This way we can still use the > build scripts for offline CI? OK. If installing scapy and libpcap doesn't take much time, should be fine. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 3/9] ci: Add make check-offloads to GitHub actions ci.
On 11/28/23 08:43, Eelco Chaudron wrote: > > > On 27 Nov 2023, at 19:04, Ilya Maximets wrote: > >> On 11/27/23 13:39, Eelco Chaudron wrote: >>> This patch also adds the 'CHECK_GITHUB_ACTION' macro to skip >>> tests that won't execute successfully through GitHub actions. >>> We could not use the -k !keyword option, as it can not be >>> combined with a range of tests. >>> >>> Signed-off-by: Eelco Chaudron >>> --- >>> .ci/linux-build.sh |2 +- >>> .github/workflows/build-and-test.yml |7 +++ >>> tests/system-common-macros.at|4 >>> tests/system-offloads-traffic.at |2 ++ >>> 4 files changed, 14 insertions(+), 1 deletion(-) >>> >>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh >>> index 4f2e36610..85788748f 100755 >>> --- a/.ci/linux-build.sh >>> +++ b/.ci/linux-build.sh >>> @@ -139,7 +139,7 @@ else >>> export DPDK_EAL_OPTIONS="--lcores 0@1,1@1,2@1" >>> fi >>> $run_as_root make $testsuite TESTSUITEFLAGS="$JOBS $TEST_RANGE" \ >>> - RECHECK=yes >>> + RECHECK=yes >>> GITHUB_ACTIONS=$GITHUB_ACTIONS >>> done >>> fi >>> >>> diff --git a/.github/workflows/build-and-test.yml >>> b/.github/workflows/build-and-test.yml >>> index 0b881ca91..586b0cdd9 100644 >>> --- a/.github/workflows/build-and-test.yml >>> +++ b/.github/workflows/build-and-test.yml >>> @@ -176,6 +176,13 @@ jobs: >>> testsuite:check-kernel >>> test_range: "100-" >>> >>> + - compiler: gcc >>> +testsuite:check-offloads >>> +test_range: "-100" >>> + - compiler: gcc >>> +testsuite:check-offloads >>> +test_range: "100-" >>> + >>> steps: >>> - name: checkout >>>uses: actions/checkout@v3 >>> diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at >>> index 0113aae8b..0620be0c7 100644 >>> --- a/tests/system-common-macros.at >>> +++ b/tests/system-common-macros.at >>> @@ -365,3 +365,7 @@ m4_define([OVS_CHECK_IPROUTE_ENCAP], >>> # OVS_CHECK_CT_CLEAR() >>> m4_define([OVS_CHECK_CT_CLEAR], >>> [AT_SKIP_IF([! grep -q "Datapath supports ct_clear action" >>> ovs-vswitchd.log])]) >>> + >>> +# OVS_CHECK_GITHUB_ACTION >>> +m4_define([OVS_CHECK_GITHUB_ACTION], >>> +[AT_SKIP_IF([test "$GITHUB_ACTIONS" = "true"])]) >> >> Can we use some pre-defined GHA env variable instead? >> Or are they not available? > > This is a pre-defined GH env. See above we re-export it as we run as root. Hmm. I got confused, because you're passing it directly to 'make' and not as part of 'run_as_root'. Maybe move it there, so it's obvious that the problem is the same as with PATH? > >>> diff --git a/tests/system-offloads-traffic.at >>> b/tests/system-offloads-traffic.at >>> index 0bedee753..6bd49a3ee 100644 >>> --- a/tests/system-offloads-traffic.at >>> +++ b/tests/system-offloads-traffic.at >>> @@ -192,6 +192,7 @@ AT_CLEANUP >>> AT_SETUP([offloads - check interface meter offloading - offloads >>> disabled]) >>> AT_KEYWORDS([dp-meter]) >>> AT_SKIP_IF([test $HAVE_NC = "no"]) >>> +OVS_CHECK_GITHUB_ACTION() >>> OVS_TRAFFIC_VSWITCHD_START() >>> >>> AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br0 'meter=1 pktps >>> bands=type=drop rate=1']) >>> @@ -240,6 +241,7 @@ AT_CLEANUP >>> >>> AT_SETUP([offloads - check interface meter offloading - offloads enabled]) >>> AT_KEYWORDS([offload-meter]) >>> +OVS_CHECK_GITHUB_ACTION() >>> CHECK_TC_INGRESS_PPS() >>> AT_SKIP_IF([test $HAVE_NC = "no"]) >>> OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . >>> other_config:hw-offload=true]) >>> >>> ___ >>> dev mailing list >>> d...@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 1/9] ci: Add make check-ovsdb-cluster tests to GitHub action ci.
On 11/28/23 08:39, Eelco Chaudron wrote:
>
>
> On 27 Nov 2023, at 18:58, Ilya Maximets wrote:
>
>> On 11/27/23 13:38, Eelco Chaudron wrote:
>>> Signed-off-by: Eelco Chaudron
>>> ---
>>> .ci/linux-build.sh |9 +
>>> .github/workflows/build-and-test.yml |3 +++
>>> 2 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>>> index aa2ecc505..e9e1e24b5 100755
>>> --- a/.ci/linux-build.sh
>>> +++ b/.ci/linux-build.sh
>>> @@ -6,6 +6,7 @@ set -x
>>> CFLAGS_FOR_OVS="-g -O2"
>>> SPARSE_FLAGS=""
>>> EXTRA_OPTS="--enable-Werror"
>>> +JOBS=${JOBS:-"-j4"}
>>
>> Is this used anywhere? Seems a little orthogonal to the
>> purpose of this set.
>
> Thought rather than adding -j4 in more places, having a seperate definition
> for it would allow it to be changed easily if we ever need to (and when using
> the script outside of GitHub actions).
>
> I can make it a seperate patch if that is preferred.
Would be better. The only thing that annoys me is that the definition
above allows passing JOBS via environment in order to override, but no
other variable is defined this way, and the functionality is not actually
used.
>
>>> function install_dpdk()
>>> {
>>> @@ -46,7 +47,7 @@ function build_ovs()
>>> configure_ovs $OPTS
>>> make selinux-policy
>>>
>>> -make -j4
>>> +make $JOBS
>>> }
>>>
>>> if [ "$DEB_PACKAGE" ]; then
>>> @@ -122,8 +123,8 @@ if [ "$TESTSUITE" = 'test' ]; then
>>> configure_ovs
>>>
>>> export DISTCHECK_CONFIGURE_FLAGS="$OPTS"
>>> -make distcheck -j4 CFLAGS="${CFLAGS_FOR_OVS}" \
>>> -TESTSUITEFLAGS=-j4 RECHECK=yes
>>> +make distcheck $JOBS CFLAGS="${CFLAGS_FOR_OVS}" \
>>> +TESTSUITEFLAGS=$JOBS RECHECK=yes
Nit: It's better to use ${JOBS} syntax. Not necessary though.
>>> else
>>> build_ovs
>>> for testsuite in $TESTSUITE; do
>>> @@ -134,7 +135,7 @@ else
>>> export DPDK_EAL_OPTIONS="--lcores 0@1,1@1,2@1"
>>> run_as_root="sudo -E PATH=$PATH"
>>> fi
>>> -$run_as_root make $testsuite TESTSUITEFLAGS=-j4 RECHECK=yes
>>> +$run_as_root make $testsuite TESTSUITEFLAGS=$JOBS RECHECK=yes
Ditto.
>>> done
>>> fi
>>>
>>> diff --git a/.github/workflows/build-and-test.yml
>>> b/.github/workflows/build-and-test.yml
>>> index 09654205e..5d441157c 100644
>>> --- a/.github/workflows/build-and-test.yml
>>> +++ b/.github/workflows/build-and-test.yml
>>> @@ -164,6 +164,9 @@ jobs:
>>> m32: m32
>>> opts: --disable-ssl
>>>
>>> + - compiler: gcc
>>> +testsuite:check-ovsdb-cluster
>>
>> FWIW, there is no need to run this testsuite as root.
>> But I'm not sure it worth changing the scripts in order
>> to avoid that.
>
> ACK, will keep it as is.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] ovn: add geneve PMTUD support
Introduce specif flows for E/W ICMPv{4,6} packets if tunnelled packets
do not fit path MTU. This patch enable PMTUD for East/West Geneve traffic.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2241711
Tested-by: Jaime Ruiz
Signed-off-by: Lorenzo Bianconi
---
NEWS| 1 +
controller/physical.c | 44 ++
northd/northd.c | 24 ++
northd/ovn-northd.8.xml | 8 +
tests/multinode.at | 69 +
tests/ovn-northd.at | 6
6 files changed, 152 insertions(+)
diff --git a/NEWS b/NEWS
index e10fb79dd..acb3b854f 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,7 @@ Post v23.09.0
connection method and doesn't require additional probing.
external_ids:ovn-openflow-probe-interval configuration option for
ovn-controller no longer matters and is ignored.
+ - Enable PMTU discovery on geneve tunnels for E/W traffic.
OVN v23.09.0 - 15 Sep 2023
--
diff --git a/controller/physical.c b/controller/physical.c
index ba88e1d8b..dc6130745 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -2446,6 +2446,50 @@ physical_run(struct physical_ctx *p_ctx,
&ofpacts, hc_uuid);
}
+/* Add specif flows for E/W ICMPv{4,6} packets if tunnelled packets do not
+ * fit path MTU.
+ */
+HMAP_FOR_EACH (tun, hmap_node, p_ctx->chassis_tunnels) {
+if (tun->type == GENEVE) {
+/* IPv4 */
+struct match match = MATCH_CATCHALL_INITIALIZER;
+match_set_in_port(&match, tun->ofport);
+match_set_dl_type(&match, htons(ETH_TYPE_IP));
+match_set_nw_proto(&match, IPPROTO_ICMP);
+match_set_icmp_type(&match, 3);
+match_set_icmp_code(&match, 4);
+
+ofpbuf_clear(&ofpacts);
+put_move(MFF_TUN_ID, 0, MFF_LOG_DATAPATH, 0, 24, &ofpacts);
+put_move(p_ctx->mff_ovn_geneve, 16, MFF_LOG_OUTPORT, 0, 16,
+ &ofpacts);
+put_move(p_ctx->mff_ovn_geneve, 0, MFF_LOG_INPORT, 0, 15,
+ &ofpacts);
+put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, &ofpacts);
+
+ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 120, 0, &match,
+&ofpacts, hc_uuid);
+/* IPv6 */
+match_init_catchall(&match);
+match_set_in_port(&match, tun->ofport);
+match_set_dl_type(&match, htons(ETH_TYPE_IPV6));
+match_set_nw_proto(&match, IPPROTO_ICMPV6);
+match_set_icmp_type(&match, 2);
+match_set_icmp_code(&match, 0);
+
+ofpbuf_clear(&ofpacts);
+put_move(MFF_TUN_ID, 0, MFF_LOG_DATAPATH, 0, 24, &ofpacts);
+put_move(p_ctx->mff_ovn_geneve, 16, MFF_LOG_OUTPORT, 0, 16,
+ &ofpacts);
+put_move(p_ctx->mff_ovn_geneve, 0, MFF_LOG_INPORT, 0, 15,
+ &ofpacts);
+put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, &ofpacts);
+
+ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 120, 0, &match,
+&ofpacts, hc_uuid);
+}
+}
+
/* Add VXLAN specific rules to transform port keys
* from 12 bits to 16 bits used elsewhere. */
HMAP_FOR_EACH (tun, hmap_node, p_ctx->chassis_tunnels) {
diff --git a/northd/northd.c b/northd/northd.c
index 29245d633..07dffb15a 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -12765,6 +12765,28 @@ build_lrouter_force_snat_flows(struct hmap *lflows,
struct ovn_datapath *od,
ds_destroy(&actions);
}
+/* Following flows are used to manage traffic redirected by the kernel
+ * (e.g. ICMP errors packets) that enter the cluster from the geneve ports
+ */
+static void
+build_lrouter_redirected_traffic_flows(
+struct ovn_port *op, struct hmap *lflows,
+struct ds *match, struct ds *actions)
+{
+ovs_assert(op->nbrp);
+if (!is_l3dgw_port(op)) {
+return;
+}
+
+ds_clear(match);
+ds_put_format(match, "inport == %s && !is_chassis_resident(%s)",
+ op->cr_port->json_key, op->cr_port->json_key);
+ds_clear(actions);
+ds_put_format(actions, "inport = %s; next;", op->json_key);
+ovn_lflow_add(lflows, op->od, S_ROUTER_IN_ADMISSION, 120,
+ ds_cstr(match), ds_cstr(actions));
+}
+
static void
build_lrouter_force_snat_flows_op(struct ovn_port *op,
struct hmap *lflows,
@@ -16168,6 +16190,8 @@ build_lswitch_and_lrouter_iterate_by_lrp(struct
ovn_port *op,
&lsi->match, &lsi->actions, lsi->meter_groups);
build_lrouter_force_snat_flows_op(op, lsi->lflows, &lsi->match,
&lsi->actions);
+build_lrouter_redirected_traffic_flows(op, lsi->lflows, &lsi->match,
+ &lsi->actions);
}
static void *
diff --git a/northd/ovn-north
Re: [ovs-dev] [PATCH v5] system-dpdk: Test with mlx5 devices.
On Wed, Nov 22, 2023 at 5:34 PM David Marchand wrote: > > The DPDK unit test only runs if vfio or igb_uio kernel modules are loaded: > on systems with only mlx5, this test is always skipped. > > Besides, the test tries to grab the first device listed by dpdk-devbind.py, > regardless of the PCI device status regarding kmod binding. > > Remove dependency on this DPDK script and use a minimal script that > reads PCI sysfs. > > This script is not perfect, as one can imagine PCI devices bound to > vfio-pci for virtual machines. > Plus, this script only tries to take over vfio-pci devices. mlx5 devices > can't be taken over blindly as it could mean losing connectivity to the > machine if the netdev was in use for this system. > > For those two reasons, add a new environment variable DPDK_PCI_ADDR for > testers to select the PCI device of their liking. > For consistency and grep, the temporary file PCI_ADDR is renamed > to DPDK_PCI_ADDR. > > Reviewed-by: Maxime Coquelin > Acked-by: Eelco Chaudron > Signed-off-by: David Marchand This patch can't be merged as is. I am preparing some fixes for the system-dpdk MTU tests that got merged since my v4. -- David Marchand ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 6/9] ci: Fix dpdk build cache key generation.
On 28 Nov 2023, at 9:06, David Marchand wrote:
> On Mon, Nov 27, 2023 at 3:26 PM Eelco Chaudron wrote:
>> On 27 Nov 2023, at 13:53, David Marchand wrote:
>>> On Mon, Nov 27, 2023 at 1:39 PM Eelco Chaudron wrote:
When new drivers are introduced, the cache key is not accurately computed.
Previously, the dpdk build process was integrated into the main Linux
build script, where specific lines were employed to generate the key.
Since it is now separated into two distinct files, this patch will
compute the key based on the content of these two files.
>>>
>>> I would rephrase this last sentence, as "two distinct files" and
>>> "these two files" are a bit unclear.
>>> Afaiu, the former refers to .ci/{linux,dpdk}-build.sh, while the
>>> latter refers to .ci/dpdk-{setup,build}.sh
>>>
>>> Otherwise the fix lgtm.
>>
>> So what about changing the commit message as follows:
>>
>>
>> Previously, the dpdk build process was integrated into the
>> .ci/{linux,dpdk}-build.sh scripts, where specific lines were employed to
>> generate the key. Since it is now separated into two distinct files,
>> .ci/dpdk-{setup,build}.sh, this patch will compute the key based on the
>> content of these two files.
>
> """
> When new drivers are introduced, the cache key is not accurately computed.
>
> Before the commit 1a1b3106d90e ("ci: Separate DPDK from OVS build."),
> the DPDK build
> process was integrated in .ci/linux-{setup,build}.sh scripts, where
> specific lines were
> employed to generate the key.
> Since it is now separated in .ci/dpdk-{setup,build}.sh, this patch
> computes the key based on the
> content of those dedicated scripts.
> """
>
> Deal?
Deal! Should we seal with a handshake?
⠀⣷⣄⣠⣾
⠀⣿⣿⣷⣄⠀⠀⠀⢀⣴⣿⣿⣿
⠀⣷⡄⠀⠙
⠀⡟⢀⣾⣿⣷⡖⠒⣀⣀⣤⣶⣾⣿⣷⣶⣤⣤⣴⣾⣆⠘⣿⣿⣿
⠀⣿⣿⣿⡿⠁⣾⣿⣿⣿⣧⣀⠙⠛⠛⠛⠋⣈⠻⢿⣿⣧⠈⢿⣿
⠀⣿⣿⣿⠁⣼⣿⠟⠻⠿⣿⣿⣿⣷⣶⣾⣿⠿⣷⣄⡉⠻⣧⠈⢿
⠀⠛⠛⠃⠀⠻⠋⣠⣶⠄⠙⠛⢻⣿⣿⣿⣷⣦⣈⠛⢿⣦⣄⠙⠻⠛⠁⠀⠀⠀
⠀⢠⣾⠟⠁⣠⣿⠇⠈⠛⣿⣿⣈⠙⠿⣷⣦⡈⠛⠛⠀⠀
⠀⠀⠁⢠⣾⡿⠁⣠⣾⠆⠸⠉⠻⣷⣤⡈⠙⠿⠃⠀⠀⠀
⠉⠀⣾⠟⢁⣴⡶⠀⣤⡀⠙⠟⠀⠀
⠀⠀⠀⠰⣿⠟⠁⠀⠛⠟
>
>
> --
> David Marchand
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 6/9] ci: Fix dpdk build cache key generation.
On Mon, Nov 27, 2023 at 3:26 PM Eelco Chaudron wrote:
> On 27 Nov 2023, at 13:53, David Marchand wrote:
> > On Mon, Nov 27, 2023 at 1:39 PM Eelco Chaudron wrote:
> >>
> >> When new drivers are introduced, the cache key is not accurately computed.
> >> Previously, the dpdk build process was integrated into the main Linux
> >> build script, where specific lines were employed to generate the key.
> >> Since it is now separated into two distinct files, this patch will
> >> compute the key based on the content of these two files.
> >
> > I would rephrase this last sentence, as "two distinct files" and
> > "these two files" are a bit unclear.
> > Afaiu, the former refers to .ci/{linux,dpdk}-build.sh, while the
> > latter refers to .ci/dpdk-{setup,build}.sh
> >
> > Otherwise the fix lgtm.
>
> So what about changing the commit message as follows:
>
>
> Previously, the dpdk build process was integrated into the
> .ci/{linux,dpdk}-build.sh scripts, where specific lines were employed to
> generate the key. Since it is now separated into two distinct files,
> .ci/dpdk-{setup,build}.sh, this patch will compute the key based on the
> content of these two files.
"""
When new drivers are introduced, the cache key is not accurately computed.
Before the commit 1a1b3106d90e ("ci: Separate DPDK from OVS build."),
the DPDK build
process was integrated in .ci/linux-{setup,build}.sh scripts, where
specific lines were
employed to generate the key.
Since it is now separated in .ci/dpdk-{setup,build}.sh, this patch
computes the key based on the
content of those dedicated scripts.
"""
Deal?
--
David Marchand
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
