Re: [ovs-dev] [PATCH] LACP-Rx packets are not captured in ovs-tcpdump.
> -Original Message- > From: Ben Pfaff [mailto:b...@ovn.org] > Sent: Thursday, February 14, 2019 10:58 PM > To: Nitin Katiyar > Cc: ovs-dev@openvswitch.org; Manohar Krishnappa Chidambaraswamy > > Subject: Re: [ovs-dev] [PATCH] LACP-Rx packets are not captured in ovs- > tcpdump. > > On Wed, Feb 13, 2019 at 10:51:18AM +, Nitin Katiyar wrote: > > Mirroring received LACP packets to help in debugging LACP issues. > > > > Co-authored-by: Manohar Krishnappa Chidambaraswamy > > > Signed-off-by: Manohar Krishnappa Chidambaraswamy > > > Signed-off-by: Nitin Katiyar > > --- > > ofproto/ofproto-dpif-xlate.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/ofproto/ofproto-dpif-xlate.c > > b/ofproto/ofproto-dpif-xlate.c index acd4817..111f36e 100644 > > --- a/ofproto/ofproto-dpif-xlate.c > > +++ b/ofproto/ofproto-dpif-xlate.c > > @@ -3308,6 +3308,7 @@ process_special(struct xlate_ctx *ctx, const > struct xport *xport) > > } else if (xport->xbundle && xport->xbundle->lacp > > && flow->dl_type == htons(ETH_TYPE_LACP)) { > > if (packet) { > > +mirror_ingress_packet(ctx); > > lacp_process_packet(xport->xbundle->lacp, xport->ofport, > > packet); > > } > > slow = SLOW_LACP; > > As far as I can tell this will get called from xlate_action() anyway: Yeah, you are right. I can drop this patch. > > if (!xin->frozen_state && process_special(, in_port)) { > /* process_special() did all the processing for this packet. > * > * We do not perform special processing on thawed packets, since that > * was done before they were frozen and should not be redone. */ > mirror_ingress_packet(); > } else if (in_port && in_port->xbundle > > Why is LACP more worthy of mirroring than the other protocols that > process_special() handles? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] Initialize the right database.
On Fri, Feb 15, 2019 at 12:59:57AM +, Ted Elhourani wrote: > Use value of db parameter in order to initialize the correct database. > > Signed-off-by: Ted Elhourani Thank you. I applied this to master and backported as far as branch-2.9. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [patch v1] ipf: Misc Cleanup.
Signed-off-by: Darrell Ball --- lib/ipf.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/ipf.c b/lib/ipf.c index df5196f..c91afd3 100644 --- a/lib/ipf.c +++ b/lib/ipf.c @@ -530,8 +530,6 @@ ipf_list_state_transition(struct ipf *ipf, struct ipf_list *ipf_list, case IPF_LIST_STATE_LAST_SEEN: if (ff) { next_state = IPF_LIST_STATE_FIRST_LAST_SEEN; -} else if (lf) { -next_state = IPF_LIST_STATE_LAST_SEEN; } else { next_state = IPF_LIST_STATE_LAST_SEEN; } @@ -765,7 +763,7 @@ ipf_list_key_eq(const struct ipf_list_key *key1, static struct ipf_list * ipf_list_key_lookup(struct ipf *ipf, const struct ipf_list_key *key, uint32_t hash) -/* OVS_REQUIRES(ipf->ipf_lock) */ +OVS_REQUIRES(ipf->ipf_lock) { struct ipf_list *ipf_list; HMAP_FOR_EACH_WITH_HASH (ipf_list, node, hash, >frag_lists) { -- 1.9.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] Initialize the right database.
Bleep bloop. Greetings Ted Elhourani, 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 Ted Elhourani needs to sign off. Lines checked: 28, Warnings: 0, Errors: 1 Please check this out. If you feel there has been an error, please email acon...@bytheb.org Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2] Initialize the right database.
Use value of db parameter in order to initialize the correct database. Signed-off-by: Ted Elhourani --- ovn/utilities/ovn-ctl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl index 1e369b1..f1297a2 100755 --- a/ovn/utilities/ovn-ctl +++ b/ovn/utilities/ovn-ctl @@ -222,7 +222,7 @@ $cluster_remote_port # Initialize the database if it's running standalone, # active-passive, or is the first server in a cluster. if test -z "$cluster_remote_addr"; then -ovn-nbctl init +ovn-${db}ctl init fi if test $mode = cluster; then -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V3 2/2] odp-util: Do not rewrite fields with the same values as matched
On Thu, Feb 07, 2019 at 06:44:16PM +0200, Eli Britstein wrote: > To improve performance and avoid wasting resources for HW offloaded > flows, do not rewrite fields that are matched with the same value. > > Signed-off-by: Eli Britstein > Reviewed-by: Roi Dayan Thanks for the patches. They don't build for me: ../lib/odp-util.c:7659:53: error: too few arguments to function call, expected 8, have 7 ../lib/odp-util.c:7201:1: note: 'commit' declared here ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] Initialize the right database.
On Fri, Feb 15, 2019 at 12:01:13AM +, Ted Elhourani wrote: > Use value of db parameter in order to initialize the correct database. Thanks for the patch. It looks good to me, but it needs a Signed-off-by. Q: What's a Signed-off-by and how do I provide one? A: Free and open source software projects usually require a contributor to provide some assurance that they're entitled to contribute the code that they provide. Some projects, for example, do this with a Contributor License Agreement (CLA) or a copyright assignment that is signed on paper or electronically. For this purpose, Open vSwitch has adopted something called the Developer's Certificate of Origin (DCO), which is also used by the Linux kernel and originated there. Informally stated, agreeing to the DCO is the developer's way of attesting that a particular commit that they are contributing is one that they are allowed to contribute. You should visit https://developercertificate.org/ to read the full statement of the DCO, which is less than 200 words long. To certify compliance with the Developer's Certificate of Origin for a particular commit, just add the following line to the end of your commit message, properly substituting your name and email address: Signed-off-by: Firstname Lastname Git has special support for adding a Signed-off-by line to a commit message: when you run "git commit", just add the -s option, as in "git commit -s". If you use the "git citool" GUI for commits, you can add a Signed-off-by line to the commit message by pressing Control+S. Other Git user interfaces may provide similar support. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] rstp: add ability to receive VLAN-tagged BPDUs
On Fri, Feb 15, 2019 at 12:27:11AM +0100, Matthias May wrote: > On 14/02/2019 20:17, Ben Pfaff wrote: > > On Thu, Feb 14, 2019 at 10:58:48AM +0100, Matthias May via dev wrote: > >> There are switches which allow to transmit their BPDUs VLAN-tagged. > >> With this change OVS is able to receive VLAN-tagged BPDUs, but still > >> transmits its own BPDUs untagged. > >> This was tested against Westermo RFI-207-F4G-T3G. > >> > >> Signed-off-by: Matthias May > > > > Thanks for the patch. > > > > To me, it seems really odd to treat packets with and without an > > arbitrary VLAN header the same way. I could see it if the VLAN header > > had VID 0 or 1 or some other specified value, but it seems unusual to > > ignore it entirely. Is this standardized or a de facto standard of some > > kind? > > > > I totally agree. > To me a VLAN header has nothing lost on a BPDU of a (R)STP frame, simply > because (R)STP is not per VLAN. > > However the fact is that there are switches which are transmitting > frames on a VLAN. > > With this change we simply ignore the VLAN header if is present. The > meaning of the BPDU doesn't cheange. The provided information still is > not per VLAN and applies to all ports the same. > > This patch does not add the ability to transmit VLAN tagged BPDUs for > the same reasoning above: RSTP/STP is not supposed to be per VLAN. > I was thinking about adding to the patch that one can specify a VLAN via > config and only BPDUs with the configured VLAN are accepted. I guess > this is what you propose: only accept vlan tagged BPDUs on a specified VLAN. > Having such a config-parameter would also enable to transmit the BPDUs > VLAN tagged. But I'm still of the opinion that this only suggests that > one could have an (R)STP tree per VLAN. I see we are basically in agreement, but I'd like more information, if you have it. Do the switches that transmit RSTP on a VLAN transmit it on a particular VLAN like 0 or 1? (Maybe they are transmitting them as priority-tagged frames for some reason?) If so, then it would be possible to accept just that VLAN. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] rstp: add ability to receive VLAN-tagged BPDUs
Bleep bloop. Greetings Matthias May via dev, 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 should not be mailing list. Lines checked: 50, Warnings: 0, Errors: 1 Please check this out. If you feel there has been an error, please email acon...@bytheb.org Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v1] Initialize the right database.
Use value of db parameter in order to initialize the correct database. --- ovn/utilities/ovn-ctl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl index 1e369b1..f1297a2 100755 --- a/ovn/utilities/ovn-ctl +++ b/ovn/utilities/ovn-ctl @@ -222,7 +222,7 @@ $cluster_remote_port # Initialize the database if it's running standalone, # active-passive, or is the first server in a cluster. if test -z "$cluster_remote_addr"; then -ovn-nbctl init +ovn-${db}ctl init fi if test $mode = cluster; then -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] rstp: add ability to receive VLAN-tagged BPDUs
On 14/02/2019 20:17, Ben Pfaff wrote: > On Thu, Feb 14, 2019 at 10:58:48AM +0100, Matthias May via dev wrote: >> There are switches which allow to transmit their BPDUs VLAN-tagged. >> With this change OVS is able to receive VLAN-tagged BPDUs, but still >> transmits its own BPDUs untagged. >> This was tested against Westermo RFI-207-F4G-T3G. >> >> Signed-off-by: Matthias May > > Thanks for the patch. > > To me, it seems really odd to treat packets with and without an > arbitrary VLAN header the same way. I could see it if the VLAN header > had VID 0 or 1 or some other specified value, but it seems unusual to > ignore it entirely. Is this standardized or a de facto standard of some > kind? > I totally agree. To me a VLAN header has nothing lost on a BPDU of a (R)STP frame, simply because (R)STP is not per VLAN. However the fact is that there are switches which are transmitting frames on a VLAN. With this change we simply ignore the VLAN header if is present. The meaning of the BPDU doesn't cheange. The provided information still is not per VLAN and applies to all ports the same. This patch does not add the ability to transmit VLAN tagged BPDUs for the same reasoning above: RSTP/STP is not supposed to be per VLAN. I was thinking about adding to the patch that one can specify a VLAN via config and only BPDUs with the configured VLAN are accepted. I guess this is what you propose: only accept vlan tagged BPDUs on a specified VLAN. Having such a config-parameter would also enable to transmit the BPDUs VLAN tagged. But I'm still of the opinion that this only suggests that one could have an (R)STP tree per VLAN. BR Matthias ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2] rstp: add ability to receive VLAN-tagged BPDUs
There are switches which allow to transmit their BPDUs VLAN-tagged. With this change OVS is able to receive VLAN-tagged BPDUs, but still transmits its own BPDUs untagged. This was tested against Westermo RFI-207-F4G-T3G. v2: Send patch to different address of mailing list according to suggestion of Flavio Leitner. Signed-off-by: Matthias May --- ofproto/ofproto-dpif-xlate.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index acd4817c2..7830fee2e 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -1771,9 +1771,12 @@ xport_rstp_should_manage_bpdu(const struct xport *xport) return rstp_should_manage_bpdu(xport_get_rstp_port_state(xport)); } +#define LEN_WITHOUT_VLAN (ETH_HEADER_LEN + LLC_HEADER_LEN) +#define LEN_WITH_VLAN (VLAN_HEADER_LEN + LEN_WITHOUT_VLAN) static void rstp_process_packet(const struct xport *xport, const struct dp_packet *packet) { +int len; struct dp_packet payload = *packet; struct eth_header *eth = dp_packet_data(); @@ -1787,7 +1790,9 @@ rstp_process_packet(const struct xport *xport, const struct dp_packet *packet) dp_packet_set_size(, ntohs(eth->eth_type) + ETH_HEADER_LEN); } -if (dp_packet_try_pull(, ETH_HEADER_LEN + LLC_HEADER_LEN)) { +/* Pull a bit less payload when the BPDU is enveloped in a VLAN header */ +len = (ntohs(eth->eth_type) == 0x8100) ? LEN_WITH_VLAN : LEN_WITHOUT_VLAN; +if (dp_packet_try_pull(, len)) { rstp_port_received_bpdu(xport->rstp_port, dp_packet_data(), dp_packet_size()); } -- 2.20.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v1] conntrack: Remove redundant call to 'hash_finish()'.
On Thu, Feb 14, 2019 at 01:15:07PM -0800, Darrell Ball wrote: > Signed-off-by: Darrell Ball Thanks for the series. I applied it to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v13 8/8] Userspace datapath: Add fragmentation handling.
Thanks for the series. I applied this to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v13 8/8] Userspace datapath: Add fragmentation handling.
I folded in the following incremental locally: --- a/lib/ipf.c +++ b/lib/ipf.c @@ -533,8 +533,6 @@ ipf_list_state_transition(struct ipf *ipf, struct ipf_list *ipf_list, case IPF_LIST_STATE_LAST_SEEN: if (ff) { next_state = IPF_LIST_STATE_FIRST_LAST_SEEN; -} else if (lf) { -next_state = IPF_LIST_STATE_LAST_SEEN; } else { next_state = IPF_LIST_STATE_LAST_SEEN; } @@ -768,7 +766,7 @@ ipf_list_key_eq(const struct ipf_list_key *key1, static struct ipf_list * ipf_list_key_lookup(struct ipf *ipf, const struct ipf_list_key *key, uint32_t hash) -/* OVS_REQUIRES(ipf->ipf_lock) */ +OVS_REQUIRES(ipf->ipf_lock) { struct ipf_list *ipf_list; HMAP_FOR_EACH_WITH_HASH (ipf_list, node, hash, >frag_lists) { On Wed, Feb 13, 2019 at 3:34 PM Darrell Ball wrote: > Fragmentation handling is added for supporting conntrack. > Both v4 and v6 are supported. > > After discussion with several people, I decided to not store > configuration state in the database to be more consistent with > the kernel in future, similarity with other conntrack configuration > which will not be in the database as well and overall simplicity. > Accordingly, fragmentation handling is enabled by default. > > This patch enables fragmentation tests for the userspace datapath. > > Signed-off-by: Darrell Ball > --- > Documentation/faq/releases.rst | 51 +- > NEWS | 10 +- > include/sparse/netinet/ip6.h |1 + > lib/automake.mk |4 +- > lib/conntrack.c | 22 +- > lib/conntrack.h |4 + > lib/ct-dpif.c| 58 +- > lib/ct-dpif.h| 12 +- > lib/dpctl.c | 215 +- > lib/dpctl.man| 36 + > lib/dpif-netdev.c| 65 +- > lib/dpif-netlink.c |9 +- > lib/dpif-provider.h | 53 +- > lib/ipf.c| 1528 > ++ > lib/ipf.h| 63 ++ > tests/system-kmod-macros.at | 46 +- > tests/system-traffic.at | 51 +- > tests/system-userspace-macros.at | 186 - > 18 files changed, 2332 insertions(+), 82 deletions(-) > create mode 100644 lib/ipf.c > create mode 100644 lib/ipf.h > > diff --git a/Documentation/faq/releases.rst > b/Documentation/faq/releases.rst > index cd7254b..a78152b 100644 > --- a/Documentation/faq/releases.rst > +++ b/Documentation/faq/releases.rst > @@ -105,31 +105,32 @@ Q: Are all features available with all datapaths? > The following table lists the datapath supported features from an Open > vSwitch user's perspective. > > -= == == = === > -Feature Linux upstream Linux OVS tree Userspace Hyper-V > -= == == = === > -NAT 4.6YESYes NO > -Connection tracking 4.3YESPARTIAL PARTIAL > -Tunnel - LISP NO YESNONO > -Tunnel - STT NO YESNOYES > -Tunnel - GRE 3.11 YESYES YES > -Tunnel - VXLAN3.12 YESYES YES > -Tunnel - Geneve 3.18 YESYES YES > -Tunnel - GRE-IPv6 4.18 YESYES NO > -Tunnel - VXLAN-IPv6 4.3YESYES NO > -Tunnel - Geneve-IPv6 4.4YESYES NO > -Tunnel - ERSPAN 4.18 YESYES NO > -Tunnel - ERSPAN-IPv6 4.18 YESYES NO > -QoS - PolicingYESYESYES NO > -QoS - Shaping YESYESNONO > -sFlow YESYESYES NO > -IPFIX 3.10 YESYES NO > -Set actionYESYESYES PARTIAL > -NIC Bonding YESYESYES YES > -Multiple VTEPsYESYESYES YES > -Meters4.15 YESYES NO > -Conntrack zone limit 4.18 YESNONO > -= == == = === > +== == == = > === > +FeatureLinux upstream Linux OVS tree Userspace > Hyper-V > +== == == = > === > +Connection tracking 4.3YES YES > YES > +Conntrack Fragment Reass. 4.3YES
[ovs-dev] [patch v1] conntrack: Simplify 'ct_addr'.
Remove the struct wrapper and remove the unneeded union members. There may even be a portability benefit here because of the type punning. Signed-off-by: Darrell Ball --- lib/conntrack-private.h | 6 +- lib/conntrack.c | 154 +++- lib/conntrack.h | 14 ++--- 3 files changed, 81 insertions(+), 93 deletions(-) diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h index a344801..96a6a9f 100644 --- a/lib/conntrack-private.h +++ b/lib/conntrack-private.h @@ -31,7 +31,7 @@ #include "dp-packet.h" struct ct_endpoint { -struct ct_addr addr; +union ct_addr addr; union { ovs_be16 port; struct { @@ -44,7 +44,7 @@ struct ct_endpoint { /* Verify that there is no padding in struct ct_endpoint, to facilitate * hashing in ct_endpoint_hash_add(). */ -BUILD_ASSERT_DECL(sizeof(struct ct_endpoint) == sizeof(struct ct_addr) + 4); +BUILD_ASSERT_DECL(sizeof(struct ct_endpoint) == sizeof(union ct_addr) + 4); /* Changes to this structure need to be reflected in conn_key_hash() * and conn_key_cmp(). */ @@ -77,7 +77,7 @@ struct alg_exp_node { /* Corresponding key of the control connection. */ struct conn_key master_key; /* The NAT replacement address to be used by the data connection. */ -struct ct_addr alg_nat_repl_addr; +union ct_addr alg_nat_repl_addr; /* The data connection inherits the master control * connection label and mark. */ ovs_u128 master_label; diff --git a/lib/conntrack.c b/lib/conntrack.c index 5c43410..c543aea 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -272,10 +272,10 @@ ct_print_conn_info(const struct conn *c, const char *log_msg, "%"PRIu16"/%"PRIu16" zone/rev zone " "%"PRIu16"/%"PRIu16" nw_proto/rev nw_proto " "%"PRIu8"/%"PRIu8, log_msg, -IP_ARGS(c->key.src.addr.ipv4_aligned), -IP_ARGS(c->key.dst.addr.ipv4_aligned), -IP_ARGS(c->rev_key.src.addr.ipv4_aligned), -IP_ARGS(c->rev_key.dst.addr.ipv4_aligned), +IP_ARGS(c->key.src.addr.ipv4), +IP_ARGS(c->key.dst.addr.ipv4), +IP_ARGS(c->rev_key.src.addr.ipv4), +IP_ARGS(c->rev_key.dst.addr.ipv4), ntohs(c->key.src.port), ntohs(c->key.dst.port), ntohs(c->rev_key.src.port), ntohs(c->rev_key.dst.port), c->key.zone, c->rev_key.zone, c->key.nw_proto, @@ -420,8 +420,8 @@ write_ct_md(struct dp_packet *pkt, uint16_t zone, const struct conn *conn, if (key) { if (key->dl_type == htons(ETH_TYPE_IP)) { pkt->md.ct_orig_tuple.ipv4 = (struct ovs_key_ct_tuple_ipv4) { -key->src.addr.ipv4_aligned, -key->dst.addr.ipv4_aligned, +key->src.addr.ipv4, +key->dst.addr.ipv4, key->nw_proto != IPPROTO_ICMP ? key->src.port : htons(key->src.icmp_type), key->nw_proto != IPPROTO_ICMP @@ -431,8 +431,8 @@ write_ct_md(struct dp_packet *pkt, uint16_t zone, const struct conn *conn, } else { pkt->md.ct_orig_tuple_ipv6 = true; pkt->md.ct_orig_tuple.ipv6 = (struct ovs_key_ct_tuple_ipv6) { -key->src.addr.ipv6_aligned, -key->dst.addr.ipv6_aligned, +key->src.addr.ipv6, +key->dst.addr.ipv6, key->nw_proto != IPPROTO_ICMPV6 ? key->src.port : htons(key->src.icmp_type), key->nw_proto != IPPROTO_ICMPV6 @@ -556,13 +556,12 @@ nat_packet(struct dp_packet *pkt, const struct conn *conn, bool related) if (conn->key.dl_type == htons(ETH_TYPE_IP)) { struct ip_header *nh = dp_packet_l3(pkt); packet_set_ipv4_addr(pkt, >ip_src, - conn->rev_key.dst.addr.ipv4_aligned); + conn->rev_key.dst.addr.ipv4); } else { struct ovs_16aligned_ip6_hdr *nh6 = dp_packet_l3(pkt); packet_set_ipv6_addr(pkt, conn->key.nw_proto, nh6->ip6_src.be32, - >rev_key.dst.addr.ipv6_aligned, - true); + >rev_key.dst.addr.ipv6, true); } if (!related) { pat_packet(pkt, conn); @@ -572,13 +571,12 @@ nat_packet(struct dp_packet *pkt, const struct conn *conn, bool related) if (conn->key.dl_type == htons(ETH_TYPE_IP)) { struct ip_header *nh = dp_packet_l3(pkt); packet_set_ipv4_addr(pkt, >ip_dst, - conn->rev_key.src.addr.ipv4_aligned); + conn->rev_key.src.addr.ipv4); } else { struct ovs_16aligned_ip6_hdr *nh6 =
[ovs-dev] [patch v1] conntrack: Remove redundant call to 'hash_finish()'.
Signed-off-by: Darrell Ball --- lib/conntrack.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/conntrack.c b/lib/conntrack.c index c543aea..9a618a5 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -2010,11 +2010,9 @@ conn_key_hash(const struct conn_key *key, uint32_t basis) hash = hsrc ^ hdst; /* Hash the rest of the key(L3 and L4 types and zone). */ -hash = hash_words((uint32_t *) (>dst + 1), +return hash_words((uint32_t *) (>dst + 1), (uint32_t *) (key + 1) - (uint32_t *) (>dst + 1), hash); - -return hash_finish(hash, 0); } static void -- 1.9.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [branch-2.9 1/2] conntrack: fix ftp ipv4 address substitution.
Thanks for the backports. I applied them to all the branches (2.6 to 2.9). ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] monitor.c: Fix crash when monitor condition adds new columns.
On Mon, Feb 11, 2019 at 06:19:21PM -0800, Han Zhou wrote: > From: Han Zhou > > The OVSDB conditional monitor implementation allows many clients > to share same copy of monitored data if the clients are sharing > same tables and columns being monitored, while they can have > different monitor conditions. In monitor conditions they can > have different columns which can be different from the columns > being monitored. So the struct ovsdb_monitor_table maintains the > union of the all the columns being used in any conditions. > > The problem of the current implementation is that for each change > set generated, it doesn't maintain any metadata for the number of > columns for the data that has already populated in it. Instead, it > always rely on the n_columns field of the struct ovsdb_monitor_table > to manipulate the data. However, the n_columns in struct > ovsdb_monitor_table can increase (e.g. when a client changes its > condition which involves more columns). So it can result in that > the existing rows in a change set with N columns being later processed > as if it had more than N columns, typically, when the row is freed. > This causes the ovsdb-server crashing (see an example of the > backtrace). Thanks. I applied this to master and backported as far as branch-2.7. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] rstp: add ability to receive VLAN-tagged BPDUs
On Thu, Feb 14, 2019 at 10:58:48AM +0100, Matthias May via dev wrote: > There are switches which allow to transmit their BPDUs VLAN-tagged. > With this change OVS is able to receive VLAN-tagged BPDUs, but still > transmits its own BPDUs untagged. > This was tested against Westermo RFI-207-F4G-T3G. > > Signed-off-by: Matthias May Thanks for the patch. To me, it seems really odd to treat packets with and without an arbitrary VLAN header the same way. I could see it if the VLAN header had VID 0 or 1 or some other specified value, but it seems unusual to ignore it entirely. Is this standardized or a de facto standard of some kind? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] hi
-- Good morning, My name is Mary, hope you are in good health? please reply my message , i have something to tell you. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Conducta y lenguaje corporal
Cursos Esenciales- Webinar Interactivo – Jueves 07 de Marzo Formación de Capacitadores Internos para la organización recibir capacitación de entidades externas pero también contar con capacitadores internos que cuenten con los conocimientos básicos para fomentar el aprendizaje del personal. Aprenderemos técnicas para estructurar un taller interno y desarrollar habilidades de enseñanza efectivas para capacitar a los colaboradores. Ejes Temáticos: • La importancia del aprendizaje. • Perfil y características del instructor. • Establecer objetivos de capacitación. • Conducta y lenguaje corporal al momento de exponer. Para mayor información, responder sobre este correo con la palabra Capacitadores + los siguientes datos: NOMBRE: TELÉFONO: EMPRESA: EMAIL ALTERNO: Llamanos al (045) 55 1554 6630 www.Innovalearn.mx ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] LACP-Rx packets are not captured in ovs-tcpdump.
On Wed, Feb 13, 2019 at 10:51:18AM +, Nitin Katiyar wrote: > Mirroring received LACP packets to help in debugging LACP issues. > > Co-authored-by: Manohar Krishnappa Chidambaraswamy > Signed-off-by: Manohar Krishnappa Chidambaraswamy > Signed-off-by: Nitin Katiyar > --- > ofproto/ofproto-dpif-xlate.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c > index acd4817..111f36e 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -3308,6 +3308,7 @@ process_special(struct xlate_ctx *ctx, const struct > xport *xport) > } else if (xport->xbundle && xport->xbundle->lacp > && flow->dl_type == htons(ETH_TYPE_LACP)) { > if (packet) { > +mirror_ingress_packet(ctx); > lacp_process_packet(xport->xbundle->lacp, xport->ofport, packet); > } > slow = SLOW_LACP; As far as I can tell this will get called from xlate_action() anyway: if (!xin->frozen_state && process_special(, in_port)) { /* process_special() did all the processing for this packet. * * We do not perform special processing on thawed packets, since that * was done before they were frozen and should not be redone. */ mirror_ingress_packet(); } else if (in_port && in_port->xbundle Why is LACP more worthy of mirroring than the other protocols that process_special() handles? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] doc: Add "Representors" topic document
> -Original Message- > From: Flavio Leitner > Sent: Thursday, February 14, 2019 2:56 PM > To: Ilya Maximets > Cc: Ian Stokes ; Ophir Munk > ; ovs-dev@openvswitch.org; Thomas Monjalon > > Subject: Re: [ovs-dev] [PATCH v1] doc: Add "Representors" topic document > > On Thu, Feb 14, 2019 at 01:47:27PM +0300, Ilya Maximets wrote: > > On 13.02.2019 19:39, Ian Stokes wrote: > > > On 2/13/2019 8:14 AM, Ophir Munk wrote: > > >> > > > > Anyway, The instructions for most of NICs, I guess, should be "just > > echo number of VFs to 'max_vfs' sysfs knob". We may add this > > information right here and make a remark that some NICs may require > > additional steps. We may also point to the openstack ovs-offload > > guide, first parts of it describes how to configure representors with > Mellanox NICs. > > IMHO, this amount of information should be enough and it does not > > deserve a separate manual. > > I agree it would be very useful to have instructions on how to put the card in > the correct state to use the representors. However, I'd avoid pointing to > other guides outside of OVS because we don't control them, so they can > change and move and our side will break. Also that usually they include more > instruction/details than actually is needed to make it work, which can be > confusing. > > I think people building OSP, for instance (but could be any other project), > would look after OVS documentation on how to build their side and not vice- > versa :-). > > fbl > I can drop the link to the external documents. The openstack document for example refers to ovs-kernel which may be confused with ovs-dpdk. To avoid confusion I will add all the information inside the OVS document here. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] doc: Add "Representors" topic document
> -Original Message- > From: Ilya Maximets > Sent: Thursday, February 14, 2019 12:47 PM > To: Ian Stokes ; Ophir Munk > ; ovs-dev@openvswitch.org > Cc: Olga Shern ; Kevin Traynor > ; Asaf Penso ; Thomas > Monjalon > Subject: Re: [PATCH v1] doc: Add "Representors" topic document > > On 13.02.2019 19:39, Ian Stokes wrote: > > On 2/13/2019 8:14 AM, Ophir Munk wrote: > >> > >> > >>> -Original Message- > -Original Message- > From: Ian Stokes > Sent: Tuesday, February 12, 2019 7:17 PM > To: Ilya Maximets ; Ophir Munk > ; ovs-dev@openvswitch.org > Cc: Olga Shern ; Kevin Traynor > ; Asaf Penso ; Thomas > Monjalon > Subject: Re: [PATCH v1] doc: Add "Representors" topic document > > On 2/12/2019 1:15 PM, Ilya Maximets wrote: > > On 11.02.2019 3:01, Ophir Munk wrote: > >> This details how to configure representors ports. > >> > >> Signed-off-by: Ophir Munk > >> --- > >> Documentation/topics/dpdk/phy.rst | 80 > +++ > >> 1 file changed, 80 insertions(+) > >> > >> ... > >> + > >> +Representors > >> + > >>> . > >> + > >> +.. important:: > >> + > >> + Representors ports are configured prior to OVS invocation and > independently > >> + of it, or by other means as well. Please consult a NIC vendor > instructions > >> + on how to establish representors. > > > > It'll be good to have configuration example for at least one > > commonly used NIC (ixgbe/i40e ?). Or maybe a link to the docs > > where the process > described. > > > > What do you think ? > > Ian, maybe you could add some example, since you have already > > tried it in > practice? > > > > Good call, I'll draw up an incremental and post here, if acceptable > we can roll it into the same patch. > > >>> > >>> In addition to Ian drawing - please find a link which details how to > >>> create VFs for ConnectX-4 5 or 6 NICs: > >>> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fd > >>> ocs.openstack.org%2Fneutron%2Frocky%2Fadmin%2Fconfig-ovs- > offload.htm > >>> > ldata=02%7C01%7Cophirmu%40mellanox.com%7C53cb5ae93dbd423a > 481608 > >>> > d69269d466%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C6368573 > 80568 > >>> > 209851sdata=Uz60DuaXDBOk7%2BkSuEG6iGk4qAOg3OfkYBj2BbUKK5 > k%3D > >>> p;reserved=0 section "Create Compute virtual functions". > >>> I will send an updated v2 with this reference. > >>> > Ian > >> > >> After giving it more thought - IMHO it's better to add a new document > under howto/ where NIC vendors could specify their configuration for > representors setup. > >> We can then refer from this document to the new one. > >> Ilay, Ian - what do you think? > >> > > > > Hmmm, for ixgbe/i40e that are bound to a dpdk compatible driver it's > > just a case of creating the vf with something similar to below > > > > echo 1 > /sys/bus/pci/devices/\:05\:00.0/max_vfs > > > > And then adding it as described above > > > > ovs-vsctl add-port br0 dpdk-rep5 -- set Interface dpdk-rep0 type=dpdk > > \ options:dpdk-devargs=:05:00.0,representor=[0] > > > > I'm not sure that it would warrant it's own document for those devices, > how does it compare to the mellanox creation of vfs? Is that more complex > or different? > > I know that to make representors appear as linux interfaces (for using them > in OVS kernel datapath, for example) you need to turn the PF into > "switchdev" > mode. Probably, the same needed with DPDK. > Exactly. For Mellanox NICs we should explicitly turn the PF into "switchdev". Those are additional commands, not mentioned here. > Anyway, The instructions for most of NICs, I guess, should be "just echo > number of VFs to 'max_vfs' sysfs knob". We may add this information right > here and make a remark that some NICs may require additional steps. Agreed. I will add them in v2. > We > may also point to the openstack ovs-offload guide, first parts of it describes > how to configure representors with Mellanox NICs. Agreed. In v2 I will add the link to the openstack ovs-offload guilde. > IMHO, this amount of information should be enough and it does not deserve > a separate manual. Agreed. Many thanks for the prompt review. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] rstp: add ability to receive VLAN-tagged BPDUs
On Thu, Feb 14, 2019 at 03:06:33PM +0100, Matthias May via dev wrote: > On 14/02/2019 14:34, Flavio Leitner wrote: > > Matthias, could you describe to where exactly you sent out the > > patch? ovs-dev@openvswitch.org or d...@openvswitch.org or something > > else? > Some info: > * git version 2.20.1 > * (Reproduced) trace of what I entered: > ``` > maym@CHD500279:~/git/ovs$ git send-email send-email/ --no-chain-reply > send-email/0001-rstp-add-ability-to-receive-VLAN-tagged-BPDUs.patch > To whom should the emails be sent (if anyone)? ovs-dev@openvswitch.org > > Message-ID to be used as In-Reply-To for the first email (if any)? > > (mbox) Adding cc: Matthias May from line 'From: > Matthias May ' > (body) Adding cc: Matthias May from line > 'Signed-off-by: Matthias May ' > > From: Matthias May > To: ovs-dev@openvswitch.org > Cc: Matthias May > Subject: [PATCH] rstp: add ability to receive VLAN-tagged BPDUs > Date: Thu, 14 Feb 2019 15:01:34 +0100 > Message-Id: <20190214140134.16754-1-matthias@neratec.com> > X-Mailer: git-send-email 2.20.1 > MIME-Version: 1.0 > Content-Transfer-Encoding: 8bit > > The Cc list above has been expanded by additional > addresses found in the patch commit message. By default > send-email prompts before sending whenever this occurs. > This behavior is controlled by the sendemail.confirm > configuration setting. > > For additional information, run 'git send-email --help'. > To retain the current behavior, but squelch this message, > run 'git config --global sendemail.confirm auto'. > > Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): y > > ``` > > This obviously has a different Message-Id then the mail sent before. Do you mind to send the same patch to d...@openvswitch.org to see if that happens again? I don't know how the list is configured but so far it only happened with ovs-dev@ and not with dev@. Thanks, fbl ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] rstp: add ability to receive VLAN-tagged BPDUs
On Thu, Feb 14, 2019 at 03:14:26PM +0100, Matthias May wrote: > On 14/02/2019 14:34, Flavio Leitner wrote: > > > > Hi Ben, > > > > This is another patch with From: field altered to be > > ovs-dev@openvswitch.org. > > > > [...] > > From: Matthias May via dev > > Reply-To: Matthias May > > > > If the patch gets applied as is, the commit's Author will have the > > wrong email. > > > > Just FYI because before Sriharsha and Neal had the same issue and we > > haven't identified the root cause back then. > > > > Matthias, could you describe to where exactly you sent out the > > patch? ovs-dev@openvswitch.org or d...@openvswitch.org or something > > else? > > > > Thanks, > > fbl > > > > > *thread snipped* > Maybe this is related to the SRS settings on mailman. > Looks to me a bit like "munge" is used instead of "wrap". I guess that you're talking about the mailman setting for "from_is_list (general): Replace the From: header address with the list's posting address to mitigate issues stemming from the original From: domain's DMARC or similar policies." This is set to "no", to avoid changing From: addresses at all. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Planes de carrera y sucesión
Cursos TOP - Webinar Interactivo – Miércoles 06 de Marzo Planes de carrera y sucesión El no disponer de un estudio de planes de carrera del personal puede suponer un factor de desmotivación de los trabajadores/as, que ven limitada su capacidad de ascenso y mejora en las condiciones de trabajo. Desarrollaremos planes de carrera y sucesión que aseguren el funcionamiento óptimo de una empresa. Ejes Temáticos: • Competencias organizacionales complejas. • Desarrollo de plan de vida y carrera. • Entrenamiento para elevar el potencial de los colaboradores. • Planes de sucesión dentro de la organización. Para mayor información, responder sobre este correo con la palabra PLAN + los siguientes datos: NOMBRE: TELÉFONO: EMPRESA: EMAIL ALTERNO: Llamanos al (045) 55 1554 6630 www.Innovalearn.mx ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] rstp: add ability to receive VLAN-tagged BPDUs
On 14/02/2019 14:34, Flavio Leitner wrote: > > Hi Ben, > > This is another patch with From: field altered to be > ovs-dev@openvswitch.org. > > [...] > From: Matthias May via dev > Reply-To: Matthias May > > If the patch gets applied as is, the commit's Author will have the > wrong email. > > Just FYI because before Sriharsha and Neal had the same issue and we > haven't identified the root cause back then. > > Matthias, could you describe to where exactly you sent out the > patch? ovs-dev@openvswitch.org or d...@openvswitch.org or something > else? > > Thanks, > fbl > > *thread snipped* Maybe this is related to the SRS settings on mailman. Looks to me a bit like "munge" is used instead of "wrap". BR Matthias ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] rstp: add ability to receive VLAN-tagged BPDUs
On 14/02/2019 14:34, Flavio Leitner wrote: > > Hi Ben, > > This is another patch with From: field altered to be > ovs-dev@openvswitch.org. > > [...] > From: Matthias May via dev > Reply-To: Matthias May > > If the patch gets applied as is, the commit's Author will have the > wrong email. > > Just FYI because before Sriharsha and Neal had the same issue and we > haven't identified the root cause back then. > > Matthias, could you describe to where exactly you sent out the > patch? ovs-dev@openvswitch.org or d...@openvswitch.org or something > else? > > Thanks, > fbl > > > On Thu, Feb 14, 2019 at 12:18:11PM +0100, Matthias May via dev wrote: >> On 14/02/2019 11:59, 0-day Robot wrote: >>> Bleep bloop. Greetings Matthias May via dev, 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 should not be mailing list. >>> Lines checked: 47, Warnings: 0, Errors: 1 >>> >>> >>> Please check this out. If you feel there has been an error, please email >>> acon...@bytheb.org >>> >>> Thanks, >>> 0-day Robot >>> ___ >>> dev mailing list >>> d...@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> >> >> What is this supposed to mean? >> Is the Signed-off-by not enough? >> >> BR >> Matthias >> ___ >> dev mailing list >> d...@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > Hi Flavio Some info: * git version 2.20.1 * (Reproduced) trace of what I entered: ``` maym@CHD500279:~/git/ovs$ git send-email send-email/ --no-chain-reply send-email/0001-rstp-add-ability-to-receive-VLAN-tagged-BPDUs.patch To whom should the emails be sent (if anyone)? ovs-dev@openvswitch.org Message-ID to be used as In-Reply-To for the first email (if any)? (mbox) Adding cc: Matthias May from line 'From: Matthias May ' (body) Adding cc: Matthias May from line 'Signed-off-by: Matthias May ' From: Matthias May To: ovs-dev@openvswitch.org Cc: Matthias May Subject: [PATCH] rstp: add ability to receive VLAN-tagged BPDUs Date: Thu, 14 Feb 2019 15:01:34 +0100 Message-Id: <20190214140134.16754-1-matthias@neratec.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit The Cc list above has been expanded by additional addresses found in the patch commit message. By default send-email prompts before sending whenever this occurs. This behavior is controlled by the sendemail.confirm configuration setting. For additional information, run 'git send-email --help'. To retain the current behavior, but squelch this message, run 'git config --global sendemail.confirm auto'. Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): y ``` This obviously has a different Message-Id then the mail sent before. BR Matthias ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netdev: Fix unsafe accesses to pmd polling lists.
Ian Stokes writes: > On 2/13/2019 10:40 AM, Ilya Maximets wrote: >> On 13.02.2019 1:24, Ben Pfaff wrote: >>> On Tue, Feb 12, 2019 at 10:40:30AM +0300, Ilya Maximets wrote: On 11.02.2019 22:15, Aaron Conole wrote: > Ilya Maximets writes: > >> All accesses to 'pmd->poll_list' should be guarded by >> 'pmd->port_mutex'. Additionally fixed inappropriate usage >> of 'HMAP_FOR_EACH_SAFE' (hmap doesn't change in a loop) >> and dropped not needed local variable 'proc_cycles'. >> >> CC: Nitin Katiyar >> Fixes: 5bf84282482a ("Adding support for PMD auto load balancing") >> Signed-off-by: Ilya Maximets >> --- >> lib/dpif-netdev.c | 14 +- >> 1 file changed, 9 insertions(+), 5 deletions(-) >> >> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c >> index 4d5bc576a..914b2bb8b 100644 >> --- a/lib/dpif-netdev.c >> +++ b/lib/dpif-netdev.c >> @@ -3776,9 +3776,12 @@ set_pmd_auto_lb(struct dp_netdev *dp) >> continue; >> } >> +ovs_mutex_lock(>port_mutex); >> if (hmap_count(>poll_list) > 1) { >> multi_rxq = true; >> } >> +ovs_mutex_unlock(>port_mutex); > > What does this mutex provide here? I ask because as soon as we unlock, > the result is no longer assured, and we've used it to inform the > multi_rxq value. > > Are you afraid that the read is non-atomic so you'll end up with the > multi_rxq condition true, even when it should not be? That can happen > anyway - as soon as we unlock the count can change. > > Maybe there's a better change to support that, where hmap_count does an > atomic read, or there's a version that can guarantee a full load with > no intervening writes. But I don't see > > Maybe the pmd object lifetime goes away... but that wouldn't make sense, > because then the port_mutex itself would be invalid. > > I'm confused what it does to add a lock here for either the safety, or > the logic. I probably missed something. Or maybe it's just trying to > be safe in case the hmap implementation would change in the future? I > think if that's the case, there might be an alternate to implement? The mutex here is mostly for a code consistency. We're marking the 'poll_list' as OVS_GUARDED. This means that we should guard it. Unfortunately, unlike С++, in C we can not use OVS_GUARDED_BY(port_mutex) inside the structure and clang could not track what metex should protect the variable. I agree that we're not really need to protect this. Furthermore, the only writer for 'poll_list' is the main thread and 'set_pmd_auto_lb' is also called only in a main thread. So we don't need to protect any read accesses. However, for the safety, these details should not be taken into account to not provide code snippets that could be copied to the other (less safe) places and produce issues there. Especially because clang can't protect us from this kind of mistakes. Code architecture also could be changed in the future. As a developer I'd like to always assume that any access to hmap is not thread safe regardless of its internal implementation. Just like I'm assuming that cmap is safe for readers. This is affordable since we're not on a hot path. >>> >>> Maybe a comment would be helpful. >>> >>> /* We don't really need this lock but it suppresses a Clang warning. */ >> >> Problem is that it's not true. Clang is not able to track this kind of >> guarding in C. >> So, the only warning I want to suppress is in my head. I'd like to keep the >> locking >> here to not think about safety of each operation with 'poll_list' in the >> future. >> (Maybe someday clang will be smart enough to show warnings for guarded >> structure >> members in C) >> >> Aaron, as I have no real technical reasons for these locks, I could drop >> this part >> of the patch. Do you think I should ? > > My 2 cents, it looks like it would help ensure it as a standard > practice when operating with poll_list, and can help avoid copy/paste > issues in the future that could be dangerous. > > Maybe it's being a tad defensive in the code but as their would be no > impact on the hot path I think that's ok. I have a different experience with unneeded locks. In small amounts, then it isn't an issue. Lots of calls, even in non-hotpath, create huge amounts of overhead (for instance, the likelihood of thread contention increases, meaning instead of the futex having a successful CAS(,0,1), we actually will fall into the system call branch, and then be at the mercy of the scheduler and doing lots of context switches, increased runqueue sizes, higher CPU utilization, etc). That said, I guess the bigger issue would be catching copy-paste where it isn't needed,
Re: [ovs-dev] rstp: add ability to receive VLAN-tagged BPDUs
Hi Ben, This is another patch with From: field altered to be ovs-dev@openvswitch.org. [...] From: Matthias May via dev Reply-To: Matthias May If the patch gets applied as is, the commit's Author will have the wrong email. Just FYI because before Sriharsha and Neal had the same issue and we haven't identified the root cause back then. Matthias, could you describe to where exactly you sent out the patch? ovs-dev@openvswitch.org or d...@openvswitch.org or something else? Thanks, fbl On Thu, Feb 14, 2019 at 12:18:11PM +0100, Matthias May via dev wrote: > On 14/02/2019 11:59, 0-day Robot wrote: > > Bleep bloop. Greetings Matthias May via dev, 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 should not be mailing list. > > Lines checked: 47, Warnings: 0, Errors: 1 > > > > > > Please check this out. If you feel there has been an error, please email > > acon...@bytheb.org > > > > Thanks, > > 0-day Robot > > ___ > > dev mailing list > > d...@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > What is this supposed to mean? > Is the Signed-off-by not enough? > > BR > Matthias > ___ > 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 v1] doc: Add "Representors" topic document
On Thu, Feb 14, 2019 at 01:47:27PM +0300, Ilya Maximets wrote: > On 13.02.2019 19:39, Ian Stokes wrote: > > On 2/13/2019 8:14 AM, Ophir Munk wrote: > >> > >> > >>> -Original Message- > -Original Message- > From: Ian Stokes > Sent: Tuesday, February 12, 2019 7:17 PM > To: Ilya Maximets ; Ophir Munk > ; ovs-dev@openvswitch.org > Cc: Olga Shern ; Kevin Traynor > ; Asaf Penso ; Thomas > Monjalon > Subject: Re: [PATCH v1] doc: Add "Representors" topic document > > On 2/12/2019 1:15 PM, Ilya Maximets wrote: > > On 11.02.2019 3:01, Ophir Munk wrote: > >> This details how to configure representors ports. > >> > >> Signed-off-by: Ophir Munk > >> --- > >> Documentation/topics/dpdk/phy.rst | 80 > +++ > >> 1 file changed, 80 insertions(+) > >> > >> ... > >> + > >> +Representors > >> + > >>> . > >> + > >> +.. important:: > >> + > >> + Representors ports are configured prior to OVS invocation and > independently > >> + of it, or by other means as well. Please consult a NIC vendor > instructions > >> + on how to establish representors. > > > > It'll be good to have configuration example for at least one > > commonly used NIC (ixgbe/i40e ?). Or maybe a link to the docs where > > the process > described. > > > > What do you think ? > > Ian, maybe you could add some example, since you have already tried > > it in > practice? > > > > Good call, I'll draw up an incremental and post here, if acceptable we > can roll it into the same patch. > > >>> > >>> In addition to Ian drawing - please find a link which details how to > >>> create VFs > >>> for ConnectX-4 5 or 6 NICs: > >>> https://docs.openstack.org/neutron/rocky/admin/config-ovs-offload.html > >>> section "Create Compute virtual functions". > >>> I will send an updated v2 with this reference. > >>> > Ian > >> > >> After giving it more thought - IMHO it's better to add a new document > >> under howto/ where NIC vendors could specify their configuration for > >> representors setup. > >> We can then refer from this document to the new one. > >> Ilay, Ian - what do you think? > >> > > > > Hmmm, for ixgbe/i40e that are bound to a dpdk compatible driver it's just a > > case of creating the vf with something similar to below > > > > echo 1 > /sys/bus/pci/devices/\:05\:00.0/max_vfs > > > > And then adding it as described above > > > > ovs-vsctl add-port br0 dpdk-rep5 -- set Interface dpdk-rep0 type=dpdk \ > > options:dpdk-devargs=:05:00.0,representor=[0] > > > > I'm not sure that it would warrant it's own document for those devices, how > > does it compare to the mellanox creation of vfs? Is that more complex or > > different? > > I know that to make representors appear as linux interfaces (for using them > in OVS kernel datapath, for example) you need to turn the PF into "switchdev" > mode. Probably, the same needed with DPDK. > > Anyway, The instructions for most of NICs, I guess, should be "just echo > number > of VFs to 'max_vfs' sysfs knob". We may add this information right here and > make a remark that some NICs may require additional steps. We may also point > to the openstack ovs-offload guide, first parts of it describes how to > configure > representors with Mellanox NICs. > IMHO, this amount of information should be enough and it does not deserve a > separate manual. I agree it would be very useful to have instructions on how to put the card in the correct state to use the representors. However, I'd avoid pointing to other guides outside of OVS because we don't control them, so they can change and move and our side will break. Also that usually they include more instruction/details than actually is needed to make it work, which can be confusing. I think people building OSP, for instance (but could be any other project), would look after OVS documentation on how to build their side and not vice-versa :-). fbl ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] how to enable the log of ovn
Dear friends, Sorry to disturb. I have use ovn with openstasck for a while time,but i still do not know how to enable the log of ovn. There are only logging options in the manpage or the help of ovn-nbctl/ovn-sbctl,but no any logging command. When i excute cmd of 'ovn-nbctl --syslog-method=libc --verbose=dbg' or others similar,the print is 'ovn-nbctl: missing command name (use --help for help)' Regards, Yunxiang ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] rstp: add ability to receive VLAN-tagged BPDUs
On 14/02/2019 11:59, 0-day Robot wrote: > Bleep bloop. Greetings Matthias May via dev, 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 should not be mailing list. > Lines checked: 47, Warnings: 0, Errors: 1 > > > Please check this out. If you feel there has been an error, please email > acon...@bytheb.org > > Thanks, > 0-day Robot > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > What is this supposed to mean? Is the Signed-off-by not enough? BR Matthias ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] rstp: add ability to receive VLAN-tagged BPDUs
Bleep bloop. Greetings Matthias May via dev, 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 should not be mailing list. Lines checked: 47, Warnings: 0, Errors: 1 Please check this out. If you feel there has been an error, please email acon...@bytheb.org Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] doc: Add "Representors" topic document
On 13.02.2019 19:39, Ian Stokes wrote: > On 2/13/2019 8:14 AM, Ophir Munk wrote: >> >> >>> -Original Message- -Original Message- From: Ian Stokes Sent: Tuesday, February 12, 2019 7:17 PM To: Ilya Maximets ; Ophir Munk ; ovs-dev@openvswitch.org Cc: Olga Shern ; Kevin Traynor ; Asaf Penso ; Thomas Monjalon Subject: Re: [PATCH v1] doc: Add "Representors" topic document On 2/12/2019 1:15 PM, Ilya Maximets wrote: > On 11.02.2019 3:01, Ophir Munk wrote: >> This details how to configure representors ports. >> >> Signed-off-by: Ophir Munk >> --- >> Documentation/topics/dpdk/phy.rst | 80 +++ >> 1 file changed, 80 insertions(+) >> >> ... >> + >> +Representors >> + >>> . >> + >> +.. important:: >> + >> + Representors ports are configured prior to OVS invocation and independently >> + of it, or by other means as well. Please consult a NIC vendor instructions >> + on how to establish representors. > > It'll be good to have configuration example for at least one > commonly used NIC (ixgbe/i40e ?). Or maybe a link to the docs where > the process described. > > What do you think ? > Ian, maybe you could add some example, since you have already tried > it in practice? > Good call, I'll draw up an incremental and post here, if acceptable we can roll it into the same patch. >>> >>> In addition to Ian drawing - please find a link which details how to create >>> VFs >>> for ConnectX-4 5 or 6 NICs: >>> https://docs.openstack.org/neutron/rocky/admin/config-ovs-offload.html >>> section "Create Compute virtual functions". >>> I will send an updated v2 with this reference. >>> Ian >> >> After giving it more thought - IMHO it's better to add a new document under >> howto/ where NIC vendors could specify their configuration for representors >> setup. >> We can then refer from this document to the new one. >> Ilay, Ian - what do you think? >> > > Hmmm, for ixgbe/i40e that are bound to a dpdk compatible driver it's just a > case of creating the vf with something similar to below > > echo 1 > /sys/bus/pci/devices/\:05\:00.0/max_vfs > > And then adding it as described above > > ovs-vsctl add-port br0 dpdk-rep5 -- set Interface dpdk-rep0 type=dpdk \ > options:dpdk-devargs=:05:00.0,representor=[0] > > I'm not sure that it would warrant it's own document for those devices, how > does it compare to the mellanox creation of vfs? Is that more complex or > different? I know that to make representors appear as linux interfaces (for using them in OVS kernel datapath, for example) you need to turn the PF into "switchdev" mode. Probably, the same needed with DPDK. Anyway, The instructions for most of NICs, I guess, should be "just echo number of VFs to 'max_vfs' sysfs knob". We may add this information right here and make a remark that some NICs may require additional steps. We may also point to the openstack ovs-offload guide, first parts of it describes how to configure representors with Mellanox NICs. IMHO, this amount of information should be enough and it does not deserve a separate manual. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] rstp: add ability to receive VLAN-tagged BPDUs
There are switches which allow to transmit their BPDUs VLAN-tagged. With this change OVS is able to receive VLAN-tagged BPDUs, but still transmits its own BPDUs untagged. This was tested against Westermo RFI-207-F4G-T3G. Signed-off-by: Matthias May --- ofproto/ofproto-dpif-xlate.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index acd4817c2..7830fee2e 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -1771,9 +1771,12 @@ xport_rstp_should_manage_bpdu(const struct xport *xport) return rstp_should_manage_bpdu(xport_get_rstp_port_state(xport)); } +#define LEN_WITHOUT_VLAN (ETH_HEADER_LEN + LLC_HEADER_LEN) +#define LEN_WITH_VLAN (VLAN_HEADER_LEN + LEN_WITHOUT_VLAN) static void rstp_process_packet(const struct xport *xport, const struct dp_packet *packet) { +int len; struct dp_packet payload = *packet; struct eth_header *eth = dp_packet_data(); @@ -1787,7 +1790,9 @@ rstp_process_packet(const struct xport *xport, const struct dp_packet *packet) dp_packet_set_size(, ntohs(eth->eth_type) + ETH_HEADER_LEN); } -if (dp_packet_try_pull(, ETH_HEADER_LEN + LLC_HEADER_LEN)) { +/* Pull a bit less payload when the BPDU is enveloped in a VLAN header */ +len = (ntohs(eth->eth_type) == 0x8100) ? LEN_WITH_VLAN : LEN_WITHOUT_VLAN; +if (dp_packet_try_pull(, len)) { rstp_port_received_bpdu(xport->rstp_port, dp_packet_data(), dp_packet_size()); } -- 2.20.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netdev: Fix unsafe accesses to pmd polling lists.
On 2/13/2019 10:40 AM, Ilya Maximets wrote: On 13.02.2019 1:24, Ben Pfaff wrote: On Tue, Feb 12, 2019 at 10:40:30AM +0300, Ilya Maximets wrote: On 11.02.2019 22:15, Aaron Conole wrote: Ilya Maximets writes: All accesses to 'pmd->poll_list' should be guarded by 'pmd->port_mutex'. Additionally fixed inappropriate usage of 'HMAP_FOR_EACH_SAFE' (hmap doesn't change in a loop) and dropped not needed local variable 'proc_cycles'. CC: Nitin Katiyar Fixes: 5bf84282482a ("Adding support for PMD auto load balancing") Signed-off-by: Ilya Maximets --- lib/dpif-netdev.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 4d5bc576a..914b2bb8b 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -3776,9 +3776,12 @@ set_pmd_auto_lb(struct dp_netdev *dp) continue; } +ovs_mutex_lock(>port_mutex); if (hmap_count(>poll_list) > 1) { multi_rxq = true; } +ovs_mutex_unlock(>port_mutex); What does this mutex provide here? I ask because as soon as we unlock, the result is no longer assured, and we've used it to inform the multi_rxq value. Are you afraid that the read is non-atomic so you'll end up with the multi_rxq condition true, even when it should not be? That can happen anyway - as soon as we unlock the count can change. Maybe there's a better change to support that, where hmap_count does an atomic read, or there's a version that can guarantee a full load with no intervening writes. But I don't see Maybe the pmd object lifetime goes away... but that wouldn't make sense, because then the port_mutex itself would be invalid. I'm confused what it does to add a lock here for either the safety, or the logic. I probably missed something. Or maybe it's just trying to be safe in case the hmap implementation would change in the future? I think if that's the case, there might be an alternate to implement? The mutex here is mostly for a code consistency. We're marking the 'poll_list' as OVS_GUARDED. This means that we should guard it. Unfortunately, unlike С++, in C we can not use OVS_GUARDED_BY(port_mutex) inside the structure and clang could not track what metex should protect the variable. I agree that we're not really need to protect this. Furthermore, the only writer for 'poll_list' is the main thread and 'set_pmd_auto_lb' is also called only in a main thread. So we don't need to protect any read accesses. However, for the safety, these details should not be taken into account to not provide code snippets that could be copied to the other (less safe) places and produce issues there. Especially because clang can't protect us from this kind of mistakes. Code architecture also could be changed in the future. As a developer I'd like to always assume that any access to hmap is not thread safe regardless of its internal implementation. Just like I'm assuming that cmap is safe for readers. This is affordable since we're not on a hot path. Maybe a comment would be helpful. /* We don't really need this lock but it suppresses a Clang warning. */ Problem is that it's not true. Clang is not able to track this kind of guarding in C. So, the only warning I want to suppress is in my head. I'd like to keep the locking here to not think about safety of each operation with 'poll_list' in the future. (Maybe someday clang will be smart enough to show warnings for guarded structure members in C) Aaron, as I have no real technical reasons for these locks, I could drop this part of the patch. Do you think I should ? My 2 cents, it looks like it would help ensure it as a standard practice when operating with poll_list, and can help avoid copy/paste issues in the future that could be dangerous. Maybe it's being a tad defensive in the code but as their would be no impact on the hot path I think that's ok. Ian ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Greetings to you,
Greetings to you, Sorry to invade in your privacy and if truly you have not received your Inheritance payment, Contract payment, lottery Payment, please reconfirm your information once again to enable us release your payment through our corresponding paying bank in Benin Republic. You are here by warned not to communicate or duplicate this message to anyone for any reason what so ever. Kindly forward your information to us immediately you receive this message. 1. Your full Name. 2. Contact Address. 3. Your Cell-phone Number. 4. Your Date of Birth. 5. Your International Passport photo page or your Driver's License We are waiting. Ms Alyssa Shaw United Nations Office United Kingdom ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Thanks For Your Effort So !! / Gracias por tu esfuerzo tan !!
Hallo , Wie geht es Ihnen heute? Ich hoffe, dass meine Post Sie und Ihren Haushalt gut und gesund trifft. Ich m?chte Sie dar?ber informieren, dass ich mit Hilfe meines neuen Partners aus Saudi-Arabien meinen Erbschaftsfonds erfolgreich nach Saudi-Arabien verlagert habe. Mein Partner ist derzeit mit dem Investitionsprojekt in Dubai besch?ftigt und ich habe auch eine erfolgreiche medizinische Behandlung in einem deutschen Krankenhaus in Hamburg. Ich bin v?llig in Ordnung und gesund. Ich kann immer noch nicht all Ihre Bem?hungen vergessen, den Fonds zu bewegen, obwohl es nicht geklappt hat. Ich und mein Partner haben zugestimmt, etwas f?r Sie als Entsch?digung f?r all Ihre Anstrengungen zu finden, die Sie tun, um mir zu helfen. Ich hinterlasse die Summe von 800.000 $ f?r Sie, setzen Sie sich mit Herrn Adam Smith in Verbindung, damit sie das Geld sofort an Sie ?berweisen kann. Nachfolgend finden Sie die Kontaktdaten von Herrn Adam Smith Name: Herr Adam Smith Anschrift: Cotonou-Benin-Rep E-Mail: cont...@getmaworldwide.org Bitte kontaktieren Sie mich, sobald Sie das Geld erhalten haben. Vielen Dank Dein Kleiner Ana Mimi In English Hello , How are you doing today, I do hope that my mail meet you and your household well and in good health? I wish to inform you that i have successfully claim and moved my inheritance fund to Saudi-Arabia through the help of my new partner from Saudi-Arabia. My Partner is currently busy with the investment project in Dubai and I also have a successful medical treatment in a Hospital in Hamburg, German.I am fully ok health wise and sound now. I still can`t forget all your effort to help move the fund despite it didn`t work out, I and my partner agreed to mapped out something for you as compensation for all your effort you put to help me. I do leave behind the sum of $800.000 for you, do contact Mr Adam Smith so that she will transfer the money to you immediately. Below is Mr Adam Smith contact details Name: Mr Adam Smith Address: Cotonou-Benin-Rep Email: cont...@getmaworldwide.org Please contact me immediately you receive the money. Thanks Yours Little Ana Mimi (PP) Disclaimer: This message is intended only for the use of the person to whom it is expressly addressed and may contain information that is confidential and legally privileged. If you are not the intended recipient, you are hereby notified that any use, reliance on, reference to, review, disclosure or copying of the message and the information it contains for any purpose is prohibited. If you have received this message in error, please notify the sender by reply e-mail of the misdelivery and delete all its contents. Opinions, conclusions and other information in this message that do not relate to the official business of Institut Terjemahan & Buku Malaysia shall be understood as neither given nor endorsed by it. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev