Re: [ovs-dev] [PATCH v2] Change in openvswitch kernel module to support MPLS label depth of 3 in ingress direction.
Hi
I have posted again.
Thanks,
Martin
-Original Message-
From: Tonghao Zhang
Sent: Friday, November 22, 2019 7:47 AM
To: Gregory Rose ; Varghese, Martin (Nokia -
IN/Bangalore)
Cc: ovs dev
Subject: Re: [ovs-dev] [PATCH v2] Change in openvswitch kernel module to
support MPLS label depth of 3 in ingress direction.
On Fri, Nov 22, 2019 at 2:15 AM Gregory Rose wrote:
>
>
> On 11/21/2019 7:49 AM, Martin Varghese wrote:
> > From: Martin Varghese
> >
> > The openvswitch kernel module was supporting a MPLS label depth of 1
> > in the ingress direction though the userspace OVS supports a max
> > depth of 3 labels. This change enables openvswitch module to support
> > a max depth of 3 labels in the ingress.
> >
> > Signed-off-by: Martin Varghese
>
> Did you submit this patch upstream as well?
This patch was applied in upstream, commit id
fbdcdd78da7c95f1b970d371e1b23cbd3aa990f3
Hi Martin, when you backport the patch, the commit message should be formatted:
http://docs.openvswitch.org/en/latest/internals/contributing/backporting-patches/
> - Greg
>
> > ---
> > Changes in v2
> > - support added for nested actions.
> >
> > datapath/actions.c | 2 +-
> > datapath/flow.c | 20
> > datapath/flow.h | 8 +++--
> > datapath/flow_netlink.c | 85
> > -
> > tests/system-traffic.at | 39 +++
> > 5 files changed, 122 insertions(+), 32 deletions(-)
> >
> > diff --git a/datapath/actions.c b/datapath/actions.c index
> > a44e804..fbf4457 100644
> > --- a/datapath/actions.c
> > +++ b/datapath/actions.c
> > @@ -276,7 +276,7 @@ static int set_mpls(struct sk_buff *skb, struct
> > sw_flow_key *flow_key,
> > }
> >
> > stack->label_stack_entry = lse;
> > - flow_key->mpls.top_lse = lse;
> > + flow_key->mpls.lse[0] = lse;
> > return 0;
> > }
> >
> > diff --git a/datapath/flow.c b/datapath/flow.c index
> > 916f7f4..6dc7402 100644
> > --- a/datapath/flow.c
> > +++ b/datapath/flow.c
> > @@ -659,27 +659,35 @@ static int key_extract_l3l4(struct sk_buff *skb,
> > struct sw_flow_key *key)
> > memset(&key->ipv4, 0, sizeof(key->ipv4));
> > }
> > } else if (eth_p_mpls(key->eth.type)) {
> > - size_t stack_len = MPLS_HLEN;
> > + u8 label_count = 1;
> >
> > + memset(&key->mpls, 0, sizeof(key->mpls));
> > skb_set_inner_network_header(skb, skb->mac_len);
> > while (1) {
> > __be32 lse;
> >
> > - error = check_header(skb, skb->mac_len + stack_len);
> > + error = check_header(skb, skb->mac_len +
> > + label_count * MPLS_HLEN);
> > if (unlikely(error))
> > return 0;
> >
> > memcpy(&lse, skb_inner_network_header(skb),
> > MPLS_HLEN);
> >
> > - if (stack_len == MPLS_HLEN)
> > - memcpy(&key->mpls.top_lse, &lse, MPLS_HLEN);
> > + if (label_count <= MPLS_LABEL_DEPTH)
> > + memcpy(&key->mpls.lse[label_count - 1], &lse,
> > +MPLS_HLEN);
> >
> > - skb_set_inner_network_header(skb, skb->mac_len +
> > stack_len);
> > + skb_set_inner_network_header(skb, skb->mac_len +
> > + label_count *
> > + MPLS_HLEN);
> > if (lse & htonl(MPLS_LS_S_MASK))
> > break;
> >
> > - stack_len += MPLS_HLEN;
> > + label_count++;
> > }
> > + if (label_count > MPLS_LABEL_DEPTH)
> > + label_count = MPLS_LABEL_DEPTH;
> > +
> > + key->mpls.num_labels_mask = GENMASK(label_count - 1,
> > + 0);
> > } else if (key->eth.type == htons(ETH_P_IPV6)) {
> > int nh_len; /* IPv6 Header + Extensions */
> >
> > diff --git a/datapath/flow.h b/datapath/flow.h index
> > 5560300..4ad5363 100644
> > --- a/datapath/flow.h
> > +++ b/datapath/flow.h
> > @@ -43,6 +43,7 @@ enum sw_flow_mac_proto {
> > MAC_PROTO_ETHERNET,
> > };
> > #define SW_FLOW_KEY_INVALID 0x80
> > +#define MPLS_LABEL_DEPTH 3
> >
> > /* Store options at the end of the array if they are less than the
> >* maximum size. This allows us to get the benefits of variable
> > length @@ -98,9 +99,6 @@ struct sw_flow_key {
> >*/
> > union {
> > struct {
> > - __be32 top_lse; /* top label stack entry */
> > - } mpls;
> > - struct {
> > u8 proto; /* IP protocol or lower 8 bits of ARP
> > opcode. */
> > u8 tos; /* IP ToS. */
> >
[ovs-dev] [PATCH] Datapath: Change in openvswitch kernel module to support MPLS label depth of 3 in ingress direction.
From: Martin Varghese
Upstream commit:
commit fbdcdd78da7c95f1b970d371e1b23cbd3aa990f3
Author: Martin Varghese
Date: Mon Nov 4 07:27:44 2019 +0530
Change in Openvswitch to support MPLS label depth of 3 in ingress
direction
The openvswitch was supporting a MPLS label depth of 1 in the
ingress direction though the userspace OVS supports a max depth
of 3 labels.This change enables openvswitch module to support a
max depth of 3 labels in the ingress.
Signed-off-by: Martin Varghese
Acked-by: Pravin B Shelar
Signed-off-by: David S. Miller
Signed-off-by: Martin Varghese
---
datapath/actions.c | 2 +-
datapath/flow.c | 20
datapath/flow.h | 8 +++--
datapath/flow_netlink.c | 85 -
tests/system-traffic.at | 39 +++
5 files changed, 122 insertions(+), 32 deletions(-)
diff --git a/datapath/actions.c b/datapath/actions.c
index a44e804..fbf4457 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -276,7 +276,7 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key
*flow_key,
}
stack->label_stack_entry = lse;
- flow_key->mpls.top_lse = lse;
+ flow_key->mpls.lse[0] = lse;
return 0;
}
diff --git a/datapath/flow.c b/datapath/flow.c
index 916f7f4..6dc7402 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -659,27 +659,35 @@ static int key_extract_l3l4(struct sk_buff *skb, struct
sw_flow_key *key)
memset(&key->ipv4, 0, sizeof(key->ipv4));
}
} else if (eth_p_mpls(key->eth.type)) {
- size_t stack_len = MPLS_HLEN;
+ u8 label_count = 1;
+ memset(&key->mpls, 0, sizeof(key->mpls));
skb_set_inner_network_header(skb, skb->mac_len);
while (1) {
__be32 lse;
- error = check_header(skb, skb->mac_len + stack_len);
+ error = check_header(skb, skb->mac_len +
+label_count * MPLS_HLEN);
if (unlikely(error))
return 0;
memcpy(&lse, skb_inner_network_header(skb), MPLS_HLEN);
- if (stack_len == MPLS_HLEN)
- memcpy(&key->mpls.top_lse, &lse, MPLS_HLEN);
+ if (label_count <= MPLS_LABEL_DEPTH)
+ memcpy(&key->mpls.lse[label_count - 1], &lse,
+ MPLS_HLEN);
- skb_set_inner_network_header(skb, skb->mac_len +
stack_len);
+ skb_set_inner_network_header(skb, skb->mac_len +
+label_count * MPLS_HLEN);
if (lse & htonl(MPLS_LS_S_MASK))
break;
- stack_len += MPLS_HLEN;
+ label_count++;
}
+ if (label_count > MPLS_LABEL_DEPTH)
+ label_count = MPLS_LABEL_DEPTH;
+
+ key->mpls.num_labels_mask = GENMASK(label_count - 1, 0);
} else if (key->eth.type == htons(ETH_P_IPV6)) {
int nh_len; /* IPv6 Header + Extensions */
diff --git a/datapath/flow.h b/datapath/flow.h
index 5560300..4ad5363 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -43,6 +43,7 @@ enum sw_flow_mac_proto {
MAC_PROTO_ETHERNET,
};
#define SW_FLOW_KEY_INVALID0x80
+#define MPLS_LABEL_DEPTH 3
/* Store options at the end of the array if they are less than the
* maximum size. This allows us to get the benefits of variable length
@@ -98,9 +99,6 @@ struct sw_flow_key {
*/
union {
struct {
- __be32 top_lse; /* top label stack entry */
- } mpls;
- struct {
u8 proto; /* IP protocol or lower 8 bits of ARP
opcode. */
u8 tos; /* IP ToS. */
u8 ttl; /* IP TTL/hop limit. */
@@ -148,6 +146,10 @@ struct sw_flow_key {
} nd;
};
} ipv6;
+ struct {
+ u32 num_labels_mask;/* labels present bitmap of
effective length MPLS_LABEL_DEPTH */
+ __be32 lse[MPLS_LABEL_DEPTH]; /* label stack entry
*/
+ } mpls;
struct ovs_key_nsh nsh; /* network service header */
};
struct {
diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index 35f13d7..9fc1a19 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -438,7 +438,7 @@ static const struct ovs_len_tbl
ovs_key_lens[OVS_KEY_ATTR_MAX + 1] = {
[OVS_
Re: [ovs-dev] [PATCH v2] Change in openvswitch kernel module to support MPLS label depth of 3 in ingress direction.
On Fri, Nov 22, 2019 at 2:15 AM Gregory Rose wrote:
>
>
> On 11/21/2019 7:49 AM, Martin Varghese wrote:
> > From: Martin Varghese
> >
> > The openvswitch kernel module was supporting a MPLS label depth of 1
> > in the ingress direction though the userspace OVS supports a max depth
> > of 3 labels. This change enables openvswitch module to support a max
> > depth of 3 labels in the ingress.
> >
> > Signed-off-by: Martin Varghese
>
> Did you submit this patch upstream as well?
This patch was applied in upstream, commit id
fbdcdd78da7c95f1b970d371e1b23cbd3aa990f3
Hi Martin, when you backport the patch, the commit message should be formatted:
http://docs.openvswitch.org/en/latest/internals/contributing/backporting-patches/
> - Greg
>
> > ---
> > Changes in v2
> > - support added for nested actions.
> >
> > datapath/actions.c | 2 +-
> > datapath/flow.c | 20
> > datapath/flow.h | 8 +++--
> > datapath/flow_netlink.c | 85
> > -
> > tests/system-traffic.at | 39 +++
> > 5 files changed, 122 insertions(+), 32 deletions(-)
> >
> > diff --git a/datapath/actions.c b/datapath/actions.c
> > index a44e804..fbf4457 100644
> > --- a/datapath/actions.c
> > +++ b/datapath/actions.c
> > @@ -276,7 +276,7 @@ static int set_mpls(struct sk_buff *skb, struct
> > sw_flow_key *flow_key,
> > }
> >
> > stack->label_stack_entry = lse;
> > - flow_key->mpls.top_lse = lse;
> > + flow_key->mpls.lse[0] = lse;
> > return 0;
> > }
> >
> > diff --git a/datapath/flow.c b/datapath/flow.c
> > index 916f7f4..6dc7402 100644
> > --- a/datapath/flow.c
> > +++ b/datapath/flow.c
> > @@ -659,27 +659,35 @@ static int key_extract_l3l4(struct sk_buff *skb,
> > struct sw_flow_key *key)
> > memset(&key->ipv4, 0, sizeof(key->ipv4));
> > }
> > } else if (eth_p_mpls(key->eth.type)) {
> > - size_t stack_len = MPLS_HLEN;
> > + u8 label_count = 1;
> >
> > + memset(&key->mpls, 0, sizeof(key->mpls));
> > skb_set_inner_network_header(skb, skb->mac_len);
> > while (1) {
> > __be32 lse;
> >
> > - error = check_header(skb, skb->mac_len + stack_len);
> > + error = check_header(skb, skb->mac_len +
> > + label_count * MPLS_HLEN);
> > if (unlikely(error))
> > return 0;
> >
> > memcpy(&lse, skb_inner_network_header(skb),
> > MPLS_HLEN);
> >
> > - if (stack_len == MPLS_HLEN)
> > - memcpy(&key->mpls.top_lse, &lse, MPLS_HLEN);
> > + if (label_count <= MPLS_LABEL_DEPTH)
> > + memcpy(&key->mpls.lse[label_count - 1], &lse,
> > +MPLS_HLEN);
> >
> > - skb_set_inner_network_header(skb, skb->mac_len +
> > stack_len);
> > + skb_set_inner_network_header(skb, skb->mac_len +
> > + label_count * MPLS_HLEN);
> > if (lse & htonl(MPLS_LS_S_MASK))
> > break;
> >
> > - stack_len += MPLS_HLEN;
> > + label_count++;
> > }
> > + if (label_count > MPLS_LABEL_DEPTH)
> > + label_count = MPLS_LABEL_DEPTH;
> > +
> > + key->mpls.num_labels_mask = GENMASK(label_count - 1, 0);
> > } else if (key->eth.type == htons(ETH_P_IPV6)) {
> > int nh_len; /* IPv6 Header + Extensions */
> >
> > diff --git a/datapath/flow.h b/datapath/flow.h
> > index 5560300..4ad5363 100644
> > --- a/datapath/flow.h
> > +++ b/datapath/flow.h
> > @@ -43,6 +43,7 @@ enum sw_flow_mac_proto {
> > MAC_PROTO_ETHERNET,
> > };
> > #define SW_FLOW_KEY_INVALID 0x80
> > +#define MPLS_LABEL_DEPTH 3
> >
> > /* Store options at the end of the array if they are less than the
> >* maximum size. This allows us to get the benefits of variable length
> > @@ -98,9 +99,6 @@ struct sw_flow_key {
> >*/
> > union {
> > struct {
> > - __be32 top_lse; /* top label stack entry */
> > - } mpls;
> > - struct {
> > u8 proto; /* IP protocol or lower 8 bits of ARP
> > opcode. */
> > u8 tos; /* IP ToS. */
> > u8 ttl; /* IP TTL/hop limit. */
> > @@ -148,6 +146,10 @@ struct sw_flow_key {
> > } nd;
> > };
> > } ipv6;
> > + struct {
> > + u32 num_labels_mask;/* labels present bitmap of
> > effective length MPLS_LABEL_DEPTH */
Re: [ovs-dev] [PATCH] ofproto: Fix crash on PACKET_OUT due to recursive locking after upcall.
Hi Ben, I was not able to reproduce this issue in ovs sandbox, but able to test this patch on local testbed which had this issue and the patch works. Thanks & Regards, Anil Kumar. -Original Message- From: Ilya Maximets Sent: Friday, 22 November, 2019 12:02 AM To: Ilya Maximets ; ovs-dev@openvswitch.org; Ben Pfaff Cc: Anil Kumar Koli Subject: Re: [PATCH] ofproto: Fix crash on PACKET_OUT due to recursive locking after upcall. On 01.11.2019 22:24, Ilya Maximets wrote: > Handling of OpenFlow PACKET_OUT implies pushing the packet through the > datapath and packet processing inside the datapath could trigger an > upcall. In case of system datapath, 'dpif_execute()' sends packet to > the kernel module and returns. If any, upcall will be triggered > inside the kernel and handled by a separate thread in userspace. > But in case of userspace datapath full processing of the packet > happens inside the 'dpif_execute()' in the same thread that handled > PACKET_OUT. > This causes an issue if upcall will lead to modification of flow rules. > For example, it could happen while processing of 'learn' actions. > Since whole handling of PACKET_OUT is protected by 'ofproto_mutex', > OVS will assert on attempt to take it recursively while processing > 'learn' actions: > >0 __GI_raise (sig=sig@entry=6) >1 __GI_abort () >2 ovs_abort_valist () >3 ovs_abort () >4 ovs_mutex_lock_at (where=where@entry=0xad4199 "ofproto/ofproto.c:5391") > >5 ofproto_flow_mod_learn () at ofproto/ofproto.c:5391 > >6 xlate_learn_action () at ofproto/ofproto-dpif-xlate.c:5378 > <'learn' action found> >7 do_xlate_actions () at ofproto/ofproto-dpif-xlate.c:6893 >8 xlate_recursively ()at ofproto/ofproto-dpif-xlate.c:4233 >9 xlate_table_action () at ofproto/ofproto-dpif-xlate.c:4361 > 10 in xlate_ofpact_resubmit () at ofproto/ofproto-dpif-xlate.c:4672 > 11 do_xlate_actions () at ofproto/ofproto-dpif-xlate.c:6773 > 12 xlate_actions ()at ofproto/ofproto-dpif-xlate.c:7570 > > 13 upcall_xlate () at ofproto/ofproto-dpif-upcall.c:1197 > 14 process_upcall () at ofproto/ofproto-dpif-upcall.c:1413 > 15 upcall_cb ()at ofproto/ofproto-dpif-upcall.c:1315 > 16 dp_netdev_upcall (DPIF_UC_MISS) at lib/dpif-netdev.c:6236 > > 17 handle_packet_upcall () at lib/dpif-netdev.c:6591 > 18 fast_path_processing () at lib/dpif-netdev.c:6709 > 19 dp_netdev_input__ ()at lib/dpif-netdev.c:6797 > 20 dp_netdev_recirculate ()at lib/dpif-netdev.c:6842 > 21 dp_execute_cb ()at lib/dpif-netdev.c:7158 > 22 odp_execute_actions () at lib/odp-execute.c:794 > 23 dp_netdev_execute_actions ()at lib/dpif-netdev.c:7332 > 24 dpif_netdev_execute () at lib/dpif-netdev.c:3725 > 25 dpif_netdev_operate () at lib/dpif-netdev.c:3756 > > 26 dpif_operate () at lib/dpif.c:1367 > 27 dpif_execute () at lib/dpif.c:1321 > 28 packet_execute () at ofproto/ofproto-dpif.c:4760 > 29 ofproto_packet_out_finish ()at ofproto/ofproto.c:3594 > > 30 handle_packet_out ()at ofproto/ofproto.c:3635 > 31 handle_single_part_openflow (OFPTYPE_PACKET_OUT) at > ofproto/ofproto.c:8400 > 32 handle_openflow () at > ofproto/ofproto.c:8587 > 33 ofconn_run () > 34 connmgr_run () > 35 ofproto_run () > 36 bridge_run__ () > 37 bridge_run () > 38 main () > > Fix that by splitting the 'ofproto_packet_out_finish()' in two parts. > First one that translates side-effects and requires holding 'ofproto_mutex' > and the second that only pushes packet to datapath. The second part > moved out of 'ofproto_mutex' because 'ofproto_mutex' is not required > and actually should not be taken in order to avoid recursive locking. > > Reported-by: Anil Kumar Koli > Reported-at: > https://mail.openvswitch.org/pipermail/ovs-discuss/2019-April/048494.h > tml > Signed-off-by: Ilya Maximets > --- > > Previous discussion about implementation: > https://mail.openvswitch.org/pipermail/ovs-dev/2019-August/362082.html > > I'm not able to reproduce the original issue, so I can not test if > this patch fixes it. > > ofproto/ofproto-dpif.c | 40 -- > ofproto/ofproto-provider.h | 12 +--- > ofproto/ofproto.c | 29 +++ > 3 files changed, 64 insertions(+), 17 deletions(-) Hi Ben, Could you, please, take a look at this patch? Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] flow: Fix IPv6 header parser with partial offloading.
On Fri, Nov 08, 2019 at 05:02:44PM +0800, Zhike Wang wrote: > Set new_proto before it is used in parse_ipv6_ext_hdrs__(). > > Signed-off-by: Zhike Wang That's embarrassing. I applied this to master and branch-2.12. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [bug-report] Check Python IDL reconnects to leader unit case failed on arm64
Hi Ben, Thanks for your reply. I checked the original email I sent. I did attach the logs in my previous email. However, as you said, they might be stripped automatically. At this time, I compress everything into a zip file. I am not sure whether it would be stripped again. So I also attach a link where you can find the related logs: https://1drv.ms/u/s!Atj0ulDEFFyaj1ds2QyEmPMkRM9s?e=Ifr7l9 . For convenience, I also copied the original email content as below in case that you did not receive the complete message: I encountered an issue when I was running testsuite on Arm. The system I tested on was an arm server running Ubuntu 18.04.3 LTS (GNU/Linux 4.15.0-65-generic aarch64). After I downloaded the up-to-date source code on master branch and built ovs, I run the testsuite with TESTSUITEFLAGS="2105" (Unit case name: Check Python IDL reconnects to leader) I run the unit case for many times, most of the time I got the following failure: make check TESTSUITEFLAGS="2105" RECHECK=yes make check-recursive make[1]: Entering directory '/home/lance/ovs' Making check in datapath make[2]: Entering directory '/home/lance/ovs/datapath' make[3]: Entering directory '/home/lance/ovs/datapath' make[3]: Leaving directory '/home/lance/ovs/datapath' make[2]: Leaving directory '/home/lance/ovs/datapath' make[2]: Entering directory '/home/lance/ovs' make[3]: Entering directory '/home/lance/ovs/datapath' make[3]: 'distfiles' is up to date. make[3]: Leaving directory '/home/lance/ovs/datapath' make utilities/ovs-appctl-bashcomp.bash utilities/ovs-vsctl-bashcomp.bash tests/atlocal make[3]: Entering directory '/home/lance/ovs' make[3]: Nothing to be done for 'utilities/ovs-appctl-bashcomp.bash'. make[3]: Nothing to be done for 'utilities/ovs-vsctl-bashcomp.bash'. make[3]: 'tests/atlocal' is up to date. make[3]: Leaving directory '/home/lance/ovs' make check-local make[3]: Entering directory '/home/lance/ovs' set /bin/bash './tests/testsuite' -C tests AUTOTEST_PATH=utilities:vswitchd:ovsdb:vtep:tests::; \ "$@" -j2 2104-2105 || (test X'' = Xyes && "$@" --recheck) ## --- ## ## openvswitch 2.12.90 test suite. ## ## --- ## 2105: Check Python IDL reconnects to leader - Python3 (leader only) FAILED (ovsdb-idl.at:1816) ## - ## ## Test results. ## ## - ## ERROR: All 1 test was run, 1 failed unexpectedly. ## -- ## ## testsuite.log was created. ## ## -- ## Please send `tests/testsuite.log' and all information you think might help: To: Subject: [openvswitch 2.12.90] testsuite: 2105 failed You may investigate any problem if you feel able to do so, in which case the test suite provides a good starting point. Its output may be found below `tests/testsuite.dir'. ## --- ## 2105: Check Python IDL reconnects to leader - Python3 (leader only) FAILED (ovsdb-idl.at:1816) ## - ## ## Test results. ## ## - ## ERROR: 1 test was run, 1 failed unexpectedly. ## -- ## ## testsuite.log was created. ## ## -- ## Please send `tests/testsuite.log' and all information you think might help: To: Subject: [openvswitch 2.12.90] testsuite: 2105 failed You may investigate any problem if you feel able to do so, in which case the test suite provides a good starting point. Its output may be found below `tests/testsuite.dir'. Makefile:6151: recipe for target 'check-local' failed make[3]: *** [check-local] Error 1 make[3]: Leaving directory '/home/lance/ovs' Makefile:5275: recipe for target 'check-am' failed make[2]: *** [check-am] Error 2 make[2]: Leaving directory '/home/lance/ovs' Makefile:4984: recipe for target 'check-recursive' failed make[1]: *** [check-recursive] Error 1 make[1]: Leaving directory '/home/lance/ovs' Makefile:5278: recipe for target 'check' failed make: *** [check] Error 2 I attached the tests/testsuite.dir/testsuite.log to this email. I also attached the logs for ovsdb. Any hints on what is wrong here? Thank you so much. Best regards, Lance Yang > -Original Message- > From: Ben Pfaff > Sent: Friday, November 22, 2019 9:10 AM > To: Lance Yang (Arm Technology China) > Cc: b...@openvswitch.org; ovs-dev@openvswitch.org; ted.elhour...@nutanix.com > Subject: Re: [ovs-dev] [bug-report] Check Python IDL reconnects to leader > unit case failed > on arm64 > > On Thu, Nov 21, 2019 at 07:54:09AM +, Lance Yang (Arm Technology China) > wrote: > > Hi, > > I encountered an issue when I was running testsuite on Arm. > > The system I tested on was an arm server running Ubuntu 18.04.3 LTS > > (GNU/Linux > 4.15.0-65-generic aarch64). > > After I downloaded the up-to-date source code on master branch and built > > ovs, I run the > testsuite with TESTSUITEFLAGS="2105" (Unit case name: Check Python IDL > reconnects to > leader) I run the unit case for many times, most of the time
Re: [ovs-dev] [PATCH] lacp: Add support to recognize LACP Marker RX PDUs.
On Tue, Nov 12, 2019 at 09:08:59AM +0100, Nitin katiyar via dev wrote: > OVS does not support the LACP Marker protocol. Typically, ToR switches > send a LACP Marker PDU when restarting LACP negotiation following a link > flap or LACP timeout. > > When a LACP Marker PDU is received, OVS logs the following warning and > drops the packet: > “lacp(pmdXXX)|WARN|bond-prv: received an unparsable LACP PDU.” > > As the above message is logged around the same time the link flap or > LACP down events are logged, it gives a misleading impression that the > reception of an unparsable LACP PDU is the reason for the LACP down > event. > > The proposed patch does not add support for the LACP Marker protocol. > It simply recognizes LACP Marker packets, drops them and logs a clear > message indicating that a Marker packet was a received. A counter to > track the number of such packets received is also added. > > Signed-off-by: Nitin katiyar Thanks a lot! I did not know about LACP Marker PDUs before. I applied this to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto-dpif: Allow IPv6 ND Extensions only if supported
On Wed, Nov 20, 2019 at 11:28:03AM -0300, Flavio Leitner wrote:
> On Wed, 20 Nov 2019 11:21:13 -0300
> Flavio Leitner wrote:
>
> > The IPv6 ND Extensions is only implemented in userspace datapath,
> > but nothing prevents that to be used with other datapaths.
> >
> > This patch probes the datapath and only allows if the support
> > is available.
> >
> > Fixes: 9b2b84973 ("Support for match & set ICMPv6 reserved and
> > options type fields") Signed-off-by: Flavio Leitner
> > ---
>
> This fix should be applied to branch-2.12 as well.
> The same patch applies today, otherwise let me know and I can send out
> the back-porting too.
I applied this to master and branch-2.12.
Since you posted this patch, William pushed commit 27501802d09f
("ofproto-dpif: Expose datapath capability to ovsdb.") that exposes
datapath capabilities via OVSDB and, more importantly, documents the
meanings of each of the capabilities. Would you mind sending a followup
patch to add documentation for this new capability?
Thanks,
Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/2] bridge: Move sec_in_state to port statistics from port status.
On Wed, Nov 20, 2019 at 05:54:28PM -0800, Krishna Kolakaluri wrote: > This commit moves the field sec_in_state field from port statistics to > port status structure.This helps in reducing the number of updates > sent to the controller, with the current mechanism everytime there is > a change in this field a notification is sent to the controller. For > this field its every second, since it's just counting the number of > secs its been in that particular state. By moving this under port > statistics, this can be controlled with the key > "stats-update-interval". > > Signed-off-by: Krishna Kolakaluri Please update the documentation in vswitch.xml to reflect the new location. The documentation should also mention that it was in the previous location in 2.12 and earlier. Please add an item to NEWS, also, since this is an incompatible change. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [bug-report] Check Python IDL reconnects to leader unit case failed on arm64
On Thu, Nov 21, 2019 at 07:54:09AM +, Lance Yang (Arm Technology China) wrote: > Hi, > I encountered an issue when I was running testsuite on Arm. > The system I tested on was an arm server running Ubuntu 18.04.3 LTS > (GNU/Linux 4.15.0-65-generic aarch64). > After I downloaded the up-to-date source code on master branch and built ovs, > I run the testsuite with TESTSUITEFLAGS="2105" (Unit case name: Check Python > IDL reconnects to leader) I run the unit case for many times, most of the > time I got the following failure: Thanks for the report. I don't see the attachment. Maybe you forgot to include it, or maybe the stripped it. Please try again? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dp-packet: Fix clearing/copying of memory layout flags.
On Thu, Nov 21, 2019 at 02:58:31PM +0100, Ilya Maximets wrote: > 'ol_flags' of DPDK mbuf could contain bits responsible for external > or indirect buffers which are not actually offload flags in a common > sense. Clearing/copying of these flags could lead to memory leaks of > external memory chunks and crashes due to access to wrong memory. > > OVS should not clear these flags while resetting offloads and also > should not copy them to the newly allocated packets. > > This change is required to support DPDK 19.11, as some drivers may > return mbufs with these flags set. However, it might be good to do > the same for DPDK 18.11, because these flags are present and should > be taken into account. > > Signed-off-by: Ilya Maximets This seems reasonable to me, but I don't have much context. With that in mind: Acked-by: Ben Pfaff I don't usually like "negative" names for things, like the "non-" infix, but it seems appropriate in this case. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] ovs container build: Make kernel module configurable
On Tue, Nov 12, 2019 at 12:47:58AM -0800, amgin...@gmail.com wrote: > -./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \ > +if [ $KERNEL_VERSION == "host" ]; then Supporting == in [] or "test" is a GNUism. Use = for wider support (including BSD). > + ./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \ > +--enable-ssl > +else > +./configure --localstatedir="/var" --sysconfdir="/etc" --prefix="/usr" \ > --with-linux=/lib/modules/$KERNEL_VERSION/build --enable-ssl > +fi There's a lot of duplication above, I'd suggest using variables to reduce the duplication. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ipf: bail out when ipf state is COMPLETED
On Thu, Nov 14, 2019 at 07:38:55AM -0800, Darrell Ball wrote:
> Hi RongQing/Wang
>
> On Thu, Nov 14, 2019 at 1:26 AM Li RongQing wrote:
>
> > it is easy to crash ovs when a packet with same id
> > hits a list that already reassembled completedly
> > but have not been sent out yet, and this packet is
> > not duplicate with this hit ipf list due to bigger
> > offset
> >
> > 1 0x7f9fef0ae2d9 in __GI_abort () at abort.c:89
> >
>
> Good DOS test.
> Fix is correct.
>
> This needs a 'Fixes' tag.
> Fixes: 4ea96698f667 ("Userspace datapath: Add fragmentation handling.")
>
> This will need to be backported to 2.10.
I pushed to master and branch-2.12. 2.11 and 2.10 need some kind of
conflict resolution.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Venture Capital & Private Investors
Attention: I am an investor that can provide funding for any viable business idea or venture. Please do let me know if you have fund management abilities, credible projects in need of funding or advanced stage projects requiring Bank Guarantees, Loans or Partnership, Joint Venture, Equity, we would be delighted to work with you. Best Regards, Mr. Joon-Kyu Lim Al Faisaliah Group (AFG) Venture Capital & Private Investors -- *This email and any attachments are intended for the named recipients only and contain confidential materials. Any unauthorized copying, reviewing, dissemination or other use by anyone other than the named recipients of this communication is strictly prohibited. If you received this email in error and/or are not a named recipient, please notify the sender (Al Faisaliah Group) and delete all copies of this email. Thank you. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1 4/4] travis: Enable OVS Travis CI for arm
On 20.11.2019 9:15, Lance Yang wrote: > Enable part of travis jobs with gcc compiler for arm64 architecture > > 1. Add arm jobs into the matrix in .travis.yml and define dpdk cache > directory for arm64 architecture. > 2. Temporarily disable sparse checker because of static code checking > failure on arm64 Could you provide the sparse error messages? Maybe we could fix them instead of disabling. > > Successful travis build jobs report: > https://travis-ci.org/yzyuestc/ovs/builds/614299933 There are some issues in Travis in above build. It seems that Travis fails to upload the resulted cache directory due to missing md5deep: "" store build cache 0.02s5.41sInstalling md5deep Reading package lists... Building dependency tree... Reading state information... md5deep is already the newest version (4.4-2). 0 upgraded, 0 newly installed, 0 to remove and 8 not upgraded. /home/travis/.casher/bin/casher: line 230: md5deep: command not found nothing changed "" Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1 3/4] travis: split cache and set target for
On 20.11.2019 9:15, Lance Yang wrote: > To compile OvS with DPDK in some Travis jobs, it is necessary to set the > build target and split the dpdk cache directory according to different CPU > architectures. > > Reviewed-by: Yanqin Wei > Reviewed-by: Malvika Gupta > Signed-off-by: Lance Yang > --- > .travis/linux-build.sh | 32 +--- > 1 file changed, 21 insertions(+), 11 deletions(-) According to this thread: https://travis-ci.community/t/no-cache-support-on-arm64/5416/20 The arch name should be included now in the cache key, so this should not be an issue. Could you, please, re-check? Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1 2/4] travis: Move x86-only addon packages
On 20.11.2019 9:14, Lance Yang wrote:
> To enable multiple CPU architectures support, it is necessary to move the
> x86-only addon packages from .travis.yml file. Otherwise, the x86-only
> addon packages will break the builds on some other CPU architectures.
>
> Reviewed-by: Yangqin Wei
> Reviewed-by: Malvika Gupta
> Reviewed-by: Gavin Hu
> Reviewed-by: Ruifeng Wang
> Signed-off-by: Lance Yang
> ---
> .travis.yml | 2 --
> .travis/linux-prepare.sh | 12
> 2 files changed, 8 insertions(+), 6 deletions(-)
Common comment for all the patches in a series:
* It's better to add a period in the end of a subject line.
>
> diff --git a/.travis.yml b/.travis.yml
> index 482efd2..2dc4d43 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -14,7 +14,6 @@ addons:
>apt:
> packages:
>- bc
> - - gcc-multilib
>- libssl-dev
>- llvm-dev
>- libjemalloc1
> @@ -26,7 +25,6 @@ addons:
>- libelf-dev
>- selinux-policy-dev
>- libunbound-dev
> - - libunbound-dev:i386
>- libunwind-dev
>
> before_install: ./.travis/${TRAVIS_OS_NAME}-prepare.sh
> diff --git a/.travis/linux-prepare.sh b/.travis/linux-prepare.sh
> index 9e3ac0d..8096abe 100755
> --- a/.travis/linux-prepare.sh
> +++ b/.travis/linux-prepare.sh
> @@ -15,10 +15,14 @@ cd ..
> pip install --disable-pip-version-check --user six flake8 hacking
> pip install --user --upgrade docutils
>
> -if [ "$M32" ]; then
> -# 32-bit and 64-bit libunwind can not be installed at the same time.
> -# This will remove the 64-bit libunwind and install 32-bit version.
> -sudo apt-get install -y libunwind-dev:i386
> +if [[ "$TRAVIS_ARCH" == "amd64" ]] || [[ -z "$TRAVIS_ARCH" ]]; then
The same comment here as for previous ppc64le patch.
Are you going to ever build 32bit binary on aarch64 on Travis?
Is it really possible to build 32bit binary on aarch64 with '-m32' flag?
> +if [ "$M32" ]; then
> +# 32-bit and 64-bit libunwind can not be installed at the same time.
> +# This will remove the 64-bit libunwind and install 32-bit version.
> +sudo apt-get install \
> +-y libunwind-dev:i386 libunbound-dev:i386 gcc-multilib
Please, add additional indentation level for above line.
> +fi
> +
> fi
>
> # IPv6 is supported by kernel but disabled in TravisCI images:
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1 1/4] dpif-netdev: Fix the pmd_perf_stats
On 20.11.2019 9:13, Lance Yang wrote: > When compiling Open vSwitch for aarch64, the compiler will warn about > an uninitailized variable in lib/dpif-netdev-perf.c. It is necessary to > initialize it to avoid build failure. Hi. Thanks for making OVS build on aarch64. Could you provide the error message? In fact this variable is never used (only assigned), so this should be a false-positive. So, I'd like to look at the error message. Also, it might be better to rename the patch to something more specific. e.g. 'dpif-netdev-perf: Avoid false-positive on uninitialized perf stats.' > > Reviewed-by: Yanqin Wei > Signed-off-by: Lance Yang > --- > lib/dpif-netdev-perf.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/lib/dpif-netdev-perf.c b/lib/dpif-netdev-perf.c > index baf90b0..f85bb0c 100644 > --- a/lib/dpif-netdev-perf.c > +++ b/lib/dpif-netdev-perf.c > @@ -63,6 +63,7 @@ pmd_perf_estimate_tsc_frequency(void) > struct pmd_perf_stats s; > uint64_t start, stop; > > +memset(&s, 0, sizeof(s)); Please, don't parenthesize the argument of a 'sizeof'. Also, it might be better to initialize the variable close to the first usage. It doesn't look good here. > /* DPDK is not available or returned unreliable value. > * Trying to estimate. */ > affinity = ovs_numa_thread_getaffinity_dump(); > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ofproto-dpif: Refactor the get capability code.
Make the code simpler by removing the use of
xasprintf and free, and use smap_add_format.
Cc: Ben Pfaff
Signed-off-by: William Tu
---
ofproto/ofproto-dpif.c | 18 --
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index fa73d06a82f4..e55c46964279 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5447,7 +5447,6 @@ ct_del_zone_timeout_policy(const char *datapath_type,
uint16_t zone_id)
static void
get_datapath_cap(const char *datapath_type, struct smap *cap)
{
-char *str_value;
struct odp_support odp;
struct dpif_backer_support s;
struct dpif_backer *backer = shash_find_data(&all_dpif_backers,
@@ -5459,14 +5458,9 @@ get_datapath_cap(const char *datapath_type, struct smap
*cap)
odp = s.odp;
/* ODP_SUPPORT_FIELDS */
-str_value = xasprintf("%"PRIuSIZE, odp.max_vlan_headers);
-smap_add(cap, "max_vlan_headers", str_value);
-free(str_value);
-
-str_value = xasprintf("%"PRIuSIZE, odp.max_mpls_depth);
-smap_add(cap, "max_mpls_depth", str_value);
-free(str_value);
-
+smap_add_format(cap, "max_vlan_headers", "%"PRIuSIZE,
+odp.max_vlan_headers);
+smap_add_format(cap, "max_mpls_depth", "%"PRIuSIZE, odp.max_mpls_depth);
smap_add(cap, "recirc", odp.recirc ? "true" : "false");
smap_add(cap, "ct_state", odp.ct_state ? "true" : "false");
smap_add(cap, "ct_zone", odp.ct_zone ? "true" : "false");
@@ -5485,11 +5479,7 @@ get_datapath_cap(const char *datapath_type, struct smap
*cap)
smap_add(cap, "sample_nesting", s.sample_nesting ? "true" : "false");
smap_add(cap, "ct_eventmask", s.ct_eventmask ? "true" : "false");
smap_add(cap, "ct_clear", s.ct_clear ? "true" : "false");
-
-str_value = xasprintf("%"PRIuSIZE, s.max_hash_alg);
-smap_add(cap, "max_hash_alg", str_value);
-free(str_value);
-
+smap_add_format(cap, "max_hash_alg", "%"PRIuSIZE, s.max_hash_alg);
smap_add(cap, "check_pkt_len", s.check_pkt_len ? "true" : "false");
smap_add(cap, "ct_timeout", s.ct_timeout ? "true" : "false");
}
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb raft: Fix election timer parsing in snapshot RPC.
On Thu, Nov 21, 2019 at 10:57:41AM -0800, Han Zhou wrote: > On Thu, Nov 21, 2019 at 10:54 AM Ben Pfaff wrote: > > > > On Wed, Nov 13, 2019 at 09:33:59AM -0800, Han Zhou wrote: > > > Commit a76ba825 took care of saving and restoring election timer in > > > file header snapshot, but it didn't handle the parsing of election > > > timer in install_snapshot_request/reply RPC, which results in problems, > > > e.g. when election timer change log is compacted in snapshot and then a > > > new node join the cluster, the new node will use the default timer > > > instead of the new value. This patch fixed it by parsing election > > > timer in snapshot RPC. > > > > > > At the same time the patch updates the test case to cover the DB > compact and > > > join senario. The test reveals another 2 problems related to clustered > DB > > > compact, as commented in the test case's XXX, which need to be addressed > > > separately. > > > > > > Signed-off-by: Han Zhou > > > > I applied this to master. Thank you! > > Thanks Ben! Could you help backport to 2.12? Done. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb raft: Fix election timer parsing in snapshot RPC.
On Thu, Nov 21, 2019 at 10:54 AM Ben Pfaff wrote: > > On Wed, Nov 13, 2019 at 09:33:59AM -0800, Han Zhou wrote: > > Commit a76ba825 took care of saving and restoring election timer in > > file header snapshot, but it didn't handle the parsing of election > > timer in install_snapshot_request/reply RPC, which results in problems, > > e.g. when election timer change log is compacted in snapshot and then a > > new node join the cluster, the new node will use the default timer > > instead of the new value. This patch fixed it by parsing election > > timer in snapshot RPC. > > > > At the same time the patch updates the test case to cover the DB compact and > > join senario. The test reveals another 2 problems related to clustered DB > > compact, as commented in the test case's XXX, which need to be addressed > > separately. > > > > Signed-off-by: Han Zhou > > I applied this to master. Thank you! Thanks Ben! Could you help backport to 2.12? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb raft: Fix election timer parsing in snapshot RPC.
On Wed, Nov 13, 2019 at 09:33:59AM -0800, Han Zhou wrote: > Commit a76ba825 took care of saving and restoring election timer in > file header snapshot, but it didn't handle the parsing of election > timer in install_snapshot_request/reply RPC, which results in problems, > e.g. when election timer change log is compacted in snapshot and then a > new node join the cluster, the new node will use the default timer > instead of the new value. This patch fixed it by parsing election > timer in snapshot RPC. > > At the same time the patch updates the test case to cover the DB compact and > join senario. The test reveals another 2 problems related to clustered DB > compact, as commented in the test case's XXX, which need to be addressed > separately. > > Signed-off-by: Han Zhou I applied this to master. Thank you! ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] jsonrpc: increase input buffer size from 512 to 4096
On Thu, Nov 21, 2019 at 10:46:59AM -0800, Ben Pfaff wrote: > On Wed, Nov 06, 2019 at 11:19:44AM +0200, Lorenzo Bianconi wrote: > > Increase jsonrpc input buffer size from 512 to 4096 bytes in order to > > reduce the syscall overhead when downloading huge db size > > > > Signed-off-by: Lorenzo Bianconi > > Applied to master, thanks! And branch-2.12. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] jsonrpc: increase input buffer size from 512 to 4096
On Wed, Nov 06, 2019 at 11:19:44AM +0200, Lorenzo Bianconi wrote: > Increase jsonrpc input buffer size from 512 to 4096 bytes in order to > reduce the syscall overhead when downloading huge db size > > Signed-off-by: Lorenzo Bianconi Applied to master, thanks! ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-detrace: Decode OVS Interfaces from ofproto/trace dumps.
On Thu, Nov 21, 2019 at 7:22 PM Mark Michelson wrote:
>
> I pushed this to master.
Thanks!
>
> On 11/15/19 8:39 PM, Mark Michelson wrote:
> > Acked-by: Mark Michelson
> >
> > On 11/15/19 2:24 PM, Dumitru Ceara wrote:
> >> Two new command line arguments are added to ovn-detrace:
> >> - "--ovs": which will instruct ovn-detrace to connect to the local OVS
> >>db.sock and try to decode input/output ofport values and pretty print
> >>the corresponding OVS interfaces.
> >> - "--ovsdb": which allows the user to point ovn-detrace to other OVS DB
> >>remotes (useful when ovn-trace is not run on the hypervisor where
> >>ofproto-trace was run.
> >>
> >> Signed-off-by: Dumitru Ceara
> >> ---
> >> utilities/ovn-detrace.1.in | 11 +
> >> utilities/ovn-detrace.in | 105
> >> +
> >> 2 files changed, 88 insertions(+), 28 deletions(-)
> >>
> >> diff --git a/utilities/ovn-detrace.1.in b/utilities/ovn-detrace.1.in
> >> index 2f662d4..b899b63 100644
> >> --- a/utilities/ovn-detrace.1.in
> >> +++ b/utilities/ovn-detrace.1.in
> >> @@ -33,6 +33,17 @@ Otherwise, the default is
> >> \fBunix:@RUNDIR@/ovnnb_db.sock\fR, but this
> >> default is unlikely to be useful outside of single-machine OVN test
> >> environments.
> >> .
> >> +.IP "\fB\-\-ovs=\fR"
> >> +Also decode flow information (like OVS ofport) from the flows by
> >> connecting
> >> +to the OVS DB.
> >> +.
> >> +.IP "\fB\-\-ovsdb=\fIserver\fR"
> >> +The OVS DB remote to contact if \fB\-\-ovs\f is present. If the
> >> +\fBOVS_RUNDIR\fR environment variable is set, its value is used as the
> >> +default. Otherwise, the default is \fBunix:@RUNDIR@/db.sock\fR, but this
> >> +default is unlikely to be useful outside of single-machine OVN test
> >> +environments.
> >> +.
> >> .SH "SEE ALSO"
> >> .
> >> .BR ovs\-appctl (8), ovn\-sbctl (8), ovn-\-nbctl (8), ovn\-trace (8)
> >> diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in
> >> index c645658..52f6f4f 100755
> >> --- a/utilities/ovn-detrace.in
> >> +++ b/utilities/ovn-detrace.in
> >> @@ -28,6 +28,7 @@ try:
> >> from ovs import jsonrpc
> >> from ovs.poller import Poller
> >> from ovs.stream import Stream
> >> +from ovs import dirs
> >> except Exception:
> >> print("ERROR: Please install the correct Open vSwitch python
> >> support")
> >> print(" libraries (@VERSION@).")
> >> @@ -49,7 +50,8 @@ The following options are also available:
> >> -h, --help display this help message
> >> -V, --version display version information
> >> --ovnsb=DATABASEuse DATABASE as southbound DB
> >> - --ovnnb=DATABASEuse DATABASE as northbound DB\
> >> + --ovnnb=DATABASEuse DATABASE as northbound DB
> >> + --ovsdb=DATABASEuse DATABASE as OVS DB\
> >> """ % {'argv0': argv0})
> >> sys.exit(0)
> >> @@ -102,15 +104,15 @@ class OVSDB(object):
> >> def get_table(self, table_name):
> >> return self._idl_conn.tables[table_name]
> >> -def _find_row(self, table_name, find_fn):
> >> +def _find_rows(self, table_name, find_fn):
> >> return filter(find_fn,
> >> self.get_table(table_name).rows.values())
> >> def _find_rows_by_name(self, table_name, value):
> >> -return self._find_row(table_name, lambda row: row.name == value)
> >> +return self._find_rows(table_name, lambda row: row.name ==
> >> value)
> >> def find_rows_by_partial_uuid(self, table_name, value):
> >> -return self._find_row(table_name,
> >> - lambda row:
> >> str(row.uuid).startswith(value))
> >> +return self._find_rows(table_name,
> >> + lambda row:
> >> str(row.uuid).startswith(value))
> >> class CookieHandler(object):
> >> def __init__(self, db, table):
> >> @@ -118,9 +120,7 @@ class CookieHandler(object):
> >> self._table = table
> >> def get_records(self, cookie):
> >> -# Adjust cookie to include leading zeroes if needed.
> >> -cookie = cookie.zfill(8)
> >> -return self._db.find_rows_by_partial_uuid(self._table, cookie)
> >> +return []
> >> def print_record(self, record):
> >> pass
> >> @@ -128,7 +128,16 @@ class CookieHandler(object):
> >> def print_hint(self, record, db):
> >> pass
> >> -class LogicalFlowHandler(CookieHandler):
> >> +class CookieHandlerByUUUID(CookieHandler):
> >> +def __init__(self, db, table):
> >> +super(CookieHandlerByUUUID, self).__init__(db, table)
> >> +
> >> +def get_records(self, cookie):
> >> +# Adjust cookie to include leading zeroes if needed.
> >> +cookie = cookie.zfill(8)
> >> +return self._db.find_rows_by_partial_uuid(self._table, cookie)
> >> +
> >> +class LogicalFlowHandler(CookieHandlerByUUUID):
> >> def __init__(self, ovnsb_db):
> >> super(Logi
Re: [ovs-dev] [PATCH] ofproto: Fix crash on PACKET_OUT due to recursive locking after upcall.
On 01.11.2019 22:24, Ilya Maximets wrote: > Handling of OpenFlow PACKET_OUT implies pushing the packet through > the datapath and packet processing inside the datapath could trigger > an upcall. In case of system datapath, 'dpif_execute()' sends packet > to the kernel module and returns. If any, upcall will be triggered > inside the kernel and handled by a separate thread in userspace. > But in case of userspace datapath full processing of the packet happens > inside the 'dpif_execute()' in the same thread that handled PACKET_OUT. > This causes an issue if upcall will lead to modification of flow rules. > For example, it could happen while processing of 'learn' actions. > Since whole handling of PACKET_OUT is protected by 'ofproto_mutex', > OVS will assert on attempt to take it recursively while processing > 'learn' actions: > >0 __GI_raise (sig=sig@entry=6) >1 __GI_abort () >2 ovs_abort_valist () >3 ovs_abort () >4 ovs_mutex_lock_at (where=where@entry=0xad4199 "ofproto/ofproto.c:5391") > >5 ofproto_flow_mod_learn () at ofproto/ofproto.c:5391 > >6 xlate_learn_action () at ofproto/ofproto-dpif-xlate.c:5378 > <'learn' action found> >7 do_xlate_actions () at ofproto/ofproto-dpif-xlate.c:6893 >8 xlate_recursively ()at ofproto/ofproto-dpif-xlate.c:4233 >9 xlate_table_action () at ofproto/ofproto-dpif-xlate.c:4361 > 10 in xlate_ofpact_resubmit () at ofproto/ofproto-dpif-xlate.c:4672 > 11 do_xlate_actions () at ofproto/ofproto-dpif-xlate.c:6773 > 12 xlate_actions ()at ofproto/ofproto-dpif-xlate.c:7570 > > 13 upcall_xlate () at ofproto/ofproto-dpif-upcall.c:1197 > 14 process_upcall () at ofproto/ofproto-dpif-upcall.c:1413 > 15 upcall_cb ()at ofproto/ofproto-dpif-upcall.c:1315 > 16 dp_netdev_upcall (DPIF_UC_MISS) at lib/dpif-netdev.c:6236 > > 17 handle_packet_upcall () at lib/dpif-netdev.c:6591 > 18 fast_path_processing () at lib/dpif-netdev.c:6709 > 19 dp_netdev_input__ ()at lib/dpif-netdev.c:6797 > 20 dp_netdev_recirculate ()at lib/dpif-netdev.c:6842 > 21 dp_execute_cb ()at lib/dpif-netdev.c:7158 > 22 odp_execute_actions () at lib/odp-execute.c:794 > 23 dp_netdev_execute_actions ()at lib/dpif-netdev.c:7332 > 24 dpif_netdev_execute () at lib/dpif-netdev.c:3725 > 25 dpif_netdev_operate () at lib/dpif-netdev.c:3756 > > 26 dpif_operate () at lib/dpif.c:1367 > 27 dpif_execute () at lib/dpif.c:1321 > 28 packet_execute () at ofproto/ofproto-dpif.c:4760 > 29 ofproto_packet_out_finish ()at ofproto/ofproto.c:3594 > > 30 handle_packet_out ()at ofproto/ofproto.c:3635 > 31 handle_single_part_openflow (OFPTYPE_PACKET_OUT) at > ofproto/ofproto.c:8400 > 32 handle_openflow () at > ofproto/ofproto.c:8587 > 33 ofconn_run () > 34 connmgr_run () > 35 ofproto_run () > 36 bridge_run__ () > 37 bridge_run () > 38 main () > > Fix that by splitting the 'ofproto_packet_out_finish()' in two parts. > First one that translates side-effects and requires holding 'ofproto_mutex' > and the second that only pushes packet to datapath. The second part moved > out of 'ofproto_mutex' because 'ofproto_mutex' is not required and actually > should not be taken in order to avoid recursive locking. > > Reported-by: Anil Kumar Koli > Reported-at: > https://mail.openvswitch.org/pipermail/ovs-discuss/2019-April/048494.html > Signed-off-by: Ilya Maximets > --- > > Previous discussion about implementation: > https://mail.openvswitch.org/pipermail/ovs-dev/2019-August/362082.html > > I'm not able to reproduce the original issue, so I can not test if > this patch fixes it. > > ofproto/ofproto-dpif.c | 40 -- > ofproto/ofproto-provider.h | 12 +--- > ofproto/ofproto.c | 29 +++ > 3 files changed, 64 insertions(+), 17 deletions(-) Hi Ben, Could you, please, take a look at this patch? Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-detrace: Decode OVS Interfaces from ofproto/trace dumps.
I pushed this to master.
On 11/15/19 8:39 PM, Mark Michelson wrote:
Acked-by: Mark Michelson
On 11/15/19 2:24 PM, Dumitru Ceara wrote:
Two new command line arguments are added to ovn-detrace:
- "--ovs": which will instruct ovn-detrace to connect to the local OVS
db.sock and try to decode input/output ofport values and pretty print
the corresponding OVS interfaces.
- "--ovsdb": which allows the user to point ovn-detrace to other OVS DB
remotes (useful when ovn-trace is not run on the hypervisor where
ofproto-trace was run.
Signed-off-by: Dumitru Ceara
---
utilities/ovn-detrace.1.in | 11 +
utilities/ovn-detrace.in | 105
+
2 files changed, 88 insertions(+), 28 deletions(-)
diff --git a/utilities/ovn-detrace.1.in b/utilities/ovn-detrace.1.in
index 2f662d4..b899b63 100644
--- a/utilities/ovn-detrace.1.in
+++ b/utilities/ovn-detrace.1.in
@@ -33,6 +33,17 @@ Otherwise, the default is
\fBunix:@RUNDIR@/ovnnb_db.sock\fR, but this
default is unlikely to be useful outside of single-machine OVN test
environments.
.
+.IP "\fB\-\-ovs=\fR"
+Also decode flow information (like OVS ofport) from the flows by
connecting
+to the OVS DB.
+.
+.IP "\fB\-\-ovsdb=\fIserver\fR"
+The OVS DB remote to contact if \fB\-\-ovs\f is present. If the
+\fBOVS_RUNDIR\fR environment variable is set, its value is used as the
+default. Otherwise, the default is \fBunix:@RUNDIR@/db.sock\fR, but this
+default is unlikely to be useful outside of single-machine OVN test
+environments.
+.
.SH "SEE ALSO"
.
.BR ovs\-appctl (8), ovn\-sbctl (8), ovn-\-nbctl (8), ovn\-trace (8)
diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in
index c645658..52f6f4f 100755
--- a/utilities/ovn-detrace.in
+++ b/utilities/ovn-detrace.in
@@ -28,6 +28,7 @@ try:
from ovs import jsonrpc
from ovs.poller import Poller
from ovs.stream import Stream
+ from ovs import dirs
except Exception:
print("ERROR: Please install the correct Open vSwitch python
support")
print(" libraries (@VERSION@).")
@@ -49,7 +50,8 @@ The following options are also available:
-h, --help display this help message
-V, --version display version information
--ovnsb=DATABASE use DATABASE as southbound DB
- --ovnnb=DATABASE use DATABASE as northbound DB\
+ --ovnnb=DATABASE use DATABASE as northbound DB
+ --ovsdb=DATABASE use DATABASE as OVS DB\
""" % {'argv0': argv0})
sys.exit(0)
@@ -102,15 +104,15 @@ class OVSDB(object):
def get_table(self, table_name):
return self._idl_conn.tables[table_name]
- def _find_row(self, table_name, find_fn):
+ def _find_rows(self, table_name, find_fn):
return filter(find_fn,
self.get_table(table_name).rows.values())
def _find_rows_by_name(self, table_name, value):
- return self._find_row(table_name, lambda row: row.name == value)
+ return self._find_rows(table_name, lambda row: row.name ==
value)
def find_rows_by_partial_uuid(self, table_name, value):
- return self._find_row(table_name,
- lambda row:
str(row.uuid).startswith(value))
+ return self._find_rows(table_name,
+ lambda row:
str(row.uuid).startswith(value))
class CookieHandler(object):
def __init__(self, db, table):
@@ -118,9 +120,7 @@ class CookieHandler(object):
self._table = table
def get_records(self, cookie):
- # Adjust cookie to include leading zeroes if needed.
- cookie = cookie.zfill(8)
- return self._db.find_rows_by_partial_uuid(self._table, cookie)
+ return []
def print_record(self, record):
pass
@@ -128,7 +128,16 @@ class CookieHandler(object):
def print_hint(self, record, db):
pass
-class LogicalFlowHandler(CookieHandler):
+class CookieHandlerByUUUID(CookieHandler):
+ def __init__(self, db, table):
+ super(CookieHandlerByUUUID, self).__init__(db, table)
+
+ def get_records(self, cookie):
+ # Adjust cookie to include leading zeroes if needed.
+ cookie = cookie.zfill(8)
+ return self._db.find_rows_by_partial_uuid(self._table, cookie)
+
+class LogicalFlowHandler(CookieHandlerByUUUID):
def __init__(self, ovnsb_db):
super(LogicalFlowHandler, self).__init__(ovnsb_db,
'Logical_Flow')
@@ -163,7 +172,7 @@ class LogicalFlowHandler(CookieHandler):
output += ' (log)'
print_h(output)
-class PortBindingHandler(CookieHandler):
+class PortBindingHandler(CookieHandlerByUUUID):
def __init__(self, ovnsb_db):
super(PortBindingHandler, self).__init__(ovnsb_db,
'Port_Binding')
@@ -173,7 +182,7 @@ class PortBindingHandler(CookieHandler):
(pb.logical_port, pb.tunnel_key,
chassis_str(pb.chassis)))
-class MacBindingHandler(C
Re: [ovs-dev] [PATCH v2] Change in openvswitch kernel module to support MPLS label depth of 3 in ingress direction.
On 11/21/2019 7:49 AM, Martin Varghese wrote:
From: Martin Varghese
The openvswitch kernel module was supporting a MPLS label depth of 1
in the ingress direction though the userspace OVS supports a max depth
of 3 labels. This change enables openvswitch module to support a max
depth of 3 labels in the ingress.
Signed-off-by: Martin Varghese
Did you submit this patch upstream as well?
- Greg
---
Changes in v2
- support added for nested actions.
datapath/actions.c | 2 +-
datapath/flow.c | 20
datapath/flow.h | 8 +++--
datapath/flow_netlink.c | 85 -
tests/system-traffic.at | 39 +++
5 files changed, 122 insertions(+), 32 deletions(-)
diff --git a/datapath/actions.c b/datapath/actions.c
index a44e804..fbf4457 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -276,7 +276,7 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key
*flow_key,
}
stack->label_stack_entry = lse;
- flow_key->mpls.top_lse = lse;
+ flow_key->mpls.lse[0] = lse;
return 0;
}
diff --git a/datapath/flow.c b/datapath/flow.c
index 916f7f4..6dc7402 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -659,27 +659,35 @@ static int key_extract_l3l4(struct sk_buff *skb, struct
sw_flow_key *key)
memset(&key->ipv4, 0, sizeof(key->ipv4));
}
} else if (eth_p_mpls(key->eth.type)) {
- size_t stack_len = MPLS_HLEN;
+ u8 label_count = 1;
+ memset(&key->mpls, 0, sizeof(key->mpls));
skb_set_inner_network_header(skb, skb->mac_len);
while (1) {
__be32 lse;
- error = check_header(skb, skb->mac_len + stack_len);
+ error = check_header(skb, skb->mac_len +
+label_count * MPLS_HLEN);
if (unlikely(error))
return 0;
memcpy(&lse, skb_inner_network_header(skb), MPLS_HLEN);
- if (stack_len == MPLS_HLEN)
- memcpy(&key->mpls.top_lse, &lse, MPLS_HLEN);
+ if (label_count <= MPLS_LABEL_DEPTH)
+ memcpy(&key->mpls.lse[label_count - 1], &lse,
+ MPLS_HLEN);
- skb_set_inner_network_header(skb, skb->mac_len + stack_len);
+ skb_set_inner_network_header(skb, skb->mac_len +
+label_count * MPLS_HLEN);
if (lse & htonl(MPLS_LS_S_MASK))
break;
- stack_len += MPLS_HLEN;
+ label_count++;
}
+ if (label_count > MPLS_LABEL_DEPTH)
+ label_count = MPLS_LABEL_DEPTH;
+
+ key->mpls.num_labels_mask = GENMASK(label_count - 1, 0);
} else if (key->eth.type == htons(ETH_P_IPV6)) {
int nh_len; /* IPv6 Header + Extensions */
diff --git a/datapath/flow.h b/datapath/flow.h
index 5560300..4ad5363 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -43,6 +43,7 @@ enum sw_flow_mac_proto {
MAC_PROTO_ETHERNET,
};
#define SW_FLOW_KEY_INVALID 0x80
+#define MPLS_LABEL_DEPTH 3
/* Store options at the end of the array if they are less than the
* maximum size. This allows us to get the benefits of variable length
@@ -98,9 +99,6 @@ struct sw_flow_key {
*/
union {
struct {
- __be32 top_lse; /* top label stack entry */
- } mpls;
- struct {
u8 proto; /* IP protocol or lower 8 bits of ARP
opcode. */
u8 tos; /* IP ToS. */
u8 ttl; /* IP TTL/hop limit. */
@@ -148,6 +146,10 @@ struct sw_flow_key {
} nd;
};
} ipv6;
+ struct {
+ u32 num_labels_mask;/* labels present bitmap of
effective length MPLS_LABEL_DEPTH */
+ __be32 lse[MPLS_LABEL_DEPTH]; /* label stack entry
*/
+ } mpls;
struct ovs_key_nsh nsh; /* network service header */
};
struct {
diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index 35f13d7..9fc1a19 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -438,7 +438,7 @@ static const struct ovs_len_tbl
ovs_key_lens[OVS_KEY_ATTR_MAX + 1] = {
[OVS_KEY_ATTR_DP_HASH] = { .len = sizeof(u32) },
[OVS_KEY_ATTR_TUNNEL]= { .len = OVS_ATTR_NESTED,
.next = ovs_tunnel_key_lens, },
- [OVS_KEY_ATTR_MPLS] = { .len = sizeof(struct ovs_key_mpls) },
+
Re: [ovs-dev] [PATCH v2] Change in openvswitch kernel module to support MPLS label depth of 3 in ingress direction.
On 11/21/2019 7:49 AM, Martin Varghese wrote:
From: Martin Varghese
The openvswitch kernel module was supporting a MPLS label depth of 1
in the ingress direction though the userspace OVS supports a max depth
of 3 labels. This change enables openvswitch module to support a max
depth of 3 labels in the ingress.
Signed-off-by: Martin Varghese
---
Changes in v2
- support added for nested actions.
Thanks Martin,
I reviewed the code and it looks fine to me. I also ran it through
Travis CI and check-kmod and that's all good too.
Tested-by: Greg Rose
Reviewed-by: Greg Rose
datapath/actions.c | 2 +-
datapath/flow.c | 20
datapath/flow.h | 8 +++--
datapath/flow_netlink.c | 85 -
tests/system-traffic.at | 39 +++
5 files changed, 122 insertions(+), 32 deletions(-)
diff --git a/datapath/actions.c b/datapath/actions.c
index a44e804..fbf4457 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -276,7 +276,7 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key
*flow_key,
}
stack->label_stack_entry = lse;
- flow_key->mpls.top_lse = lse;
+ flow_key->mpls.lse[0] = lse;
return 0;
}
diff --git a/datapath/flow.c b/datapath/flow.c
index 916f7f4..6dc7402 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -659,27 +659,35 @@ static int key_extract_l3l4(struct sk_buff *skb, struct
sw_flow_key *key)
memset(&key->ipv4, 0, sizeof(key->ipv4));
}
} else if (eth_p_mpls(key->eth.type)) {
- size_t stack_len = MPLS_HLEN;
+ u8 label_count = 1;
+ memset(&key->mpls, 0, sizeof(key->mpls));
skb_set_inner_network_header(skb, skb->mac_len);
while (1) {
__be32 lse;
- error = check_header(skb, skb->mac_len + stack_len);
+ error = check_header(skb, skb->mac_len +
+label_count * MPLS_HLEN);
if (unlikely(error))
return 0;
memcpy(&lse, skb_inner_network_header(skb), MPLS_HLEN);
- if (stack_len == MPLS_HLEN)
- memcpy(&key->mpls.top_lse, &lse, MPLS_HLEN);
+ if (label_count <= MPLS_LABEL_DEPTH)
+ memcpy(&key->mpls.lse[label_count - 1], &lse,
+ MPLS_HLEN);
- skb_set_inner_network_header(skb, skb->mac_len + stack_len);
+ skb_set_inner_network_header(skb, skb->mac_len +
+label_count * MPLS_HLEN);
if (lse & htonl(MPLS_LS_S_MASK))
break;
- stack_len += MPLS_HLEN;
+ label_count++;
}
+ if (label_count > MPLS_LABEL_DEPTH)
+ label_count = MPLS_LABEL_DEPTH;
+
+ key->mpls.num_labels_mask = GENMASK(label_count - 1, 0);
} else if (key->eth.type == htons(ETH_P_IPV6)) {
int nh_len; /* IPv6 Header + Extensions */
diff --git a/datapath/flow.h b/datapath/flow.h
index 5560300..4ad5363 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -43,6 +43,7 @@ enum sw_flow_mac_proto {
MAC_PROTO_ETHERNET,
};
#define SW_FLOW_KEY_INVALID 0x80
+#define MPLS_LABEL_DEPTH 3
/* Store options at the end of the array if they are less than the
* maximum size. This allows us to get the benefits of variable length
@@ -98,9 +99,6 @@ struct sw_flow_key {
*/
union {
struct {
- __be32 top_lse; /* top label stack entry */
- } mpls;
- struct {
u8 proto; /* IP protocol or lower 8 bits of ARP
opcode. */
u8 tos; /* IP ToS. */
u8 ttl; /* IP TTL/hop limit. */
@@ -148,6 +146,10 @@ struct sw_flow_key {
} nd;
};
} ipv6;
+ struct {
+ u32 num_labels_mask;/* labels present bitmap of
effective length MPLS_LABEL_DEPTH */
+ __be32 lse[MPLS_LABEL_DEPTH]; /* label stack entry
*/
+ } mpls;
struct ovs_key_nsh nsh; /* network service header */
};
struct {
diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index 35f13d7..9fc1a19 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -438,7 +438,7 @@ static const struct ovs_len_tbl
ovs_key_lens[OVS_KEY_ATTR_MAX + 1] = {
[OVS_KEY_ATTR_DP_HASH] = { .len = sizeof(u32) },
[OVS_KEY_ATTR_TUNNEL]= { .len = OVS_ATTR_NESTED,
Re: [ovs-dev] [PATCH v1 0/4] Fix issues and enable Travis CI on arm
Hi Lance. Thanks for sending all the patches. In the future, please just send them to either d...@openvswitch.org or ovs-dev@openvswitch.org, not both. They are the same list. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCHv4] ofproto-dpif: Expose datapath capability to ovsdb.
On Wed, Nov 20, 2019 at 05:20:35PM -0800, William Tu wrote:
> On Wed, Nov 20, 2019 at 11:25:12AM -0800, Ben Pfaff wrote:
> > On Wed, Nov 06, 2019 at 12:35:49PM -0800, William Tu wrote:
> > > On Tue, Nov 05, 2019 at 04:49:10PM -0800, Ben Pfaff wrote:
> > > > On Tue, Nov 05, 2019 at 02:57:46PM -0800, William Tu wrote:
> > > > > +/* ODP_SUPPORT_FIELDS */
> > > > > +str_value = xasprintf("%"PRIuSIZE, odp.max_vlan_headers);
> > > > > +smap_add(cap, "max_vlan_headers", str_value);
> > > > > +free(str_value);
> > > >
> > > > I think that you can shorten the above to:
> > > > smap_add_format(cap, "max_vlan_headers", "%"PRIuSIZE,
> > > > odp.max_vlan_headers);
> > > > and similarly for other cases.
Hi Ben,
I forgot to add above fixes.
I will send out another patch, sorry about that.
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [bug-report] bfd decay unit case failure
On Thu, Nov 21, 2019 at 09:25:49AM +, Lance Yang (Arm Technology China) wrote: > I encountered a unit test failure when I ran the Open vSwitch testsuite on > Travis CI aarch64 lxd container. > The environment can be found at line 7 : build system information section in > the report on https://travis-ci.org/yzyuestc/ovs/jobs/614941322 . The unit > test case name is "bfd decay" , you can find the unit test failure details in > the report after line 6520. The failure is not 100% reproducible on Travis CI. > Could anyone give some hint on what is wrong for this unit test case? We do our best to make the Open vSwitch test cases resist differences in timing from one environment to another, but there still may be some that are sensitive to it. This one may be such a test case. These problems can be difficult to debug without being able to trigger them in interactive environments. Have you been able to see it when you build by hand? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto-dpif: Allow IPv6 ND Extensions only if supported
On 20 Nov 2019, at 15:21, Flavio Leitner wrote:
The IPv6 ND Extensions is only implemented in userspace datapath,
but nothing prevents that to be used with other datapaths.
This patch probes the datapath and only allows if the support
is available.
Fixes: 9b2b84973 ("Support for match & set ICMPv6 reserved and options
type fields")
Signed-off-by: Flavio Leitner
Changes look good to me…
Acked-by: Eelco Chaudron
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-afxdp: Enable libbpf logging to OVS.
On Thu, Nov 21, 2019 at 11:37:48AM +0100, Eelco Chaudron wrote: > > On 20 Nov 2019, at 21:25, William Tu wrote: > > > libbpf has pr_warn, pr_info, and pr_debug. The patch registers > > these print functions, integrating the libbpf logs to OVS log. > > > > Signed-off-by: William Tu > > Looks good to me… > > Acked-by: Eelco Chaudron > Thanks, applied to master ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCHv4] ofproto-dpif: Expose datapath capability to ovsdb.
On Wed, Nov 20, 2019 at 06:00:44PM -0800, Ben Pfaff wrote: > On Wed, Nov 20, 2019 at 05:20:35PM -0800, William Tu wrote: > > I think the above is not related. I will remove the manpages.mk diff. > > Rest below looks good to me. > > Yes, thanks! I fixed a couple minor issues (tab and lines > 79 char) and applied to master. Thank you! ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto-dpif: Allow IPv6 ND Extensions only if supported
Flavio Leitner writes:
> The IPv6 ND Extensions is only implemented in userspace datapath,
> but nothing prevents that to be used with other datapaths.
>
> This patch probes the datapath and only allows if the support
> is available.
>
> Fixes: 9b2b84973 ("Support for match & set ICMPv6 reserved and options type
> fields")
> Signed-off-by: Flavio Leitner
> ---
LGTM
Acked-by: Aaron Conole
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2] Change in openvswitch kernel module to support MPLS label depth of 3 in ingress direction.
From: Martin Varghese
The openvswitch kernel module was supporting a MPLS label depth of 1
in the ingress direction though the userspace OVS supports a max depth
of 3 labels. This change enables openvswitch module to support a max
depth of 3 labels in the ingress.
Signed-off-by: Martin Varghese
---
Changes in v2
- support added for nested actions.
datapath/actions.c | 2 +-
datapath/flow.c | 20
datapath/flow.h | 8 +++--
datapath/flow_netlink.c | 85 -
tests/system-traffic.at | 39 +++
5 files changed, 122 insertions(+), 32 deletions(-)
diff --git a/datapath/actions.c b/datapath/actions.c
index a44e804..fbf4457 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -276,7 +276,7 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key
*flow_key,
}
stack->label_stack_entry = lse;
- flow_key->mpls.top_lse = lse;
+ flow_key->mpls.lse[0] = lse;
return 0;
}
diff --git a/datapath/flow.c b/datapath/flow.c
index 916f7f4..6dc7402 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -659,27 +659,35 @@ static int key_extract_l3l4(struct sk_buff *skb, struct
sw_flow_key *key)
memset(&key->ipv4, 0, sizeof(key->ipv4));
}
} else if (eth_p_mpls(key->eth.type)) {
- size_t stack_len = MPLS_HLEN;
+ u8 label_count = 1;
+ memset(&key->mpls, 0, sizeof(key->mpls));
skb_set_inner_network_header(skb, skb->mac_len);
while (1) {
__be32 lse;
- error = check_header(skb, skb->mac_len + stack_len);
+ error = check_header(skb, skb->mac_len +
+label_count * MPLS_HLEN);
if (unlikely(error))
return 0;
memcpy(&lse, skb_inner_network_header(skb), MPLS_HLEN);
- if (stack_len == MPLS_HLEN)
- memcpy(&key->mpls.top_lse, &lse, MPLS_HLEN);
+ if (label_count <= MPLS_LABEL_DEPTH)
+ memcpy(&key->mpls.lse[label_count - 1], &lse,
+ MPLS_HLEN);
- skb_set_inner_network_header(skb, skb->mac_len +
stack_len);
+ skb_set_inner_network_header(skb, skb->mac_len +
+label_count * MPLS_HLEN);
if (lse & htonl(MPLS_LS_S_MASK))
break;
- stack_len += MPLS_HLEN;
+ label_count++;
}
+ if (label_count > MPLS_LABEL_DEPTH)
+ label_count = MPLS_LABEL_DEPTH;
+
+ key->mpls.num_labels_mask = GENMASK(label_count - 1, 0);
} else if (key->eth.type == htons(ETH_P_IPV6)) {
int nh_len; /* IPv6 Header + Extensions */
diff --git a/datapath/flow.h b/datapath/flow.h
index 5560300..4ad5363 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -43,6 +43,7 @@ enum sw_flow_mac_proto {
MAC_PROTO_ETHERNET,
};
#define SW_FLOW_KEY_INVALID0x80
+#define MPLS_LABEL_DEPTH 3
/* Store options at the end of the array if they are less than the
* maximum size. This allows us to get the benefits of variable length
@@ -98,9 +99,6 @@ struct sw_flow_key {
*/
union {
struct {
- __be32 top_lse; /* top label stack entry */
- } mpls;
- struct {
u8 proto; /* IP protocol or lower 8 bits of ARP
opcode. */
u8 tos; /* IP ToS. */
u8 ttl; /* IP TTL/hop limit. */
@@ -148,6 +146,10 @@ struct sw_flow_key {
} nd;
};
} ipv6;
+ struct {
+ u32 num_labels_mask;/* labels present bitmap of
effective length MPLS_LABEL_DEPTH */
+ __be32 lse[MPLS_LABEL_DEPTH]; /* label stack entry
*/
+ } mpls;
struct ovs_key_nsh nsh; /* network service header */
};
struct {
diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index 35f13d7..9fc1a19 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -438,7 +438,7 @@ static const struct ovs_len_tbl
ovs_key_lens[OVS_KEY_ATTR_MAX + 1] = {
[OVS_KEY_ATTR_DP_HASH] = { .len = sizeof(u32) },
[OVS_KEY_ATTR_TUNNEL]= { .len = OVS_ATTR_NESTED,
.next = ovs_tunnel_key_lens, },
- [OVS_KEY_ATTR_MPLS] = { .len = sizeof(struct ovs_key_mpls) },
+ [OVS_KEY_ATTR_MPL
Re: [ovs-dev] [PATCH] dp-packet: Fix clearing/copying of memory layout flags.
On 21.11.2019 14:58, Ilya Maximets wrote:
> 'ol_flags' of DPDK mbuf could contain bits responsible for external
> or indirect buffers which are not actually offload flags in a common
> sense. Clearing/copying of these flags could lead to memory leaks of
> external memory chunks and crashes due to access to wrong memory.
>
> OVS should not clear these flags while resetting offloads and also
> should not copy them to the newly allocated packets.
>
> This change is required to support DPDK 19.11, as some drivers may
> return mbufs with these flags set. However, it might be good to do
> the same for DPDK 18.11, because these flags are present and should
> be taken into account.
>
> Signed-off-by: Ilya Maximets
We could also add the following tag:
Fixes: 03f3f9c0faf8 ("dpdk: Update to use DPDK 18.11.")
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] netdev-afxdp: Best-effort configuration of XDP mode.
On 20 Nov 2019, at 17:43, Ilya Maximets wrote: On 20.11.2019 8:35, Eelco Chaudron wrote: On 19 Nov 2019, at 17:52, Ilya Maximets wrote: On 19.11.2019 17:16, Eelco Chaudron wrote: On 7 Nov 2019, at 12:36, Ilya Maximets wrote: Until now there was only two options for XDP mode in OVS: SKB or DRV. i.e. 'generic XDP' or 'native XDP with zero-copy enabled'. Devices like 'veth' interfaces in Linux supports native XDP, but doesn't support zero-copy mode. This case can not be covered by existing API and we have to use slower generic XDP for such devices. There are few more issues, e.g. TCP is not supported in generic XDP mode for veth interfaces due to kernel limitations, however it is supported in native mode. This change introduces ability to use native XDP without zero-copy along with best-effort configuration option that enabled by default. In best-effort case OVS will sequentially try different modes starting from the fastest one and will choose the first acceptable for current interface. This will guarantee the best possible performance. If user will want to choose specific mode, it's still possible by setting the 'options:xdp-mode'. This change additionally changes the API by renaming the configuration knob from 'xdpmode' to 'xdp-mode' and also renaming the modes themselves to be more user-friendly. The full list of currently supported modes: * native-with-zerocopy - former DRV * native - new one, DRV without zero-copy * generic - former SKB * best-effort - new one, chooses the best available from 3 above modes Since 'best-effort' is a default mode, users will not need to explicitely set 'xdp-mode' in most cases. TCP related tests enabled back in system afxdp testsuite, because 'best-effort' will choose 'native' mode for veth interfaces and this mode has no issues with TCP. Patch in general looks good, two small comments inline. Thanks for review. The only thing that bothers me is the worse performance of the TAP interface with the new default config. Can we somehow keep the old behavior for TAP interfaces? Could you check if TCP works over tap interfaces in generic mode? For me the point is that correctness is better than performance. I also hope that native implementation for tap will be improved over time. So if I understood your email chain with William correctly TCP is not working, so I affray correctness is better than performance. Not exactly. William didn't test the actual TAP interfaces. I tested today with kernel vhost backed virtio-user port and it seems to pass TCP frames in generic mode. The setup is following: tap1 <-- ovs-vswitchd --> tap0 <-- testpmd --> tap2 tap1 -- tap port created by OVS (type=tap) tap0 -- tap port, virtio-user, created by testpmd, opened by OVS with type=afxdp tap2 -- tap port created by testpmd (net_tap) tap1 and tap2 are in their own network namespaces and iperf works on them. TCP works in this scenario regardless of xdp-mode on tap0 interface. However, let's look at the difference before-after this patch: PHY NIC TAP VETH before (skb default) slow by default fast by default broken TCP after (best-effort) fast by default slower by default works For the best speed before you had to configure xdp-mode for physical NICs, after you need to configure xdp-mode for TAPs. No much difference from the configuration side, but with patch applied veths are working. So, I think this change makes sense anyway. We can think about workaround for TAP devices, but we still need to check more cases. For now, I updated the 'Limitations' section with information that generic mode works for TAP devices and might be even faster in some cases. I also reformatted the code for 'unspecified' and 'best-effort' modes as you asked and applied to master. Best regards, Ilya Maximets. Thanks! //Eelco ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] include/sparse/rte_flow.h
On 20.11.2019 19:14, Stokes, Ian wrote:
>
>
> On 11/20/2019 6:02 PM, Kevin Traynor wrote:
>> On 19/11/2019 18:48, Ilya Maximets wrote:
>>> On 19.11.2019 19:01, Eli Britstein wrote:
On 11/19/2019 7:46 PM, Ilya Maximets wrote:
> On 19.11.2019 18:29, Eli Britstein wrote:
>> On 11/19/2019 7:27 PM, Eli Britstein wrote:
>>> Hi
>>>
>>> I see this file has many inconsistencies against the one from DPDK
>>> (18.11.2).
>>>
>>> For example, this API:
>>>
>>> rte_flow_query(uint16_t port_id,
>>> struct rte_flow *flow,
>>> enum rte_flow_action_type action,
>>> void *data,
>>> struct rte_flow_error *error);
>>>
>>> is wrong, vs the one from DPDK:
>>>
>>> rte_flow_query(uint16_t port_id,
>>> struct rte_flow *flow,
>>> const struct rte_flow_action *action,
>>> void *data,
>>> struct rte_flow_error *error);
>>>
>>> Note the "action" argument.
>>>
>>>
>>> I also see in it this line:
>>>
>>> #error "Use this header only with sparse. It is not a correct
>>> implementation."
>>>
>>>
>>> So, is it wrong on purpose? If so, why?
>>>
>>> I test my patch-set before I submit using travis, and it fails because
>>> of this wrong file. Can we just take the correct code from DPDK?
>>> Should I maybe take only the parts that cause me to fail?
> Hi. DPDK headers before 18.11.3 has issues that makes sparse unhappy.
> This header will be removed along with upgrade to 18.11.3 or higher.
> Right now we're not experiencing issues with current version of
> sparse header probably just because we're not using most of the functions.
I see. Thanks.
>
> We're not going to update this header only remove. You may update it in
> your patches or base your changes on top of dpdk-latest branch where this
> header already removed.
So, what is the preferred way for submission?
1. cherry-pick those commits from dpdk-latest on top of master and my
patches on top of that
>>>
>>> This doesn't sound like a good option.
>>> If sparse header needs only few small changes for your patches to work,
>>> you may create a special patch for that. If not, you may send patches
>>> as-is but mention that these patches depends on a DPDK 18.11.3+ and another
>>> patch that removes the sparse header.
>>>
2. submit directly on dpdk-latest
>>>
>>> Not sure about this option because dpdk-latest is mostly for changes that
>>> requires most recent DPDK, but this is not exactly your case.
>>>
>
> I'm not sure when we're going to migrate to 18.11.{3,5}.
> @Ian, @Kevin, is validation still in progress? Does anyone work on this?
>>
>> Ian ran his automated tests at the time of 18.11.3 and reported results
>> here:
>> http://inbox.dpdk.org/stable/c243e0b9-bac9-9759-c51e-e40320100...@intel.com/
>> I ran some PVP tests also at that time but they were on rpms with some
>> patches, so not as relevant.
>>
>> Other general 18.11.3 validation is in that thread or there is a summary
>> in the release notes
>> http://doc.dpdk.org/guides-18.11/rel_notes/release_18_11.html#id7
>>
>> I don't think the changes in 18.11.4/5 will have an impact, but if Ian
>> is able to re-run those automated tests again, it might be best.
>
> I was holding off moving to 18.11.3 as there was talk on a .4 (and now .5 due
> to CVE I believe), so from a validation point of view we've held off until it
> was settled. We can run validation on .5 if its the case it has all required
> cve fixes.
>
>>
Is it a question of "if" or "when"? what is the purpose of migrating to
18.11.3/5 and not to 19.11 soon?
>>>
>>> 18.11.3/5 requires validation + small patch for docs/CI.
>>> 19.11 requires additional development that didn't started yet
>>> + validation + patch for docs/CI.
>>>
>>> Plus, 18.11 needs to be upgraded on previous versions of OVS too.
>>>
>>> With current speed of development and validation I will not be surprised if
>>> 19.11 will not be supported in next OVS release.
>
> So I would think that this upgrade would go ahead, with RC3 imminent I think
> 19.11 will settle.
>
> I know there is a few issues such as RSS offload which we're looking to patch
> and we're beginning validation now on existing features along with required
> fixes. Is there a particular issue you are aware of that would block the
> 19.11 upgrade?
I don’t know (at least I don’t remember) any specific problem other than
clearing 'ol_flags'. BTW, The patch is available here:
https://patchwork.ozlabs.org/patch/1198954/
I'm just a little skeptical, as we have less than 4 weeks (excluding OVS
conf and Christmas / New Year holidays) until a soft freeze and there is
no much activity in a mail-list.
Best regards, Ilya Maximets.
__
[ovs-dev] [PATCH] dp-packet: Fix clearing/copying of memory layout flags.
'ol_flags' of DPDK mbuf could contain bits responsible for external
or indirect buffers which are not actually offload flags in a common
sense. Clearing/copying of these flags could lead to memory leaks of
external memory chunks and crashes due to access to wrong memory.
OVS should not clear these flags while resetting offloads and also
should not copy them to the newly allocated packets.
This change is required to support DPDK 19.11, as some drivers may
return mbufs with these flags set. However, it might be good to do
the same for DPDK 18.11, because these flags are present and should
be taken into account.
Signed-off-by: Ilya Maximets
---
lib/dp-packet.c | 1 +
lib/dp-packet.h | 7 ++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index 62d7faa4c..8dfedcb7c 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -194,6 +194,7 @@ dp_packet_clone_with_headroom(const struct dp_packet
*buffer, size_t headroom)
#ifdef DPDK_NETDEV
new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags;
+new_buffer->mbuf.ol_flags &= ~DPDK_MBUF_NON_OFFLOADING_FLAGS;
#endif
if (dp_packet_rss_valid(buffer)) {
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 14f0897fa..3dd59e25d 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -54,6 +54,11 @@ enum dp_packet_offload_mask {
DP_PACKET_OL_RSS_HASH_MASK = 0x1, /* Is the 'rss_hash' valid? */
DP_PACKET_OL_FLOW_MARK_MASK = 0x2, /* Is the 'flow_mark' valid? */
};
+#else
+/* DPDK mbuf ol_flags that are not really an offload flags. These are mostly
+ * related to mbuf memory layout and OVS should not touch/clear them. */
+#define DPDK_MBUF_NON_OFFLOADING_FLAGS (EXT_ATTACHED_MBUF | \
+IND_ATTACHED_MBUF)
#endif
/* Buffer for holding packet data. A dp_packet is automatically reallocated
@@ -538,7 +543,7 @@ dp_packet_rss_valid(const struct dp_packet *p)
static inline void
dp_packet_reset_offload(struct dp_packet *p)
{
-p->mbuf.ol_flags = 0;
+p->mbuf.ol_flags &= DPDK_MBUF_NON_OFFLOADING_FLAGS;
}
static inline bool
--
2.17.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] B2B COLLABORATION ( BUYING)
Hi, Located in the United Kingdom, famous supermarket chain Sainsbury's Supermarkets Limited, is UK's one of the largest multi-channel retailer with over 1423 shops furnished by European products. We are looking for new products to attract new customers and also retained our existing ones , create new partnerships with companies dealing with different kinds goods. Please send us your catalog or your website through email to speed up and to learn more about your company’s products and wholesale quote. We hope being able to order with you and start long-term friendly, respectable and solid business partnership. We count on the reliability for both sides. We commit ourselves for a successful and professional processing for a good cooperation in all ranges. Could you also send to us all information required to become one of your regular distributors in Europe and worldwide. Please, we would appreciate if you could send us your stock availability via email ( recept...@sainsburys-suppliers.com ) We will also be pleased to receive any offers or proposals from other products available and ready (Stocks and rates). Target: Our Payment Term is within 15 days net in Europe and 30 days NET in UK as we operate with all our suppliers. Sainsbury's Supermarkets Limited Purchasing Department 33 Holborn London United Kingdom EC1N 2HT Email: recept...@sainsburys-suppliers.com Helpline: +442039981997 (+44 203 998 1997 or 02039981997) ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 0/3] Associate identifier with OVN ACL connection tracking entry
On Sat, Nov 9, 2019 at 8:20 AM Ankur Sharma wrote: > > I submitted this patch long time back and somehow lost track it. > Resubmitting the series, calling it as V4, as it addresses the > review comments given till v3. > https://mail.openvswitch.org/pipermail/ovs-dev/2019-April/358280.html > > What: > > a. Goal is to be able to associate some identifier with a connection tracking > entry. > > b. This identifier can be used to map OVN ACL which added this entry or > higher level constructs like openstack security group etc. > > c. There are 2 connection tracking fields which can be used for it. > ct.mark (32 bits) and ct.label (128 bits). > > d. Patch intends to use ct.label, as this is a longer field and > hence would be put to a better use, if it stores the identifier. > > Why: > > a. Adding an identifier would help in debugging. > b. Now, we can map a connection tracking entry to corresponding >acl, security group etc. > c. One of the use cases for this mapping would be to identify >ACLs which added corresponding connection tracker entry, which >is causing unexpected drops/leaks. > > How: > > Following is the sequence of changes: > > Patch 1: >i. Current implementation uses a bit ct.label to handle policy update > cases, >where we use a bit in ct.label to indicate that reply traffic should >be dropped now. > ii. Swap the usage of ct.label in current implementation with ct.mark. > > Patch 2: >i. Add support in parser to allow ct.label and mark to be set from > registers > as well (as of now only integer/masked integer is allowed). > > Patch 3: >i. Add a new column (named 'label') to Table ACL in northbound schema. > ii. ovn-northd changes to enhance logical flows to set ct.label to > acl->label. > For example: > table=4 (ls_out_acl ), action=(reg0[1] = 1; reg0[3] = 1; > xxreg1 = 0x1234; next;) > . > . > . > table=7 (ls_out_stateful), ... match=(reg0[1] == 1 && reg0[3] == 1), > > Hi Ankur, Overall the series looks good to me. Can you please rebase and submit v5. In patch 2, using register offsets is not supported. Something like below "ct_commit(ct_label=xreg1[10..30]);" I would suggest to support that case as well. Thanks Numan > Ankur Sharma (3): > OVN ACL: Replace the usage of ct_label with ct_mark > OVN ACL: Allow ct_mark and ct_label values to be set from register as > well > OVN ACL: Allow a user to input ct.label value for an acl > > Documentation/tutorials/ovn-openstack.rst | 14 ++--- > include/ovn/actions.h | 3 + > lib/actions.c | 72 ++ > lib/logical-fields.c | 3 + > northd/ovn-northd.8.xml | 14 ++--- > northd/ovn-northd.c | 87 +-- > ovn-nb.ovsschema | 5 +- > ovn-nb.xml| 12 > ovn-sb.xml| 41 + > tests/ovn-nbctl.at| 12 +++- > tests/ovn.at | 99 > +-- > utilities/ovn-nbctl.c | 24 +++- > 12 files changed, 310 insertions(+), 76 deletions(-) > > -- > 1.8.3.1 > > ___ > 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] [PATCHv4] netdev-afxdp: Enable loading XDP program.
On 20 Nov 2019, at 21:17, William Tu wrote: On Tue, Nov 19, 2019 at 10:51:01AM -0800, William Tu wrote: Hi Eelco, Thanks for your testing. On Tue, Nov 12, 2019 at 11:25:55AM +0100, Eelco Chaudron wrote: See one remark below, however when I did a quick test with a program that would not load it goes into some re-try loop: 2019-11-12T10:13:21.658Z|01609|netdev_afxdp|INFO|eno1: Removing xdp program. 2019-11-12T10:13:21.658Z|01610|netdev_afxdp|INFO|Removed program ID: 0, fd: 0 2019-11-12T10:13:21.658Z|01611|netdev_afxdp|INFO|eno1: Setting XDP mode to DRV. 2019-11-12T10:13:22.224Z|01612|netdev_afxdp|INFO|eno1: Load custom XDP program at /root/xdp_pass_kern.o. 2019-11-12T10:13:22.274Z|01613|netdev_afxdp|ERR|xsk_socket__create failed (Resource temporarily unavailable) mode: DRV use-need-wakeup: true qid: 0 I couldn't reproduce this issue. Also looking at xsk_socket__create, I couln't figure out how it returns EAGAIN (Resource temporarily unavailable) in your case. 2019-11-12T10:13:22.300Z|01614|netdev_afxdp|ERR|Failed to create AF_XDP socket on queue 0. 2019-11-12T10:13:22.300Z|01615|netdev_afxdp|INFO|eno1: Removing xdp program. 2019-11-12T10:13:22.658Z|00047|ovs_rcu(urcu2)|WARN|blocked 1000 ms waiting for main to quiesce 2019-11-12T10:13:22.735Z|01616|netdev_afxdp|INFO|Removed program ID: 320, fd: 0 2019-11-12T10:13:22.735Z|01617|netdev_afxdp|ERR|AF_XDP device eno1 reconfig failed. 2019-11-12T10:13:22.735Z|01618|dpif_netdev|ERR|Failed to set interface eno1 new configuration 2019-11-12T10:13:22.735Z|01619|netdev_afxdp|INFO|FIXME: Device tapVM always use numa id 0. 2019-11-12T10:13:22.735Z|01620|dpif_netdev|INFO|Core 1 on numa node 0 assigned port 'tapVM' rx queue 0 (measured processing cycles 0). 2019-11-12T10:13:22.735Z|01621|dpif|WARN|netdev@ovs-netdev: failed to add eno1 as port: Invalid argument 2019-11-12T10:13:22.735Z|01622|netdev_afxdp|INFO|eno1: Removing xdp program. 2019-11-12T10:13:22.736Z|01623|netdev_afxdp|INFO|Removed program ID: 0, fd: 0 2019-11-12T10:13:22.736Z|01624|timeval|WARN|Unreasonably long 1079ms poll interval (7ms user, 80ms system) 2019-11-12T10:13:22.736Z|01625|timeval|WARN|faults: 16387 minor, 0 major 2019-11-12T10:13:22.736Z|01626|timeval|WARN|context switches: 327 voluntary, 337 involuntary 2019-11-12T10:13:22.738Z|01627|netdev_afxdp|INFO|eno1: Removing xdp program. 2019-11-12T10:13:22.738Z|01628|netdev_afxdp|INFO|Removed program ID: 0, fd: 0 2019-11-12T10:13:22.739Z|01629|netdev_afxdp|INFO|eno1: Setting XDP mode to DRV. 2019-11-12T10:13:23.312Z|01630|netdev_afxdp|INFO|eno1: Load custom XDP program at /root/xdp_pass_kern.o. 2019-11-12T10:13:23.363Z|01631|netdev_afxdp|ERR|xsk_socket__create failed (Resource temporarily unavailable) mode: DRV use-need-wakeup: true qid: 0 2019-11-12T10:13:23.392Z|01632|netdev_afxdp|ERR|Failed to create AF_XDP socket on queue 0. 2019-11-12T10:13:23.392Z|01633|netdev_afxdp|INFO|eno1: Removing xdp program. 2019-11-12T10:13:23.738Z|00048|ovs_rcu(urcu2)|WARN|blocked 1000 ms waiting for main to quiesce 2019-11-12T10:13:23.823Z|01634|netdev_afxdp|INFO|Removed program ID: 324, fd: 0 2019-11-12T10:13:23.823Z|01635|netdev_afxdp|ERR|AF_XDP device eno1 reconfig failed. 2019-11-12T10:13:23.823Z|01636|dpif_netdev|ERR|Failed to set interface eno1 new configuration 2019-11-12T10:13:23.823Z|01637|netdev_afxdp|INFO|FIXME: Device tapVM always use numa id 0. 2019-11-12T10:13:23.823Z|01638|dpif_netdev|INFO|Core 1 on numa node 0 assigned port 'tapVM' rx queue 0 (measured processing cycles 0). 2019-11-12T10:13:23.823Z|01639|dpif|WARN|netdev@ovs-netdev: failed to add eno1 as port: Invalid argument But in addition during this loop it’s not freeing it’s resources: $ bpftool prog list && bpftool map 4: xdp tag 80b55d8a76303785 loaded_at 2019-11-12T05:11:15-0500 uid 0 xlated 136B jited 96B memlock 4096B map_ids 4 12: xdp name xdp_prog_simple tag 3b185187f1855c4c gpl loaded_at 2019-11-12T05:11:58-0500 uid 0 xlated 16B jited 35B memlock 4096B 16: xdp name xdp_prog_simple tag 3b185187f1855c4c gpl loaded_at 2019-11-12T05:11:59-0500 uid 0 xlated 16B jited 35B memlock 4096B 20: xdp name xdp_prog_simple tag 3b185187f1855c4c gpl loaded_at 2019-11-12T05:12:00-0500 uid 0 xlated 16B jited 35B memlock 4096B … … 120: (null) name xsks_map flags 0x0 key 4B value 4B max_entries 32 memlock 4096B 122: (null) name xsks_map flags 0x0 key 4B value 4B max_entries 32 memlock 4096B 124: (null) name xsks_map flags 0x0 key 4B value 4B max_entries 32 memlock 4096B 126: (null) name xsks_map flags 0x0 key 4B value 4B max_entries 32 memlock 4096B 128: (null) name xsks_map flags 0x0 key 4B value 4B max_entries 32 memlock 4096B 130: (null) name xsks_map flags 0x0 key 4B value 4B max_entries 32 memlock 4096B 132: (null) name xsks_map flags 0x0 key 4B value 4B m
Re: [ovs-dev] [PATCH] netdev-afxdp: Enable libbpf logging to OVS.
On 20 Nov 2019, at 21:25, William Tu wrote: > libbpf has pr_warn, pr_info, and pr_debug. The patch registers > these print functions, integrating the libbpf logs to OVS log. > > Signed-off-by: William Tu Looks good to me… Acked-by: Eelco Chaudron ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [bug-report] bfd decay unit case failure
Hi, I encountered a unit test failure when I ran the Open vSwitch testsuite on Travis CI aarch64 lxd container. The environment can be found at line 7 : build system information section in the report on https://travis-ci.org/yzyuestc/ovs/jobs/614941322 . The unit test case name is "bfd decay" , you can find the unit test failure details in the report after line 6520. The failure is not 100% reproducible on Travis CI. Could anyone give some hint on what is wrong for this unit test case? Best regards, Lance IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn 0/2] Add IPv6 Prefix delegation (RFC3633)
> On Thu, Nov 7, 2019 at 2:18 PM Lorenzo Bianconi
> wrote:
> >
> > Introduce IPv6 Prefix delegation state machine according to RFC 3633
> > https://tools.ietf.org/html/rfc3633.
> > Add dhcp6_server_pkt controller action to parse advertise/reply from
> > IPv6 delegation server.
> > Introduce logical flows in ovn router pipeline in order to parse dhcpv6
> > advertise/reply from IPv6 prefix delegation router.
> > This series relies on the following OVS commit:
> > https://github.com/openvswitch/ovs/commit/cec89046f72cb044b068ba6a4e30dbcc4292c4c1
> >
> > Lorenzo Bianconi (2):
> > controller: add ipv6 prefix delegation state machine
> > northd: add logical flows for dhcpv6 pfd parsing
>
> Hi Lorenzo,
>
> I did a higher level review. I have few comments
Hi Numan,
thx for the review
>
> (1) - I woukd suggest to change the name of new action from
> dhcp6_server_pkt to may be "handle_dhcpv6_reply".
ack, will do in v2
>
> (2) Let's say you have the below logical network topology
>
> **
> switch 41afb0dc-fea4-4464-831a-879dfcde7999 (sw1)
> port sw1-lr0
> type: router
> router-port: lr0-sw1
> port sw1-port1
> addresses: ["40:54:00:00:00:03"]
> switch f826550f-4340-43b0-ac82-4baac6a0b668 (public)
> port public-lr0
> type: router
> router-port: lr0-public
> switch 346d731f-1d28-4c94-a96c-d456eb9a489e (sw0)
> port sw0-lr0
> type: router
> router-port: lr0-sw0
> port sw0-port1
> addresses: ["50:54:00:00:00:03"]
> router 40525d05-09d7-4d9c-9194-31d0f2afcfd6 (lr0)
> port lr0-public
> mac: "00:00:20:20:12:13"
> networks: ["172.168.0.100/24"]
> port lr0-sw0
> mac: "00:00:00:00:ff:01"
> networks: ["10.0.0.1/24"]
> port lr0-sw1
> mac: "00:00:00:00:ff:02"
> networks: ["20.0.0.1/24"]
>
> *
>
> If I understand correctly, this patch expects that
> "prefix_delegation=true" is set in options columns of lr0-public right
> ?
> So how would sw0 and sw1 logical switches get their prefixes ?
>
> The main goal of prefix delegation is that the private networks
> connected to a logical router should get their prefixes
> from an upstream prefix delegation server. I don't see that addressed
> in this patch series.
According to the RFC3633:
"The requesting router is then responsible for the delegated
prefix(es). For example, the requesting router might assign a subnet
from a delegated prefix to one of its interfaces, and begin sending
router advertisements for the prefix on that link"
In the current implementation the requesting router (OVN) will get a single
IPv6 prefix and let the CMS divide the prefix among the downstream interfaces
in order to send this info in Router Advertisement. I though this approach is
the most configurable and moreover it is more scalable since reduces the number
of states/network traffic. (e.g what if the logical router has 1 logical
router port?)
>
> I think we should allow CMS to set "prefix_delegation=true" in
> sw0-lr0 and sw1-lr0's options so
> that ovn-controller will send IPv6 prefix delegation request for each
> of these logical router ports.
>
> ovn-controller where cr-lr0-public is resident should send out these
> packets via the lr0-public port and it can
> allocate unique IAID for each logical router port - i.e lr0-sw0 and
> lr0-sw1. This way we can distinguish
> to which logical port the prefix delegation packet belongs to.
>
> (3) Prefix delegation feature is used if CMS is not sure what IPv6
> prefix to use for each of the logical switches.
> When adding a logical router port to a router, we won't be knowing the
> network prefix. So we should allow something
> like - ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 and
> then once we receive the prefix from the PD server
> we can use that in router flows. This patch series should handle this
> use case as well.
>
>
> (4) Patch 1 needs to add documentation for the new action
ack will do in v2
>
> (5) Test cases should be added - both unit and system tests.
ack will do in v2
>
> (6) In my sandbox when I ran "ovn-nbctl set logical_router_port
> lr0-public options:prefix_delegation=true" I see the below logical
> flows for dhcp6 handling
>
> ***
> ovn-sbctl dump-flows lr0 | grep dhcp
> table=3 (lr_in_ip_input ), priority=100 , match=(inport ==
> "cr-lr0-public" && ip6.dst == fe80::200:20ff:fe20:1213 && udp.src ==
> 547 && udp.dst == 546), action=(reg0 = 0; dhcp6_server_pkt { eth.dst
> <-> eth.src; ip6.dst <-> ip6.src; outport <-> inport; output; };)
> table=3 (lr_in_ip_input ), priority=100 , match=(inport ==
> "lr0-public" && ip6.dst == fe80::200:20ff:fe20:1213 && udp.src == 547
> && udp.dst == 546), action=(reg0 = 0; dhcp6_server_pkt { eth.dst <->
> eth.src; ip6.dst <-> ip6.src; outport <-> inport; output; };)
> ***
>
> I think it should not add flows for cr-lr0-public.
ack will fix in v2
Regards,
Lorenzo
>
>
> Thanks
> Numan
>
> >
> >
[ovs-dev] [bug-report] Check Python IDL reconnects to leader unit case failed on arm64
Hi, I encountered an issue when I was running testsuite on Arm. The system I tested on was an arm server running Ubuntu 18.04.3 LTS (GNU/Linux 4.15.0-65-generic aarch64). After I downloaded the up-to-date source code on master branch and built ovs, I run the testsuite with TESTSUITEFLAGS="2105" (Unit case name: Check Python IDL reconnects to leader) I run the unit case for many times, most of the time I got the following failure: make check TESTSUITEFLAGS="2105" RECHECK=yes make check-recursive make[1]: Entering directory '/home/lance/ovs' Making check in datapath make[2]: Entering directory '/home/lance/ovs/datapath' make[3]: Entering directory '/home/lance/ovs/datapath' make[3]: Leaving directory '/home/lance/ovs/datapath' make[2]: Leaving directory '/home/lance/ovs/datapath' make[2]: Entering directory '/home/lance/ovs' make[3]: Entering directory '/home/lance/ovs/datapath' make[3]: 'distfiles' is up to date. make[3]: Leaving directory '/home/lance/ovs/datapath' make utilities/ovs-appctl-bashcomp.bash utilities/ovs-vsctl-bashcomp.bash tests/atlocal make[3]: Entering directory '/home/lance/ovs' make[3]: Nothing to be done for 'utilities/ovs-appctl-bashcomp.bash'. make[3]: Nothing to be done for 'utilities/ovs-vsctl-bashcomp.bash'. make[3]: 'tests/atlocal' is up to date. make[3]: Leaving directory '/home/lance/ovs' make check-local make[3]: Entering directory '/home/lance/ovs' set /bin/bash './tests/testsuite' -C tests AUTOTEST_PATH=utilities:vswitchd:ovsdb:vtep:tests::; \ "$@" -j2 2104-2105 || (test X'' = Xyes && "$@" --recheck) ## --- ## ## openvswitch 2.12.90 test suite. ## ## --- ## 2105: Check Python IDL reconnects to leader - Python3 (leader only) FAILED (ovsdb-idl.at:1816) ## - ## ## Test results. ## ## - ## ERROR: All 1 test was run, 1 failed unexpectedly. ## -- ## ## testsuite.log was created. ## ## -- ## Please send `tests/testsuite.log' and all information you think might help: To: Subject: [openvswitch 2.12.90] testsuite: 2105 failed You may investigate any problem if you feel able to do so, in which case the test suite provides a good starting point. Its output may be found below `tests/testsuite.dir'. ## --- ## 2105: Check Python IDL reconnects to leader - Python3 (leader only) FAILED (ovsdb-idl.at:1816) ## - ## ## Test results. ## ## - ## ERROR: 1 test was run, 1 failed unexpectedly. ## -- ## ## testsuite.log was created. ## ## -- ## Please send `tests/testsuite.log' and all information you think might help: To: Subject: [openvswitch 2.12.90] testsuite: 2105 failed You may investigate any problem if you feel able to do so, in which case the test suite provides a good starting point. Its output may be found below `tests/testsuite.dir'. Makefile:6151: recipe for target 'check-local' failed make[3]: *** [check-local] Error 1 make[3]: Leaving directory '/home/lance/ovs' Makefile:5275: recipe for target 'check-am' failed make[2]: *** [check-am] Error 2 make[2]: Leaving directory '/home/lance/ovs' Makefile:4984: recipe for target 'check-recursive' failed make[1]: *** [check-recursive] Error 1 make[1]: Leaving directory '/home/lance/ovs' Makefile:5278: recipe for target 'check' failed make: *** [check] Error 2 I attached the tests/testsuite.dir/testsuite.log to this email. I also attached the logs for ovsdb. Any hints on what is wrong here? Thank you so much. Best regards, Lance Yang IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
