[ovs-dev] Reply For More Details.
-- My dear, I am Mrs Maryalice Williams, I want to send you donation of two million seven hundred thousand Dollars ($2.7M) for volunteer projects in your country due to my ill health that could not permit me. Kindly reply for more details, and also send me the following details, as per below, your full Name .., Address..., Age..., Occupation ... Remain blessed, Mrs. Maryalice Williams. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next] net: Fix typo of SKB_SGO_CB_OFFSET
From: Cambda Zhu
Date: Thu, 26 Mar 2020 15:33:14 +0800
> The SKB_SGO_CB_OFFSET should be SKB_GSO_CB_OFFSET which means the
> offset of the GSO in skb cb. This patch fixes the typo.
>
> Fixes: 9207f9d45b0a ("net: preserve IP control block during GSO segmentation")
> Signed-off-by: Cambda Zhu
Applied.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] BUG: Weird failure of OVS classifier
On Wed, Mar 25, 2020 at 07:35:22PM -0700, Jean Tourrilhes wrote: > Hi all, > > I have stumbled on a weird failure of the OVS classifier. This > is a bit beyond my abilities, so I would like other people to have > a look at it. > This is the error in the log : > --- > 2020-03-26T02:05:22.693Z|01237|dpif(handler86)|DBG|system@ovs-system: failed > to put[create] (File exists) ufid:a4bba536-21ad-4c5d-9dd9-ab8f72263ce1 > recirc_id(0),dp_hash(0/0),skb_priority(0/0),in_port(1),skb_mark(0/0),ct_state(0x2a/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),ct_tuple4(src=10.68.58.40/0.0.0.0,dst=10.68.58.41/0.0.0.0,proto=17/0,tp_src=55732/0,tp_dst=7364/0),eth(src=22:c3:19:dc:23:47/00:00:00:00:00:00,dst=fa:c1:63:72:87:45/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=10.68.58.41/0.0.0.0,dst=10.68.58.40/255.255.255.0,proto=17,tos=0/0x3,ttl=64/0,frag=no),udp(src=7364/0,dst=55732/0x8000), > > actions:set(tunnel(tun_id=0x0,dst=10.68.68.40,ttl=64,tp_dst=4789,flags(df|key))),2 > --- > From my understanding, this should not happen, so one of the > classifier is wrong, the main question is if it is the kernel > classifier or the vswitchd classifier. Usually this kind of thing means that userspace and the kernel don't have the same idea of the definition of a flow. Tracking it down is often tricky. This kind of miserable problem is one reason why I wish that OVS could switch to an entirely packet-based interface. The user/kernel interface is almost unmaintainable. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v1 1/3] net: openvswitch: expand the meters number supported
On Mon, Mar 30, 2020 at 12:46 AM Pravin Shelar wrote:
>
> On Sat, Mar 28, 2020 at 8:46 AM wrote:
> >
> > From: Tonghao Zhang
> >
> > In kernel datapath of Open vSwitch, there are only 1024
> > buckets of meter in one dp. If installing more than 1024
> > (e.g. 8192) meters, it may lead to the performance drop.
> > But in some case, for example, Open vSwitch used as edge
> > gateway, there should be 200,000+ at least, meters used for
> > IP address bandwidth limitation.
> >
> > [Open vSwitch userspace datapath has this issue too.]
> >
> > For more scalable meter, this patch expands the buckets
> > when necessary, so we can install more meters in the datapath.
> >
> > * Introducing the struct *dp_meter_instance*, it's easy to
> > expand meter though change the *ti* point in the struct
> > *dp_meter_table*.
> > * Using kvmalloc_array instead of kmalloc_array.
> >
> Thanks for working on this, I have couple of comments.
>
> > Cc: Pravin B Shelar
> > Cc: Andy Zhou
> > Signed-off-by: Tonghao Zhang
> > ---
> > net/openvswitch/datapath.h | 2 +-
> > net/openvswitch/meter.c| 168 ++---
> > net/openvswitch/meter.h| 17 +++-
> > 3 files changed, 153 insertions(+), 34 deletions(-)
> >
> > diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
> > index e239a46c2f94..785105578448 100644
> > --- a/net/openvswitch/datapath.h
> > +++ b/net/openvswitch/datapath.h
> > @@ -82,7 +82,7 @@ struct datapath {
> > u32 max_headroom;
> >
> > /* Switch meters. */
> > - struct hlist_head *meters;
> > + struct dp_meter_table *meters;
> > };
> >
> > /**
> > diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
> > index 5010d1ddd4bd..98003b201b45 100644
> > --- a/net/openvswitch/meter.c
> > +++ b/net/openvswitch/meter.c
> > @@ -47,40 +47,136 @@ static void ovs_meter_free(struct dp_meter *meter)
> > kfree_rcu(meter, rcu);
> > }
> >
> > -static struct hlist_head *meter_hash_bucket(const struct datapath *dp,
> > +static struct hlist_head *meter_hash_bucket(struct dp_meter_instance *ti,
> > u32 meter_id)
> > {
> > - return &dp->meters[meter_id & (METER_HASH_BUCKETS - 1)];
> > + u32 hash = jhash_1word(meter_id, ti->hash_seed);
> > +
> I do not see any need to hash meter-id, can you explain it.
>
> > + return &ti->buckets[hash & (ti->n_buckets - 1)];
> > }
> >
> > /* Call with ovs_mutex or RCU read lock. */
> > -static struct dp_meter *lookup_meter(const struct datapath *dp,
> > +static struct dp_meter *lookup_meter(const struct dp_meter_table *tbl,
> > u32 meter_id)
> > {
> > + struct dp_meter_instance *ti = rcu_dereference_ovsl(tbl->ti);
> > struct dp_meter *meter;
> > struct hlist_head *head;
> >
> > - head = meter_hash_bucket(dp, meter_id);
> > - hlist_for_each_entry_rcu(meter, head, dp_hash_node,
> > - lockdep_ovsl_is_held()) {
> > + head = meter_hash_bucket(ti, meter_id);
> > + hlist_for_each_entry_rcu(meter, head, hash_node[ti->node_ver],
> > +lockdep_ovsl_is_held()) {
> > if (meter->id == meter_id)
> > return meter;
> > }
> > +
> This patch is expanding meter table linearly with number meters added
> to datapath. so I do not see need to have hash table. it can be a
> simple array. This would also improve lookup efficiency.
> For hash collision we could find next free slot in array. let me know
> what do you think about this approach.
Hi Pravin
If we use the simple array, when inserting the meter, for hash collision, we can
find next free slot, but one case, when there are many meters in the array.
we may find many slot for the free slot.
And when we lookup the meter, for hash collision, we may find many
array slots, and
then find it, or that meter does not exist in the array, In that case,
there may be a lookup performance
drop.
For hash meter-id in meter_hash_bucket, I am not 100% sure it is
useful. it just update
hash_seed when expand meters. For performance, we can remove it. Thanks.
>
> > return NULL;
> > }
> >
> > -static void attach_meter(struct datapath *dp, struct dp_meter *meter)
> > +static struct dp_meter_instance *dp_meter_instance_alloc(const int size)
> > +{
> > + struct dp_meter_instance *ti;
> > + int i;
> > +
> > + ti = kmalloc(sizeof(*ti), GFP_KERNEL);
> > + if (!ti)
> > + return NULL;
> > +
> > + ti->buckets = kvmalloc_array(size, sizeof(struct hlist_head),
> > +GFP_KERNEL);
> > + if (!ti->buckets) {
> > + kfree(ti);
> > + return NULL;
> > + }
> > +
> > + for (i = 0; i < size; i++)
> > + INIT_HLIST_HEAD(&ti->buckets[i]);
> > +
> > + ti->n_buckets = size;
> > + ti->node_ver = 0;
> > +
[ovs-dev] Good Day
-- Good Day, I am Mr Liang Chung, Senior Account Officer with the International bank of Taipei, I am requesting for your partnership in re-profiling funds. Contact me for further details on private Email (liangchun...@gmail.com) I look forward to it. Regards, Liang Chung ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] AB bonding: Add "primary" interface concept
I haven't seen any comments on this patch yet; just wanted to make sure it
didn't get overlooked.
Thanks!
> On Mar 23, 2020, at 4:02 PM, Jeff Squyres wrote:
>
> A "primary" slave interface will always become active if it is
> enabled. The "primary" concept exists in Linux kernel network
> interface bonding, but did not previously exist in OVS bonding.
>
> Only one "primary" slave inteface is supported per bond, and
> is only supported for active/backup bonding.
>
> The primary slave interface is designated via
> "other_config:bond-primary" when creating a bond.
>
> Signed-off-by: Jeff Squyres
> ---
> ofproto/bond.c| 58 +++-
> ofproto/bond.h| 1 +
> tests/lacp.at | 1 +
> tests/ofproto-dpif.at | 78 ++-
> vswitchd/bridge.c | 5 +++
> vswitchd/vswitch.xml | 8 +
> 6 files changed, 149 insertions(+), 2 deletions(-)
>
> diff --git a/ofproto/bond.c b/ofproto/bond.c
> index 405202fb6..2437246ca 100644
> --- a/ofproto/bond.c
> +++ b/ofproto/bond.c
> @@ -93,6 +93,7 @@ struct bond_slave {
> /* Link status. */
> bool enabled; /* May be chosen for flows? */
> bool may_enable;/* Client considers this slave bondable. */
> +bool is_primary;/* This slave is preferred over others. */
> long long delay_expires;/* Time after which 'enabled' may change. */
>
> /* Rebalancing info. Used only by bond_rebalance(). */
> @@ -126,6 +127,7 @@ struct bond {
> enum lacp_status lacp_status; /* Status of LACP negotiations. */
> bool bond_revalidate; /* True if flows need revalidation. */
> uint32_t basis; /* Basis for flow hash function. */
> +char *primary; /* Name of the primary slave interface. */
>
> /* SLB specific bonding info. */
> struct bond_entry *hash; /* An array of BOND_BUCKETS elements. */
> @@ -241,6 +243,7 @@ bond_create(const struct bond_settings *s, struct
> ofproto_dpif *ofproto)
>
> bond->active_slave_mac = eth_addr_zero;
> bond->active_slave_changed = false;
> +bond->primary = NULL;
>
> bond_reconfigure(bond, s);
> return bond;
> @@ -290,6 +293,7 @@ bond_unref(struct bond *bond)
> update_recirc_rules__(bond);
>
> hmap_destroy(&bond->pr_rule_ops);
> +free(bond->primary);
> free(bond->name);
> free(bond);
> }
> @@ -459,6 +463,31 @@ bond_reconfigure(struct bond *bond, const struct
> bond_settings *s)
> bond->bond_revalidate = false;
> }
>
> +/*
> + * If a primary interface is set on the new settings:
> + * 1. If the bond has no primary previously set, save it and
> + * revalidate.
> + * 2. If the bond has a different primary previously set, save the
> + * new one and revalidate.
> + * 3. If the bond has the same primary previously set, do nothing.
> + */
> +if (s->primary) {
> +bool changed = false;
> +if (bond->primary) {
> +if (strcmp(bond->primary, s->primary) != 0) {
> +free(bond->primary);
> +changed = true;
> +}
> +} else {
> +changed = true;
> +}
> +
> +if (changed) {
> +bond->primary = xstrdup(s->primary);
> +revalidate = true;
> +}
> +}
> +
> if (bond->balance != BM_AB) {
> if (!bond->recirc_id) {
> bond->recirc_id = recirc_alloc_id(bond->ofproto);
> @@ -549,6 +578,12 @@ bond_slave_register(struct bond *bond, void *slave_,
> slave->name = xstrdup(netdev_get_name(netdev));
> bond->bond_revalidate = true;
>
> +if (bond->primary && strcmp(bond->primary, slave->name) == 0) {
> +slave->is_primary = true;
> +} else {
> +slave->is_primary = false;
> +}
> +
> slave->enabled = false;
> bond_enable_slave(slave, netdev_get_carrier(netdev));
> }
> @@ -644,6 +679,7 @@ bond_run(struct bond *bond, enum lacp_status lacp_status)
> {
> struct bond_slave *slave;
> bool revalidate;
> +struct bond_slave *primary;
>
> ovs_rwlock_wrlock(&rwlock);
> if (bond->lacp_status != lacp_status) {
> @@ -659,11 +695,19 @@ bond_run(struct bond *bond, enum lacp_status
> lacp_status)
> }
>
> /* Enable slaves based on link status and LACP feedback. */
> +primary = NULL;
> HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) {
> bond_link_status_update(slave);
> slave->change_seq = seq_read(connectivity_seq_get());
> +
> +/* Discover if there is an active slave marked "primary". */
> +if (bond->balance == BM_AB && slave->is_primary && slave->enabled) {
> +primary = slave;
> +}
> }
> -if (!bond->active_slave || !bond->active_slave->enabled) {
> +
> +if (!bond->active_slave || !bond->active_slave->enabled ||
> +(primary && bond->active_slave != primary)) {
> bo
Re: [ovs-dev] [PATCH ovn] Rename "VTEP switch" to "ramp switch".
On Tue, Mar 24, 2020 at 5:15 AM Ben Pfaff wrote: > > The term "VTEP" is a poor choice because a VTEP is just a VXLAN Tunnel > Endpoint, which is a generic term not specific to OVN. When we talk > about "VTEP switches" and "VTEP gateways", therefore, it's somewhat > confusing. > > This commit introduces the term "ramp", which uses the metaphor of one > of these switches as an on-ramp and an off-ramp to an OVN logical > network, and substitutes it wherever it can be in the documentation. > Bits of the OVN database are harder to change, since changing them > would cause backward compatibility problems, so this commit doesn't try. > > It might be a good idea to change some program names (e.g. > ovn-controller-vtep), some distro package names, and so on. This commit > does not do that. > > CC: Ihar Hrachyshka > Signed-off-by: Ben Pfaff Acked-by: Numan Siddique I think it's better to rename ovn-controller-vtep to ovn-controller-ramp (or ovn-ramp-controller ?) If the rename is fine, I can work on that once this patch is merged. Thanks Numan > --- > Documentation/faq/general.rst | 10 +-- > Documentation/topics/high-availability.rst | 10 +-- > Documentation/tutorials/ovn-openstack.rst | 2 +- > NEWS | 2 + > controller/physical.c | 15 ++-- > include/ovn/logical-fields.h | 10 ++- > northd/ovn-northd.c| 13 ++-- > ovn-architecture.7.xml | 84 -- > ovn-nb.xml | 12 ++-- > ovn-sb.xml | 19 ++--- > utilities/ovn-nbctl.8.xml | 2 +- > 11 files changed, 97 insertions(+), 82 deletions(-) > > diff --git a/Documentation/faq/general.rst b/Documentation/faq/general.rst > index 831ca0445d08..c1b0a6907771 100644 > --- a/Documentation/faq/general.rst > +++ b/Documentation/faq/general.rst > @@ -108,10 +108,12 @@ Q: Why does OVN use STT and Geneve instead of VLANs or > VXLAN (or GRE)? > may need to allocate more bits to the datapath or egress port > identifiers. > > -As a side note, OVN does support VXLAN for use with ASIC-based top of > rack > -switches, using ``ovn-controller-vtep(8)`` and the OVSDB VTEP schema > -described in ``vtep(5)``, but this limits the features available from OVN > -to the subset available from the VTEP schema. > +As a side note, OVN does support VXLAN for use with "ramp > +gateways" that allow ASIC-based top of rack switches to act as > +on-ramp from a physical network into an OVN logical network. Ramp > +switches use ``ovn-controller-vtep(8)`` and the OVSDB VTEP schema > +described in ``vtep(5)``. This limits the features available from > +OVN to the subset available from the VTEP schema. > > Q: How can I contribute to the OVN Community? > > diff --git a/Documentation/topics/high-availability.rst > b/Documentation/topics/high-availability.rst > index c3c962c1d045..9f1d94722ee2 100644 > --- a/Documentation/topics/high-availability.rst > +++ b/Documentation/topics/high-availability.rst > @@ -52,11 +52,11 @@ OVN Gateway High Availability Plan > > The OVN gateway is responsible for shuffling traffic between the tunneled > overlay network (governed by ovn-northd), and the legacy physical network. > In > -a naive implementation, the gateway is a single x86 server, or hardware VTEP. > -For most deployments, a single system has enough forwarding capacity to > service > -the entire virtualized network, however, it introduces a single point of > -failure. If this system dies, the entire OVN deployment becomes unavailable. > -To mitigate this risk, an HA solution is critical -- by spreading > +a naive implementation, the gateway is a single x86 server or a hardware ToR > +switch. For most deployments, a single system has enough forwarding capacity > +to service the entire virtualized network, however, it introduces a single > +point of failure. If this system dies, the entire OVN deployment becomes > +unavailable. To mitigate this risk, an HA solution is critical -- by > spreading > responsibility across multiple systems, no single server failure can take > down > the network. > > diff --git a/Documentation/tutorials/ovn-openstack.rst > b/Documentation/tutorials/ovn-openstack.rst > index 2e4f63404670..e5356dad6f4f 100644 > --- a/Documentation/tutorials/ovn-openstack.rst > +++ b/Documentation/tutorials/ovn-openstack.rst > @@ -1959,4 +1959,4 @@ explore some of these directions: > > * Other kinds of gateways. In addition to floating IPs with NAT, OVN >supports directly attaching VMs to a physical network and connecting > - logical switches to VTEP hardware. > + logical switches to hardware top-of-rack switches. > diff --git a/NEWS b/NEWS > index 393721d70111..cb322d03ee6b 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,5 +1,7 @@ > Post-v20.03.0 > -- > + - "VTEP" gateways and
Re: [ovs-dev] [PATCH net-next v1 1/3] net: openvswitch: expand the meters number supported
On Sat, Mar 28, 2020 at 8:46 AM wrote:
>
> From: Tonghao Zhang
>
> In kernel datapath of Open vSwitch, there are only 1024
> buckets of meter in one dp. If installing more than 1024
> (e.g. 8192) meters, it may lead to the performance drop.
> But in some case, for example, Open vSwitch used as edge
> gateway, there should be 200,000+ at least, meters used for
> IP address bandwidth limitation.
>
> [Open vSwitch userspace datapath has this issue too.]
>
> For more scalable meter, this patch expands the buckets
> when necessary, so we can install more meters in the datapath.
>
> * Introducing the struct *dp_meter_instance*, it's easy to
> expand meter though change the *ti* point in the struct
> *dp_meter_table*.
> * Using kvmalloc_array instead of kmalloc_array.
>
Thanks for working on this, I have couple of comments.
> Cc: Pravin B Shelar
> Cc: Andy Zhou
> Signed-off-by: Tonghao Zhang
> ---
> net/openvswitch/datapath.h | 2 +-
> net/openvswitch/meter.c| 168 ++---
> net/openvswitch/meter.h| 17 +++-
> 3 files changed, 153 insertions(+), 34 deletions(-)
>
> diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
> index e239a46c2f94..785105578448 100644
> --- a/net/openvswitch/datapath.h
> +++ b/net/openvswitch/datapath.h
> @@ -82,7 +82,7 @@ struct datapath {
> u32 max_headroom;
>
> /* Switch meters. */
> - struct hlist_head *meters;
> + struct dp_meter_table *meters;
> };
>
> /**
> diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
> index 5010d1ddd4bd..98003b201b45 100644
> --- a/net/openvswitch/meter.c
> +++ b/net/openvswitch/meter.c
> @@ -47,40 +47,136 @@ static void ovs_meter_free(struct dp_meter *meter)
> kfree_rcu(meter, rcu);
> }
>
> -static struct hlist_head *meter_hash_bucket(const struct datapath *dp,
> +static struct hlist_head *meter_hash_bucket(struct dp_meter_instance *ti,
> u32 meter_id)
> {
> - return &dp->meters[meter_id & (METER_HASH_BUCKETS - 1)];
> + u32 hash = jhash_1word(meter_id, ti->hash_seed);
> +
I do not see any need to hash meter-id, can you explain it.
> + return &ti->buckets[hash & (ti->n_buckets - 1)];
> }
>
> /* Call with ovs_mutex or RCU read lock. */
> -static struct dp_meter *lookup_meter(const struct datapath *dp,
> +static struct dp_meter *lookup_meter(const struct dp_meter_table *tbl,
> u32 meter_id)
> {
> + struct dp_meter_instance *ti = rcu_dereference_ovsl(tbl->ti);
> struct dp_meter *meter;
> struct hlist_head *head;
>
> - head = meter_hash_bucket(dp, meter_id);
> - hlist_for_each_entry_rcu(meter, head, dp_hash_node,
> - lockdep_ovsl_is_held()) {
> + head = meter_hash_bucket(ti, meter_id);
> + hlist_for_each_entry_rcu(meter, head, hash_node[ti->node_ver],
> +lockdep_ovsl_is_held()) {
> if (meter->id == meter_id)
> return meter;
> }
> +
This patch is expanding meter table linearly with number meters added
to datapath. so I do not see need to have hash table. it can be a
simple array. This would also improve lookup efficiency.
For hash collision we could find next free slot in array. let me know
what do you think about this approach.
> return NULL;
> }
>
> -static void attach_meter(struct datapath *dp, struct dp_meter *meter)
> +static struct dp_meter_instance *dp_meter_instance_alloc(const int size)
> +{
> + struct dp_meter_instance *ti;
> + int i;
> +
> + ti = kmalloc(sizeof(*ti), GFP_KERNEL);
> + if (!ti)
> + return NULL;
> +
> + ti->buckets = kvmalloc_array(size, sizeof(struct hlist_head),
> +GFP_KERNEL);
> + if (!ti->buckets) {
> + kfree(ti);
> + return NULL;
> + }
> +
> + for (i = 0; i < size; i++)
> + INIT_HLIST_HEAD(&ti->buckets[i]);
> +
> + ti->n_buckets = size;
> + ti->node_ver = 0;
> + get_random_bytes(&ti->hash_seed, sizeof(u32));
> +
> + return ti;
> +}
> +
> +static void dp_meter_instance_free_rcu(struct rcu_head *rcu)
> {
> - struct hlist_head *head = meter_hash_bucket(dp, meter->id);
> + struct dp_meter_instance *ti;
>
> - hlist_add_head_rcu(&meter->dp_hash_node, head);
> + ti = container_of(rcu, struct dp_meter_instance, rcu);
> + kvfree(ti->buckets);
> + kfree(ti);
> }
>
> -static void detach_meter(struct dp_meter *meter)
> +static void dp_meter_instance_insert(struct dp_meter_instance *ti,
> +struct dp_meter *meter)
> +{
> + struct hlist_head *head = meter_hash_bucket(ti, meter->id);
> +
> + hlist_add_head_rcu(&meter->hash_node[ti->node_ver], head);
> +}
> +
> +static void dp_meter_instance_remove(struct dp_meter_instanc
