Re: [ovs-dev] [PATCH v4 5/7] timeval: Introduce time_usec().
On 25.10.2017 20:28, Ben Pfaff wrote: > On Fri, Oct 13, 2017 at 01:03:18PM +, Bodireddy, Bhanuprakash wrote: >>> This fanction will provide monotonic time in microseconds. >> >> [BHANU] Typo here with function. >> >>> >>> Signed-off-by: Ilya Maximets >>> --- >>> lib/timeval.c | 22 ++ lib/timeval.h | 2 ++ >>> 2 files changed, 24 insertions(+) >>> >>> diff --git a/lib/timeval.c b/lib/timeval.c index dd63f03..be2eddc 100644 >>> --- a/lib/timeval.c >>> +++ b/lib/timeval.c >>> @@ -233,6 +233,22 @@ time_wall_msec(void) >>> return time_msec__(&wall_clock); >>> } >>> >>> +static long long int >>> +time_usec__(struct clock *c) >>> +{ >>> +struct timespec ts; >>> + >>> +time_timespec__(c, &ts); >>> +return timespec_to_usec(&ts); >>> +} >>> + >>> +/* Returns a monotonic timer, in microseconds. */ long long int >>> +time_usec(void) >>> +{ >>> +return time_usec__(&monotonic_clock); } >>> + >> >> [BHANU] As you are introducing the support for microsecond granularity, can >> you also add time_wall_usec() and time_wall_usec__() here? I'm not sure what you meant under 'time_wall_usec__()'. There is no such function for msec. >> The ipfix code (ipfix_now()) can be the first one to use it for now. May be >> more in the future! >> >>> /* Configures the program to die with SIGALRM 'secs' seconds from now, if >>> * 'secs' is nonzero, or disables the feature if 'secs' is zero. */ void >>> @@ -360,6 >>> +376,12 @@ timeval_to_msec(const struct timeval *tv) >>> return (long long int) tv->tv_sec * 1000 + tv->tv_usec / 1000; } >>> >>> +long long int >>> +timespec_to_usec(const struct timespec *ts) { >>> +return (long long int) ts->tv_sec * 1000 * 1000 + ts->tv_nsec / >>> +1000; } >>> + >> >> [BHANU] how about adding timeval_to_usec()? >> Also it would be nice to have the usec_to_timespec() and >> timeval_diff_usec() implemented to make this commit complete. > > I'd appreciate those changes too; with those changes, I'd be happy to > apply this, independent of anything else in the series. OK. Cool. I'm going to implement additionally: time_wall_usec() usec_to_timespec() timeval_to_usec() timeval_diff_usec() Please, correct me if I missed something. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH net-next v13] openvswitch: enable NSH support
v12->v13 - Fix NSH header length check in set_nsh v11->v12 - Fix missing changes old comments pointed out - Fix new comments for v11 v10->v11 - Fix the left three disputable comments for v9 but not fixed in v10. v9->v10 - Change struct ovs_key_nsh to struct ovs_nsh_key_base base; __be32 context[NSH_MD1_CONTEXT_SIZE]; - Fix new comments for v9 v8->v9 - Fix build error reported by daily intel build because nsh module isn't selected by openvswitch v7->v8 - Rework nested value and mask for OVS_KEY_ATTR_NSH - Change pop_nsh to adapt to nsh kernel module - Fix many issues per comments from Jiri Benc v6->v7 - Remove NSH GSO patches in v6 because Jiri Benc reworked it as another patch series and they have been merged. - Change it to adapt to nsh kernel module added by NSH GSO patch series v5->v6 - Fix the rest comments for v4. - Add NSH GSO support for VxLAN-gpe + NSH and Eth + NSH. v4->v5 - Fix many comments by Jiri Benc and Eric Garver for v4. v3->v4 - Add new NSH match field ttl - Update NSH header to the latest format which will be final format and won't change per its author's confirmation. - Fix comments for v3. v2->v3 - Change OVS_KEY_ATTR_NSH to nested key to handle length-fixed attributes and length-variable attriubte more flexibly. - Remove struct ovs_action_push_nsh completely - Add code to handle nested attribute for SET_MASKED - Change PUSH_NSH to use the nested OVS_KEY_ATTR_NSH to transfer NSH header data. - Fix comments and coding style issues by Jiri and Eric v1->v2 - Change encap_nsh and decap_nsh to push_nsh and pop_nsh - Dynamically allocate struct ovs_action_push_nsh for length-variable metadata. OVS master and 2.8 branch has merged NSH userspace patch series, this patch is to enable NSH support in kernel data path in order that OVS can support NSH in compat mode by porting this. Signed-off-by: Yi Yang --- include/net/nsh.h| 3 + include/uapi/linux/openvswitch.h | 29 net/nsh/nsh.c| 61 net/openvswitch/Kconfig | 1 + net/openvswitch/actions.c| 117 +++ net/openvswitch/flow.c | 51 +++ net/openvswitch/flow.h | 7 + net/openvswitch/flow_netlink.c | 310 ++- net/openvswitch/flow_netlink.h | 5 + 9 files changed, 583 insertions(+), 1 deletion(-) diff --git a/include/net/nsh.h b/include/net/nsh.h index a1eaea2..7dcf377 100644 --- a/include/net/nsh.h +++ b/include/net/nsh.h @@ -304,4 +304,7 @@ static inline void nsh_set_flags_ttl_len(struct nshhdr *nsh, u8 flags, NSH_FLAGS_MASK | NSH_TTL_MASK | NSH_LEN_MASK); } +int skb_push_nsh(struct sk_buff *skb, const struct nshhdr *pushed_nh); +int skb_pop_nsh(struct sk_buff *skb); + #endif /* __NET_NSH_H */ diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h index 0cd6f88..ac2623b 100644 --- a/include/uapi/linux/openvswitch.h +++ b/include/uapi/linux/openvswitch.h @@ -333,6 +333,7 @@ enum ovs_key_attr { OVS_KEY_ATTR_CT_LABELS, /* 16-octet connection tracking label */ OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4, /* struct ovs_key_ct_tuple_ipv4 */ OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV6, /* struct ovs_key_ct_tuple_ipv6 */ + OVS_KEY_ATTR_NSH, /* Nested set of ovs_nsh_key_* */ #ifdef __KERNEL__ OVS_KEY_ATTR_TUNNEL_INFO, /* struct ip_tunnel_info */ @@ -492,6 +493,30 @@ struct ovs_key_ct_tuple_ipv6 { __u8 ipv6_proto; }; +enum ovs_nsh_key_attr { + OVS_NSH_KEY_ATTR_UNSPEC, + OVS_NSH_KEY_ATTR_BASE, /* struct ovs_nsh_key_base. */ + OVS_NSH_KEY_ATTR_MD1, /* struct ovs_nsh_key_md1. */ + OVS_NSH_KEY_ATTR_MD2, /* variable-length octets for MD type 2. */ + __OVS_NSH_KEY_ATTR_MAX +}; + +#define OVS_NSH_KEY_ATTR_MAX (__OVS_NSH_KEY_ATTR_MAX - 1) + +struct ovs_nsh_key_base { + __u8 flags; + __u8 ttl; + __u8 mdtype; + __u8 np; + __be32 path_hdr; +}; + +#define NSH_MD1_CONTEXT_SIZE 4 + +struct ovs_nsh_key_md1 { + __be32 context[NSH_MD1_CONTEXT_SIZE]; +}; + /** * enum ovs_flow_attr - attributes for %OVS_FLOW_* commands. * @OVS_FLOW_ATTR_KEY: Nested %OVS_KEY_ATTR_* attributes specifying the flow @@ -808,6 +833,8 @@ struct ovs_action_push_eth { * @OVS_ACTION_ATTR_POP_ETH: Pop the outermost Ethernet header off the * packet. * @OVS_ACTION_ATTR_CT_CLEAR: Clear conntrack state from the packet. + * @OVS_ACTION_ATTR_PUSH_NSH: push NSH header to the packet. + * @OVS_ACTION_ATTR_POP_NSH: pop the outermost NSH header off the packet. * * Only a single header can be set with a single %OVS_ACTION_ATTR_SET. Not all * fields within a header are modifiable, e.g. the IPv4 protocol and fragment @@ -838,6 +865,8 @@ enum ovs_action_attr { OVS_ACTION_ATTR_PUSH_ETH, /* struct ovs_action_push_eth. */ OVS_ACTION_ATTR_POP_ETH, /* No argument. */
[ovs-dev] Bug#771507: marked as done (openvswitch-switch: missing systemd files)
Your message dated Thu, 26 Oct 2017 09:00:34 + with message-id and subject line Bug#771507: fixed in openvswitch 2.8.1+dfsg1-1 has caused the Debian Bug report #771507, regarding openvswitch-switch: missing systemd files to be marked as done. This means that you claim that the problem has been dealt with. If this is not the case it is now your responsibility to reopen the Bug report if necessary, and/or fix the problem forthwith. (NB: If you are a system administrator and have no idea what this message is talking about, this may indicate a serious mail system misconfiguration somewhere. Please contact ow...@bugs.debian.org immediately.) -- 771507: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=771507 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Bug#863228: marked as done (openvswtich: CVE-2017-9214)
Your message dated Thu, 26 Oct 2017 09:00:34 + with message-id and subject line Bug#863228: fixed in openvswitch 2.8.1+dfsg1-1 has caused the Debian Bug report #863228, regarding openvswtich: CVE-2017-9214 to be marked as done. This means that you claim that the problem has been dealt with. If this is not the case it is now your responsibility to reopen the Bug report if necessary, and/or fix the problem forthwith. (NB: If you are a system administrator and have no idea what this message is talking about, this may indicate a serious mail system misconfiguration somewhere. Please contact ow...@bugs.debian.org immediately.) -- 863228: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=863228 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Bug#863655: marked as done (openvswitch: CVE-2017-9263)
Your message dated Thu, 26 Oct 2017 09:00:34 + with message-id and subject line Bug#863655: fixed in openvswitch 2.8.1+dfsg1-1 has caused the Debian Bug report #863655, regarding openvswitch: CVE-2017-9263 to be marked as done. This means that you claim that the problem has been dealt with. If this is not the case it is now your responsibility to reopen the Bug report if necessary, and/or fix the problem forthwith. (NB: If you are a system administrator and have no idea what this message is talking about, this may indicate a serious mail system misconfiguration somewhere. Please contact ow...@bugs.debian.org immediately.) -- 863655: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=863655 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Bug#863661: marked as done (openvswitch: CVE-2017-9264)
Your message dated Thu, 26 Oct 2017 09:00:34 + with message-id and subject line Bug#863661: fixed in openvswitch 2.8.1+dfsg1-1 has caused the Debian Bug report #863661, regarding openvswitch: CVE-2017-9264 to be marked as done. This means that you claim that the problem has been dealt with. If this is not the case it is now your responsibility to reopen the Bug report if necessary, and/or fix the problem forthwith. (NB: If you are a system administrator and have no idea what this message is talking about, this may indicate a serious mail system misconfiguration somewhere. Please contact ow...@bugs.debian.org immediately.) -- 863661: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=863661 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Bug#863662: marked as done (openvswitch: CVE-2017-9265)
Your message dated Thu, 26 Oct 2017 09:00:34 + with message-id and subject line Bug#863662: fixed in openvswitch 2.8.1+dfsg1-1 has caused the Debian Bug report #863662, regarding openvswitch: CVE-2017-9265 to be marked as done. This means that you claim that the problem has been dealt with. If this is not the case it is now your responsibility to reopen the Bug report if necessary, and/or fix the problem forthwith. (NB: If you are a system administrator and have no idea what this message is talking about, this may indicate a serious mail system misconfiguration somewhere. Please contact ow...@bugs.debian.org immediately.) -- 863662: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=863662 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Bug#877543: marked as done (CVE-2017-14970)
Your message dated Thu, 26 Oct 2017 09:00:34 + with message-id and subject line Bug#877543: fixed in openvswitch 2.8.1+dfsg1-1 has caused the Debian Bug report #877543, regarding CVE-2017-14970 to be marked as done. This means that you claim that the problem has been dealt with. If this is not the case it is now your responsibility to reopen the Bug report if necessary, and/or fix the problem forthwith. (NB: If you are a system administrator and have no idea what this message is talking about, this may indicate a serious mail system misconfiguration somewhere. Please contact ow...@bugs.debian.org immediately.) -- 877543: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=877543 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Bug#878249: marked as done (Please package >= 2.7.0)
Your message dated Thu, 26 Oct 2017 09:00:34 + with message-id and subject line Bug#878249: fixed in openvswitch 2.8.1+dfsg1-1 has caused the Debian Bug report #878249, regarding Please package >= 2.7.0 to be marked as done. This means that you claim that the problem has been dealt with. If this is not the case it is now your responsibility to reopen the Bug report if necessary, and/or fix the problem forthwith. (NB: If you are a system administrator and have no idea what this message is talking about, this may indicate a serious mail system misconfiguration somewhere. Please contact ow...@bugs.debian.org immediately.) -- 878249: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=878249 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Bug#878757: marked as done (Openvswitch must started before networking servise)
Your message dated Thu, 26 Oct 2017 09:00:34 + with message-id and subject line Bug#878757: fixed in openvswitch 2.8.1+dfsg1-1 has caused the Debian Bug report #878757, regarding Openvswitch must started before networking servise to be marked as done. This means that you claim that the problem has been dealt with. If this is not the case it is now your responsibility to reopen the Bug report if necessary, and/or fix the problem forthwith. (NB: If you are a system administrator and have no idea what this message is talking about, this may indicate a serious mail system misconfiguration somewhere. Please contact ow...@bugs.debian.org immediately.) -- 878757: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=878757 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 5/7] timeval: Introduce time_usec().
On 26.10.2017 10:12, Ilya Maximets wrote: > On 25.10.2017 20:28, Ben Pfaff wrote: >> On Fri, Oct 13, 2017 at 01:03:18PM +, Bodireddy, Bhanuprakash wrote: This fanction will provide monotonic time in microseconds. >>> >>> [BHANU] Typo here with function. >>> Signed-off-by: Ilya Maximets --- lib/timeval.c | 22 ++ lib/timeval.h | 2 ++ 2 files changed, 24 insertions(+) diff --git a/lib/timeval.c b/lib/timeval.c index dd63f03..be2eddc 100644 --- a/lib/timeval.c +++ b/lib/timeval.c @@ -233,6 +233,22 @@ time_wall_msec(void) return time_msec__(&wall_clock); } +static long long int +time_usec__(struct clock *c) +{ +struct timespec ts; + +time_timespec__(c, &ts); +return timespec_to_usec(&ts); +} + +/* Returns a monotonic timer, in microseconds. */ long long int +time_usec(void) +{ +return time_usec__(&monotonic_clock); } + >>> >>> [BHANU] As you are introducing the support for microsecond granularity, >>> can you also add time_wall_usec() and time_wall_usec__() here? > > I'm not sure what you meant under 'time_wall_usec__()'. There is no such > function for msec. > >>> The ipfix code (ipfix_now()) can be the first one to use it for now. May be >>> more in the future! >>> /* Configures the program to die with SIGALRM 'secs' seconds from now, if * 'secs' is nonzero, or disables the feature if 'secs' is zero. */ void @@ -360,6 +376,12 @@ timeval_to_msec(const struct timeval *tv) return (long long int) tv->tv_sec * 1000 + tv->tv_usec / 1000; } +long long int +timespec_to_usec(const struct timespec *ts) { +return (long long int) ts->tv_sec * 1000 * 1000 + ts->tv_nsec / +1000; } + >>> >>> [BHANU] how about adding timeval_to_usec()? >>> Also it would be nice to have the usec_to_timespec() and >>> timeval_diff_usec() implemented to make this commit complete. >> >> I'd appreciate those changes too; with those changes, I'd be happy to >> apply this, independent of anything else in the series. > > > OK. Cool. > I'm going to implement additionally: > > time_wall_usec() > usec_to_timespec() > timeval_to_usec() > timeval_diff_usec() > > Please, correct me if I missed something. I figured out that usec_to_timespec() and timeval_diff_usec() are static and will produce 'defined but not used' warning. And, actually, they are used only for poll logging and time warping, so those are not important to have usec granularity. I will add only two functions: time_wall_usec() timeval_to_usec() because they are part of the public timeval interface. > Best regards, Ilya Maximets. > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2] Add dl_type to flow metadata for correct interpretation of conntrack metadata
When a packet is sent to the controller, dl_type is not stored in the 'ofputil_packet_in_private'. When the packet is resumed, the flow's dl_type is 0 and this leads to invalid value in ct_orig_tuple in the pkt_metadata. This patch adds the dl_type to the metadata so that conntrack information can be interpreted correctly when packets are resumed. This is a change from the ordinary practice, since flow_get_metadata() is only supposed to deal with metadata and dl_type is not metadata. It is necessary when ct_state is involved, though, because ct_state only applies in the case of particular Ethertypes (IPv4 and IPv6 currently), so we need to add it as a kind of prerequisite. (This isn't ideal; maybe we didn't think through the ct_state mechanism carefully enough.) Reported-by: Daniel Alvarez Sanchez Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2017-October/339868.html Signed-off-by: Daniel Alvarez Signed-off-by: Numan Siddique --- lib/flow.c| 3 +++ tests/ofproto-dpif.at | 30 +++--- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/lib/flow.c b/lib/flow.c index b2b10aa48..4d2b7747a 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -1073,6 +1073,9 @@ flow_get_metadata(const struct flow *flow, struct match *flow_metadata) if (flow->ct_state != 0) { match_set_ct_state(flow_metadata, flow->ct_state); +/* Match dl_type since it is required for the later interpretation of + * the conntrack metadata. */ +match_set_dl_type(flow_metadata, flow->dl_type); if (is_ct_valid(flow, NULL, NULL) && flow->ct_nw_proto != 0) { if (flow->dl_type == htons(ETH_TYPE_IP)) { match_set_ct_nw_src(flow_metadata, flow->ct_nw_src); diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 28a7e827c..c75a1acbf 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -8952,7 +8952,7 @@ AT_CHECK([cat ofctl_monitor.log], [0], [dnl NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x1,reg4=0x1,in_port=1 (via action) data_len=42 (unbuffered) udp,vlan_tci=0x,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=1,tp_dst=2 udp_csum:e9d6 dnl -NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 ct_state=est|rpl|trk,ct_zone=1,ct_mark=0x1,ct_label=0x4d2,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x2,reg4=0x1,in_port=2 (via action) data_len=42 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 ct_state=est|rpl|trk,ct_zone=1,ct_mark=0x1,ct_label=0x4d2,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,ip,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x2,reg4=0x1,in_port=2 (via action) data_len=42 (unbuffered) udp,vlan_tci=0x,dl_src=50:54:00:00:00:0a,dl_dst=50:54:00:00:00:09,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=2,tp_dst=1 udp_csum:e9d6 ]) @@ -8973,7 +8973,7 @@ AT_CHECK([cat ofctl_monitor.log], [0], [dnl NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x1,reg4=0x1,in_port=1 (via action) data_len=42 (unbuffered) udp,vlan_tci=0x,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=3,tp_dst=2 udp_csum:e9d4 dnl -NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 ct_state=est|rpl|trk,ct_zone=1,ct_mark=0x1,ct_label=0x4d2,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=3,ct_tp_dst=2,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x2,reg4=0x1,in_port=2 (via action) data_len=42 (unbuffered) +NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 ct_state=est|rpl|trk,ct_zone=1,ct_mark=0x1,ct_label=0x4d2,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=3,ct_tp_dst=2,ip,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x2,reg4=0x1,in_port=2 (via action) data_len=42 (unbuffered) udp,vlan_tci=0x,dl_src=50:54:00:00:00:0a,dl_dst=50:54:00:00:00:09,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=2,tp_dst=3 udp_csum:e9d4 ]) @@ -9105,7 +9105,7 @@ dnl happens because the ct_state field is available only after recirc. AT_CHECK([cat ofctl_monitor.log], [0], [dnl NXT_PACKET_IN (xid=0x0): cookie=0x0 total_len=62 in_port=1 (via action) data_len=62 (unbuffered) udp6,vlan_tci=0x,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,ipv6_src=2001:db8::1,ipv6_dst=2001:db8::2,ipv6_label=0x0,nw_tos=112,nw_ecn=0,nw_ttl=128,tp_src=1,tp_dst=2 udp_csum:a466 -NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=62 ct_state=est|rpl|trk,ct_ipv6_src=2001:db8::1,ct_ipv6_dst=2001:db8::2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,in_port=2 (via action) data_len=62 (unbuffered) +NXT_PACKET_
Re: [ovs-dev] [ovs-discuss] [OVN] OVN doesn't work using OVS 2.8.1 on Centos 7.3 using conntrack
Thanks Ben for the review. I have fixed the tests (basically, now ip or ipv6 is added to the flows) and submitted a v2 of the patch. Daniel On Wed, Oct 25, 2017 at 7:21 PM, Ben Pfaff wrote: > On Wed, Oct 25, 2017 at 06:50:29PM +0530, Numan Siddique wrote: > > On Wed, Oct 25, 2017 at 3:09 PM, Daniel Alvarez Sanchez < > dalva...@redhat.com > > > wrote: > > > > > > > > > > > On Tue, Oct 24, 2017 at 11:35 PM, Ben Pfaff wrote: > > > > > >> On Tue, Oct 24, 2017 at 02:27:59PM -0700, Ben Pfaff wrote: > > >> > On Tue, Oct 24, 2017 at 11:07:58PM +0200, Daniel Alvarez Sanchez > wrote: > > >> > > Hi guys, > > >> > > > > >> > > Great job Numan! > > >> > > As we discussed over IRC, the patch below may make more sense. > > >> > > It essentially sets the dl_type so that when packet comes from the > > >> > > controller, it matches > > >> > > a valid type and OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4 is not added. > > >> > > Maybe what Numan proposed and this patch could be a good > combination > > >> but I > > >> > > think > > >> > > that we definitely need to set the dl_type as it's later checked > in > > >> > > pkt_metadata_from_flow() > > >> > > and it'll be zero there otherwise. > > >> > > What do you guys think? I have tried the patch below and the > kernel > > >> error > > >> > > is not shown > > >> > > anymore when issuing DHCP requests. > > >> > > > > >> > > > > >> > > diff --git a/lib/flow.c b/lib/flow.c > > >> > > index b2b10aa..62b948f 100644 > > >> > > --- a/lib/flow.c > > >> > > +++ b/lib/flow.c > > >> > > @@ -1073,6 +1073,7 @@ flow_get_metadata(const struct flow *flow, > > >> struct > > >> > > match *flow_metadata) > > >> > > > > >> > > if (flow->ct_state != 0) { > > >> > > match_set_ct_state(flow_metadata, flow->ct_state); > > >> > > +match_set_dl_type(flow_metadata, flow->dl_type); > > >> > > if (is_ct_valid(flow, NULL, NULL) && flow->ct_nw_proto > != 0) > > >> { > > >> > > if (flow->dl_type == htons(ETH_TYPE_IP)) { > > >> > > match_set_ct_nw_src(flow_metadata, > flow->ct_nw_src); > > >> > > > >> > This patch seems reasonable too. > > >> > > > >> > I recommend adding a comment above it to explain what's going on, > > >> > because dl_type is not a metadata field and it's confusing to deal > with > > >> > it in a context that's supposed to be all about metadata. I guess > the > > >> > comment would essentially say that dl_type is essential to the > > >> > interpretation of the conntrack metadata. > > >> > > >> Oh, and for this patch too I'd welcome a formal patch proposal. > > >> > > > > > > Done. Thanks a lot Ben. > > > If this get merged, it would be great if we can get it into 2.8 branch > and > > > add a new tag (2.8.2). > > > > > > Thanks!! > > > > > > > Ben, we have submitted both the patches. The patch from Daniel - ( > > https://patchwork.ozlabs.org/patch/830160/) will fix the issue. > > Not sure if this patch https://patchwork.ozlabs.org/patch/830132/ is > > needed any more. > > > > Request to review these patches if possible as RDO is blocked on these > > patches before we can update from OVS 2.7.2 to OVS 2.8(.2) > > I've reviewed both. I wasn't able to immediately apply either one, but > they're obviously moving in the right direction, so I'd appreciate > follow-up from both of you so that we can get them in. > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 3/3] system-traffic: Add conntrack floating IP test
Hi Eric, Thanks for the reply. >> >> dnl means the datapath didn't process the ct_clear action. Ending in >> >> SYN_RECV >> >> dnl (OVS maps to ESTABLISHED) means the initial frame was committed, but >> >> not a >> >> dnl second time after the FIP translation (because ct_clear didn't >> >> occur). >> >> -NS_CHECK_EXEC([at_ns1], [echo "foobar" |nc $NC_EOF_OPT 10.254.254.1]) >> >> +NS_CHECK_EXEC([at_ns1], [echo "foobar" |nc $NC_EOF_OPT 10.254.254.1 >> >> 1234]) >> > >> > Are you saying it works for you with the explicit port? Yes, it works for me with explicit port (sorry I didn't answer this question) >> > If not, can you show the error? >> > >> > FWIW, nmap-ncat defaults to 31337. I guess other nc implementations may >> > be different. >> >> The error message: >> >> 83: conntrack - floating IP FAILED >> (system-traffic.at:4055) >> +This is nc from the netcat-openbsd package. An alternative nc is available >> +in the netcat-traditional package. >> +usage: nc [-46bCDdhjklnrStUuvZz] [-I length] [-i interval] [-O length] >> + [-P proxy_username] [-p source_port] [-q seconds] [-s source] >> + [-T toskeyword] [-V rtable] [-w timeout] [-X proxy_protocol] >> + [-x proxy_address[:port]] [destination] [port] >> ./system-traffic.at:4055: exit code was 1, expected 0 >> >> on my system (ubuntu 1604), it is using the BSD >> tests/atlocal.in:NC_EOF_OPT="-q 1 -w 5" > > There are other test cases that use NC_EOF_OPT, so I expect those are > failing for you as well. Can you verify? > So my other test cases work OK since it has explicit port number. > Indeed, OpenBSD nc(1) man page does not list the -q option. Nor do the > other BSDs. I don't know why that option is being used. The first Actually my BSD nc (netcat-traditional package) has -q options. > evidence of it's usage seems to be 9ac0aadab9f9 ("conntrack: Add support > for NAT."). I think "-q 1" is mentioned at b54971f72ec ("system-traffic: use appropriate nc options for installed version") William ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 3/3] system-traffic: Add conntrack floating IP test
On Thu, Oct 26, 2017 at 06:06:15AM -0700, William Tu wrote: > Hi Eric, > Thanks for the reply. > > >> >> dnl means the datapath didn't process the ct_clear action. Ending in > >> >> SYN_RECV > >> >> dnl (OVS maps to ESTABLISHED) means the initial frame was committed, > >> >> but not a > >> >> dnl second time after the FIP translation (because ct_clear didn't > >> >> occur). > >> >> -NS_CHECK_EXEC([at_ns1], [echo "foobar" |nc $NC_EOF_OPT 10.254.254.1]) > >> >> +NS_CHECK_EXEC([at_ns1], [echo "foobar" |nc $NC_EOF_OPT 10.254.254.1 > >> >> 1234]) > >> > > >> > Are you saying it works for you with the explicit port? > Yes, it works for me with explicit port (sorry I didn't answer this question) Perfect. I'll send a v2 with only this change. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto-dpif: Delete system tunnel interface when remove ovs bridge
On Wed, Oct 25, 2017 at 11:41:27AM +0800, ju...@redhat.com wrote: > When there is only one bridge,create tunnel in the bridge, > then delete the bridge directly. the system tunnel interface > still in. > > Cause of only one bridge, backer->refcount values 1, when > delete bridge, "close_dpif_backer" will delete the backer, > so type_run will return directly, doesn't delete the interface. > This patch delete the system interface before free the backer. I'll add a bit more explanation.. This occurs when a tunnel is created with rtnetlink. With the compat API the tunnel is created via the vport tunnel interface, so it can be implicitly cleaned up by the kernel when the dp is closed or the module unloaded. But with rtnetlink the kernel module is not involved with the tunnel device creation (it's added to OVS as a netdev vport), so userspace needs to explicitly clean up the tunnel backers - type_run can't garbage collect them if the dpif is already deleted. > > Fixes: 921c370a9df5 ("dpif-netlink: Probe for out-of-tree tunnels, decides > used interface") > Signed-off-by: JunhanYan > --- > ofproto/ofproto-dpif.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index 43d670a..72993a4 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -646,6 +646,8 @@ dealloc(struct ofproto *ofproto_) > static void > close_dpif_backer(struct dpif_backer *backer, bool del) > { > +struct simap_node *node; > + > ovs_assert(backer->refcount > 0); > > if (--backer->refcount) { > @@ -654,6 +656,9 @@ close_dpif_backer(struct dpif_backer *backer, bool del) > > udpif_destroy(backer->udpif); > > +SIMAP_FOR_EACH (node, &backer->tnl_backers) { > +dpif_port_del(backer->dpif, u32_to_odp(node->data)); > +} > simap_destroy(&backer->tnl_backers); > ovs_rwlock_destroy(&backer->odp_to_ofport_lock); > hmap_destroy(&backer->odp_to_ofport_map); > -- > 2.9.5 > I didn't run the testsuite, but this looks okay to me. Acked-by: Eric Garver ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Fix ovs_geneve probing after restart
On Wed, Oct 25, 2017 at 10:39 AM, Eric Garver wrote: > On Tue, Oct 24, 2017 at 02:43:31PM -0700, William Tu wrote: > > Hi William, > Thanks for taking a look at this. > >> When using the out-of-tree (openvswitch compat) geneve module, >> the first time oot tunnel probing returns true (correct). >> Without unloading the geneve module, if the userspace ovs-vswitchd >> restarts, because the 'geneve_sys_6081' still exists, the probing >> incorrectly returns false and loads the in-tree (upstream kernel) >> geneve module. > > The reason for this is because rtnl sees the existing device and tries > to call ->changelink, but since the out-of-tree tunnel has no handler it > returns EOPNOTSUPP. > >> The patch fixes it by adding NLM_F_EXCL flags, so if geneve already >> exists, the dpif_netlink_rtnl_create return EEXIST and the oot tunnel >> probing returns true. To reproduce the issue, start the ovs > > While this fixes this scenario, it breaks another; an existing device > in-tree/kernel geneve tunnel. We'll end up with the opposite occurring - > it returns true when it should return false. > > So in addition to adding NLM_F_EXCL, this needs to also try to delete > the existing device in the case of EEXISTS, then restart the probe. > You're right, thanks! How about we unconditionally delete the device before the probe?. I'm simply adding one line without using NLM_F_EXCL: + dpif_netlink_rtnl_destroy(name); error = dpif_netlink_rtnl_create(tnl_cfg, name, OVS_VPORT_TYPE_GENEVE, "ovs_geneve", (NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE)); I'm testing using in-tree or out-of-tree and the result is consistent. Regards, William ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 5/7] timeval: Introduce time_usec().
On Thu, Oct 26, 2017 at 10:12:19AM +0300, Ilya Maximets wrote: > On 25.10.2017 20:28, Ben Pfaff wrote: > > On Fri, Oct 13, 2017 at 01:03:18PM +, Bodireddy, Bhanuprakash wrote: > >>> This fanction will provide monotonic time in microseconds. > >> > >> [BHANU] Typo here with function. > >> > >>> > >>> Signed-off-by: Ilya Maximets > >>> --- > >>> lib/timeval.c | 22 ++ lib/timeval.h | 2 ++ > >>> 2 files changed, 24 insertions(+) > >>> > >>> diff --git a/lib/timeval.c b/lib/timeval.c index dd63f03..be2eddc 100644 > >>> --- a/lib/timeval.c > >>> +++ b/lib/timeval.c > >>> @@ -233,6 +233,22 @@ time_wall_msec(void) > >>> return time_msec__(&wall_clock); > >>> } > >>> > >>> +static long long int > >>> +time_usec__(struct clock *c) > >>> +{ > >>> +struct timespec ts; > >>> + > >>> +time_timespec__(c, &ts); > >>> +return timespec_to_usec(&ts); > >>> +} > >>> + > >>> +/* Returns a monotonic timer, in microseconds. */ long long int > >>> +time_usec(void) > >>> +{ > >>> +return time_usec__(&monotonic_clock); } > >>> + > >> > >> [BHANU] As you are introducing the support for microsecond granularity, > >> can you also add time_wall_usec() and time_wall_usec__() here? > > I'm not sure what you meant under 'time_wall_usec__()'. There is no such > function for msec. > > >> The ipfix code (ipfix_now()) can be the first one to use it for now. May > >> be more in the future! > >> > >>> /* Configures the program to die with SIGALRM 'secs' seconds from now, if > >>> * 'secs' is nonzero, or disables the feature if 'secs' is zero. */ void > >>> @@ -360,6 > >>> +376,12 @@ timeval_to_msec(const struct timeval *tv) > >>> return (long long int) tv->tv_sec * 1000 + tv->tv_usec / 1000; } > >>> > >>> +long long int > >>> +timespec_to_usec(const struct timespec *ts) { > >>> +return (long long int) ts->tv_sec * 1000 * 1000 + ts->tv_nsec / > >>> +1000; } > >>> + > >> > >> [BHANU] how about adding timeval_to_usec()? > >> Also it would be nice to have the usec_to_timespec() and > >> timeval_diff_usec() implemented to make this commit complete. > > > > I'd appreciate those changes too; with those changes, I'd be happy to > > apply this, independent of anything else in the series. > > > OK. Cool. > I'm going to implement additionally: > > time_wall_usec() > usec_to_timespec() > timeval_to_usec() > timeval_diff_usec() > > Please, correct me if I missed something. Thanks, that's what I mean, and I'd appreciate it. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 5/7] timeval: Introduce time_usec().
On Thu, Oct 26, 2017 at 01:57:59PM +0300, Ilya Maximets wrote: > On 26.10.2017 10:12, Ilya Maximets wrote: > > On 25.10.2017 20:28, Ben Pfaff wrote: > >> On Fri, Oct 13, 2017 at 01:03:18PM +, Bodireddy, Bhanuprakash wrote: > This fanction will provide monotonic time in microseconds. > >>> > >>> [BHANU] Typo here with function. > >>> > > Signed-off-by: Ilya Maximets > --- > lib/timeval.c | 22 ++ lib/timeval.h | 2 ++ > 2 files changed, 24 insertions(+) > > diff --git a/lib/timeval.c b/lib/timeval.c index dd63f03..be2eddc 100644 > --- a/lib/timeval.c > +++ b/lib/timeval.c > @@ -233,6 +233,22 @@ time_wall_msec(void) > return time_msec__(&wall_clock); > } > > +static long long int > +time_usec__(struct clock *c) > +{ > +struct timespec ts; > + > +time_timespec__(c, &ts); > +return timespec_to_usec(&ts); > +} > + > +/* Returns a monotonic timer, in microseconds. */ long long int > +time_usec(void) > +{ > +return time_usec__(&monotonic_clock); } > + > >>> > >>> [BHANU] As you are introducing the support for microsecond granularity, > >>> can you also add time_wall_usec() and time_wall_usec__() here? > > > > I'm not sure what you meant under 'time_wall_usec__()'. There is no such > > function for msec. > > > >>> The ipfix code (ipfix_now()) can be the first one to use it for now. May > >>> be more in the future! > >>> > /* Configures the program to die with SIGALRM 'secs' seconds from now, if > * 'secs' is nonzero, or disables the feature if 'secs' is zero. */ > void @@ -360,6 > +376,12 @@ timeval_to_msec(const struct timeval *tv) > return (long long int) tv->tv_sec * 1000 + tv->tv_usec / 1000; } > > +long long int > +timespec_to_usec(const struct timespec *ts) { > +return (long long int) ts->tv_sec * 1000 * 1000 + ts->tv_nsec / > +1000; } > + > >>> > >>> [BHANU] how about adding timeval_to_usec()? > >>> Also it would be nice to have the usec_to_timespec() and > >>> timeval_diff_usec() implemented to make this commit complete. > >> > >> I'd appreciate those changes too; with those changes, I'd be happy to > >> apply this, independent of anything else in the series. > > > > > > OK. Cool. > > I'm going to implement additionally: > > > > time_wall_usec() > > usec_to_timespec() > > timeval_to_usec() > > timeval_diff_usec() > > > > Please, correct me if I missed something. > > I figured out that usec_to_timespec() and timeval_diff_usec() are static and > will produce 'defined but not used' warning. And, actually, they are used only > for poll logging and time warping, so those are not important to have usec > granularity. > I will add only two functions: > > time_wall_usec() > timeval_to_usec() > > because they are part of the public timeval interface. OK. It's only the public interface I care about. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] Add dl_type to flow metadata for correct interpretation of conntrack metadata
On Thu, Oct 26, 2017 at 02:52:22PM +0200, Daniel Alvarez wrote: > When a packet is sent to the controller, dl_type is not stored in the > 'ofputil_packet_in_private'. When the packet is resumed, the flow's > dl_type is 0 and this leads to invalid value in ct_orig_tuple in the > pkt_metadata. > > This patch adds the dl_type to the metadata so that conntrack > information can be interpreted correctly when packets are resumed. > > This is a change from the ordinary practice, since flow_get_metadata() is > only supposed to deal with metadata and dl_type is not metadata. It is > necessary when ct_state is involved, though, because ct_state only applies > in the case of particular Ethertypes (IPv4 and IPv6 currently), so we need > to add it as a kind of prerequisite. (This isn't ideal; maybe we didn't > think through the ct_state mechanism carefully enough.) > > Reported-by: Daniel Alvarez Sanchez > Reported-at: > https://mail.openvswitch.org/pipermail/ovs-dev/2017-October/339868.html > Signed-off-by: Daniel Alvarez > Signed-off-by: Numan Siddique Thanks a lot. I applied this to master and branch-2.8. Let me know if it should also go to older branches. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] OVN: Don't let peers be set to "" on port bindings.
There are a couple of places in ovn-northd that set the "peer" option on certain ports to "" in certain cases. In every case where a peer is looked up on a port binding, the code performs a NULL check in order to ensure a peer exists. None check for the "" string. They assume that the presence of a peer string means a peer is defined and all is well. In the past (OVS 2.6 series), this sometimes led to patch ports being created in ovs that had names like "patch-ro-to-". This particular problem resolved itself in OVS 2.7 since such patch ports were no longer automatically created. However, by naming the peer "" the seeds are still sown for similar issues to occur. The solution this patch suggests is to no longer set the "peer" option on a port binding to "". Instead, if no peer can be set, then we set no peer. Since other code is already equipped to deal with this, this poses no problem. Signed-off-by: Mark Michelson --- ovn/northd/ovn-northd.c | 27 --- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index 2db238073..ec981541e 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -1924,8 +1924,9 @@ ovn_port_update_sbrec(struct northd_context *ctx, } smap_add(&new, "distributed-port", op->nbrp->name); } else { -const char *peer = op->peer ? op->peer->key : ""; -smap_add(&new, "peer", peer); +if (op->peer) { +smap_add(&new, "peer", op->peer->key); +} if (chassis_name) { smap_add(&new, "l3gateway-chassis", chassis_name); } @@ -1978,16 +1979,20 @@ ovn_port_update_sbrec(struct northd_context *ctx, sbrec_port_binding_set_type(op->sb, "patch"); } -const char *router_port = smap_get_def(&op->nbsp->options, - "router-port", ""); -struct smap new; -smap_init(&new); -smap_add(&new, "peer", router_port); -if (chassis) { -smap_add(&new, "l3gateway-chassis", chassis); +const char *router_port = smap_get(&op->nbsp->options, + "router-port"); +if (router_port || chassis) { +struct smap new; +smap_init(&new); +if (router_port) { +smap_add(&new, "peer", router_port); +} +if (chassis) { +smap_add(&new, "l3gateway-chassis", chassis); +} +sbrec_port_binding_set_options(op->sb, &new); +smap_destroy(&new); } -sbrec_port_binding_set_options(op->sb, &new); -smap_destroy(&new); const char *nat_addresses = smap_get(&op->nbsp->options, "nat-addresses"); -- 2.13.5 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Fix ovs_geneve probing after restart
On Thu, Oct 26, 2017 at 09:12:14AM -0700, William Tu wrote: > On Wed, Oct 25, 2017 at 10:39 AM, Eric Garver wrote: > > On Tue, Oct 24, 2017 at 02:43:31PM -0700, William Tu wrote: > > > > Hi William, > > Thanks for taking a look at this. > > > >> When using the out-of-tree (openvswitch compat) geneve module, > >> the first time oot tunnel probing returns true (correct). > >> Without unloading the geneve module, if the userspace ovs-vswitchd > >> restarts, because the 'geneve_sys_6081' still exists, the probing > >> incorrectly returns false and loads the in-tree (upstream kernel) > >> geneve module. > > > > The reason for this is because rtnl sees the existing device and tries > > to call ->changelink, but since the out-of-tree tunnel has no handler it > > returns EOPNOTSUPP. > > > >> The patch fixes it by adding NLM_F_EXCL flags, so if geneve already > >> exists, the dpif_netlink_rtnl_create return EEXIST and the oot tunnel > >> probing returns true. To reproduce the issue, start the ovs > > > > While this fixes this scenario, it breaks another; an existing device > > in-tree/kernel geneve tunnel. We'll end up with the opposite occurring - > > it returns true when it should return false. > > > > So in addition to adding NLM_F_EXCL, this needs to also try to delete > > the existing device in the case of EEXISTS, then restart the probe. > > > > You're right, thanks! > How about we unconditionally delete the device before the probe?. I'm > simply adding one line without using NLM_F_EXCL: > > + dpif_netlink_rtnl_destroy(name); > error = dpif_netlink_rtnl_create(tnl_cfg, name, > OVS_VPORT_TYPE_GENEVE, > "ovs_geneve", > (NLM_F_REQUEST | NLM_F_ACK >| NLM_F_CREATE)); > > I'm testing using in-tree or out-of-tree and the result is consistent. > I think that will work. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Fix ovs_geneve probing after restart
On Thu, Oct 26, 2017 at 11:15 AM, Eric Garver wrote: > On Thu, Oct 26, 2017 at 09:12:14AM -0700, William Tu wrote: >> On Wed, Oct 25, 2017 at 10:39 AM, Eric Garver wrote: >> > On Tue, Oct 24, 2017 at 02:43:31PM -0700, William Tu wrote: >> > >> > Hi William, >> > Thanks for taking a look at this. >> > >> >> When using the out-of-tree (openvswitch compat) geneve module, >> >> the first time oot tunnel probing returns true (correct). >> >> Without unloading the geneve module, if the userspace ovs-vswitchd >> >> restarts, because the 'geneve_sys_6081' still exists, the probing >> >> incorrectly returns false and loads the in-tree (upstream kernel) >> >> geneve module. >> > >> > The reason for this is because rtnl sees the existing device and tries >> > to call ->changelink, but since the out-of-tree tunnel has no handler it >> > returns EOPNOTSUPP. >> > >> >> The patch fixes it by adding NLM_F_EXCL flags, so if geneve already >> >> exists, the dpif_netlink_rtnl_create return EEXIST and the oot tunnel >> >> probing returns true. To reproduce the issue, start the ovs >> > >> > While this fixes this scenario, it breaks another; an existing device >> > in-tree/kernel geneve tunnel. We'll end up with the opposite occurring - >> > it returns true when it should return false. >> > >> > So in addition to adding NLM_F_EXCL, this needs to also try to delete >> > the existing device in the case of EEXISTS, then restart the probe. >> > >> >> You're right, thanks! >> How about we unconditionally delete the device before the probe?. I'm >> simply adding one line without using NLM_F_EXCL: >> >> + dpif_netlink_rtnl_destroy(name); >> error = dpif_netlink_rtnl_create(tnl_cfg, name, >> OVS_VPORT_TYPE_GENEVE, >> "ovs_geneve", >> (NLM_F_REQUEST | NLM_F_ACK >>| NLM_F_CREATE)); >> >> I'm testing using in-tree or out-of-tree and the result is consistent. >> > > I think that will work. thanks, I will send out v2. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] DNS support feature (was: Re: DNS support options)
On Wed, Oct 25, 2017 at 4:16 PM Yifeng Sun wrote: > I feel that unbound stands out in the available open source DNS resolver. > > Below is the summary for unbound: > * The actual resolving work is done by a background process or thread. A > background process or thread seems unavoidable. Linux's getaddrinfo_a > clones a thread similarly. > * It is ported on Linux, BSD, Windows, MacOS/X and Solaris/SPARC. This is > good because OVS runs on a large range of platforms. > * It complies to the standard, with optional DNSSEC support. Some of its > features may not be needed in our case. > * The unbound context is thread-safe. Its internal locks may bring some > overhead. But since the DNS resolving is not frequent in OVS, I suppose > this small overhead is not an issue. > > Unbound looks like a good option. Another option is to create a background > thread which processes DNS resolving requests from the main thread and > sends back the resulting events to the main thread. This method is quite > simple and straightforward. > > The above are what I got so far. Please give your thoughts, thanks a lot. > If portability to all of the systems you mentioned in your second bullet point is important, then you can rule out a couple of options: * getaddrinfo_a is a GNU extension and is only available with glibc * The resolver functions[1] are a BSD specification so they'd be available on most platforms, but not on Windows. I don't personally recommend these because of the need to manually parse the DNS responses you receive. That leaves two options: * Run a background thread uses getaddrinfo() to perform resolution. * Use a third-party library (like unbound). Of these two options, I feel like the third-party library is the better option. The only downside I can think of is the extra dependency for OVS. And as far as what third-party library to use, I was the one that suggested unbound in the first place, so obviously I'm fine with using it :) Mark [1] http://man7.org/linux/man-pages/man3/resolver.3.html > > Below is the link for original discussion: > https://mail.openvswitch.org/pipermail/ovs-dev/2017-August/337038.html > > > > On Wed, Oct 25, 2017 at 2:11 PM, Ben Pfaff wrote: > >> Hello everyone, please allow me to introduce Yifeng Sun (CCed), who >> recently joined VMware's Open vSwitch team. I've asked Yifeng to start >> out by working on DNS support for Open vSwitch. Yifeng, can you tell us >> about what you've discovered so far, based on this thread from August >> that I'm reviving, and your tentative conclusions? >> >> Thanks, >> >> Ben. >> > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 0/3] Add dpif support for ct_clear action
This series adds dpif support for OVS_ACTION_ATTR_CT_CLEAR. Previously the ct_clear action was a userspace only operation, but it has use cases in the dpif as well. Namely changing a packet's tuple after a ct() lookup has occurred. Kernel support has already be accepted upstream: b8226962b1c4 ("openvswitch: add ct_clear action"). travis-ci: https://travis-ci.org/erig0/ovs/builds/290487503 v2: - Specify port for nc in test (patch 3) Eric Garver (3): dpif: Add support for OVS_ACTION_ATTR_CT_CLEAR system-common-macros: Check for ct_clear action in datapath system-traffic: Add conntrack floating IP test datapath/linux/compat/include/linux/openvswitch.h | 1 + lib/conntrack.c | 10 lib/conntrack.h | 1 + lib/dpif-netdev.c | 1 + lib/dpif.c| 1 + lib/odp-execute.c | 7 +++ lib/odp-util.c| 4 ++ lib/ofp-actions.c | 1 + ofproto/ofproto-dpif-ipfix.c | 1 + ofproto/ofproto-dpif-sflow.c | 1 + ofproto/ofproto-dpif-xlate.c | 14 - ofproto/ofproto-dpif.c| 32 ++ ofproto/ofproto-dpif.h| 5 +- tests/system-common-macros.at | 4 ++ tests/system-traffic.at | 73 +++ 15 files changed, 154 insertions(+), 2 deletions(-) -- 2.12.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 1/3] dpif: Add support for OVS_ACTION_ATTR_CT_CLEAR
This supports using the ct_clear action in the kernel datapath. To preserve compatibility with current ct_clear behavior on old kernels, we only pass this action down to the datapath if a probe reveals the datapath actually supports it. Signed-off-by: Eric Garver Acked-by: William Tu --- datapath/linux/compat/include/linux/openvswitch.h | 1 + lib/conntrack.c | 10 +++ lib/conntrack.h | 1 + lib/dpif-netdev.c | 1 + lib/dpif.c| 1 + lib/odp-execute.c | 7 + lib/odp-util.c| 4 +++ lib/ofp-actions.c | 1 + ofproto/ofproto-dpif-ipfix.c | 1 + ofproto/ofproto-dpif-sflow.c | 1 + ofproto/ofproto-dpif-xlate.c | 14 +- ofproto/ofproto-dpif.c| 32 +++ ofproto/ofproto-dpif.h| 5 +++- 13 files changed, 77 insertions(+), 2 deletions(-) diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h index bc6c94b8d52d..28f20103af81 100644 --- a/datapath/linux/compat/include/linux/openvswitch.h +++ b/datapath/linux/compat/include/linux/openvswitch.h @@ -924,6 +924,7 @@ enum ovs_action_attr { OVS_ACTION_ATTR_TRUNC,/* u32 struct ovs_action_trunc. */ OVS_ACTION_ATTR_PUSH_ETH, /* struct ovs_action_push_eth. */ OVS_ACTION_ATTR_POP_ETH, /* No argument. */ + OVS_ACTION_ATTR_CT_CLEAR, /* No argument. */ #ifndef __KERNEL__ OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/ diff --git a/lib/conntrack.c b/lib/conntrack.c index e555b5501da9..ddd6de4daff8 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -1242,6 +1242,16 @@ conntrack_execute(struct conntrack *ct, struct dp_packet_batch *pkt_batch, return 0; } +int +conntrack_clear(struct dp_packet *packet) +{ +/* According to pkt_metadata_init(), ct_state == 0 is enough to make all of + * the conntrack fields invalid. */ +packet->md.ct_state = 0; + +return 0; +} + static void set_mark(struct dp_packet *pkt, struct conn *conn, uint32_t val, uint32_t mask) { diff --git a/lib/conntrack.h b/lib/conntrack.h index fbeef1c4754e..6c19f3c65804 100644 --- a/lib/conntrack.h +++ b/lib/conntrack.h @@ -97,6 +97,7 @@ int conntrack_execute(struct conntrack *, struct dp_packet_batch *, const char *helper, const struct nat_action_info_t *nat_action_info, long long now); +int conntrack_clear(struct dp_packet *packet); struct conntrack_dump { struct conntrack *ct; diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index d5eb8305c8a2..a3046b259c2e 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -5640,6 +5640,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_, case OVS_ACTION_ATTR_CLONE: case OVS_ACTION_ATTR_ENCAP_NSH: case OVS_ACTION_ATTR_DECAP_NSH: +case OVS_ACTION_ATTR_CT_CLEAR: case __OVS_ACTION_ATTR_MAX: OVS_NOT_REACHED(); } diff --git a/lib/dpif.c b/lib/dpif.c index 79b2e6c97305..febeb816e4c4 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -1273,6 +1273,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_, case OVS_ACTION_ATTR_CLONE: case OVS_ACTION_ATTR_ENCAP_NSH: case OVS_ACTION_ATTR_DECAP_NSH: +case OVS_ACTION_ATTR_CT_CLEAR: case OVS_ACTION_ATTR_UNSPEC: case __OVS_ACTION_ATTR_MAX: OVS_NOT_REACHED(); diff --git a/lib/odp-execute.c b/lib/odp-execute.c index 5f4d23a91a3e..01ac62b25bca 100644 --- a/lib/odp-execute.c +++ b/lib/odp-execute.c @@ -34,6 +34,7 @@ #include "unaligned.h" #include "util.h" #include "csum.h" +#include "conntrack.h" /* Masked copy of an ethernet address. 'src' is already properly masked. */ static void @@ -654,6 +655,7 @@ requires_datapath_assistance(const struct nlattr *a) case OVS_ACTION_ATTR_CLONE: case OVS_ACTION_ATTR_ENCAP_NSH: case OVS_ACTION_ATTR_DECAP_NSH: +case OVS_ACTION_ATTR_CT_CLEAR: return false; case OVS_ACTION_ATTR_UNSPEC: @@ -837,6 +839,11 @@ odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal, } break; } +case OVS_ACTION_ATTR_CT_CLEAR: +DP_PACKET_BATCH_FOR_EACH (packet, batch) { +conntrack_clear(packet); +} +break; case OVS_ACTION_ATTR_OUTPUT: case OVS_ACTION_ATTR_TUNNEL_PUSH: diff --git a/lib/odp-util.c b/lib/odp-util.c index 6304b3dd299a..83b936d2a0fd 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -126,6 +126,7 @@ odp_action_len(uint16_t type) case OVS_ACTION_ATTR_SET_MASKED: return ATTR_LEN_VARIABLE; case OVS_ACTION_AT
[ovs-dev] [PATCH v2 2/3] system-common-macros: Check for ct_clear action in datapath
New macro OVS_CHECK_CT_CLEAR() to check if ct_clear action is supported by the datapath. Signed-off-by: Eric Garver Tested-by: William Tu --- tests/system-common-macros.at | 4 1 file changed, 4 insertions(+) diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at index 73ae4829dac4..f7d4adb947a0 100644 --- a/tests/system-common-macros.at +++ b/tests/system-common-macros.at @@ -319,3 +319,7 @@ m4_define([OVS_CHECK_8021AD], # OVS_CHECK_IPROUTE_ENCAP() m4_define([OVS_CHECK_IPROUTE_ENCAP], [AT_SKIP_IF([! ip route help 2>&1 |grep encap >/dev/null])]) + +# OVS_CHECK_CT_CLEAR() +m4_define([OVS_CHECK_CT_CLEAR], +[AT_SKIP_IF([! grep -q "Datapath supports ct_clear action" ovs-vswitchd.log])]) -- 2.12.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 3/3] system-traffic: Add conntrack floating IP test
This test cases uses floating IP (FIP) addresses for each endpoint. If the destination is a FIP, the packet will undergo a transformation of the form (dst=FIP, src=non-FIP) --> (dst=non-FIP, src=FIP) before egress. Otherwise the packet is untouched. This exercises the ct_clear action in the datapath. Signed-off-by: Eric Garver --- tests/system-traffic.at | 73 + 1 file changed, 73 insertions(+) diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 522eaa615834..cf915d6be7cd 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -3996,6 +3996,79 @@ ovs-ofctl -O OpenFlow15 dump-group-stats br0 OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([conntrack - floating IP]) +AT_SKIP_IF([test $HAVE_NC = no]) +CHECK_CONNTRACK() +OVS_TRAFFIC_VSWITCHD_START() +OVS_CHECK_CT_CLEAR() + +ADD_NAMESPACES(at_ns0, at_ns1) +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "f0:00:00:01:01:01") dnl FIP 10.254.254.1 +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "f0:00:00:01:01:02") dnl FIP 10.254.254.2 + +dnl Static ARPs +NS_CHECK_EXEC([at_ns0], [ip neigh add 10.1.1.2 lladdr f0:00:00:01:01:02 dev p0]) +NS_CHECK_EXEC([at_ns1], [ip neigh add 10.1.1.1 lladdr f0:00:00:01:01:01 dev p1]) + +dnl Static ARP and route entries for the FIP "gateway" +NS_CHECK_EXEC([at_ns0], [ip neigh add 10.1.1.254 lladdr f0:00:00:01:01:FE dev p0]) +NS_CHECK_EXEC([at_ns1], [ip neigh add 10.1.1.254 lladdr f0:00:00:01:01:FE dev p1]) +NS_CHECK_EXEC([at_ns0], [ip route add default nexthop via 10.1.1.254]) +NS_CHECK_EXEC([at_ns1], [ip route add default nexthop via 10.1.1.254]) + +NETNS_DAEMONIZE([at_ns0], [nc -l -k 1234 > /dev/null], [nc0.pid]) + +AT_DATA([flows.txt], [dnl +table=0,priority=10 ip action=ct(table=1) +table=0,priority=1 action=drop +dnl dst FIP +table=1,priority=20 ip,ct_state=+trk+est,nw_dst=10.254.254.0/24 action=goto_table:10 +table=1,priority=20 ip,ct_state=+trk+new,nw_dst=10.254.254.0/24 action=ct(commit,table=10) +dnl dst local +table=1,priority=10 ip,ct_state=+trk+est action=goto_table:20 +table=1,priority=10 ip,ct_state=+trk+new action=ct(commit,table=20) +table=1,priority=1 ip,ct_state=+trk+inv action=drop +dnl +dnl FIP translation (dst FIP, src local) --> (dst local, src FIP) +table=10 ip,nw_dst=10.254.254.1 action=set_field:10.1.1.1->nw_dst,goto_table:11 +table=10 ip,nw_dst=10.254.254.2 action=set_field:10.1.1.2->nw_dst,goto_table:11 +table=11 ip,nw_src=10.1.1.1 action=set_field:10.254.254.1->nw_src,goto_table:12 +table=11 ip,nw_src=10.1.1.2 action=set_field:10.254.254.2->nw_src,goto_table:12 +dnl clear conntrack and do another lookup since we changed the tuple +table=12,priority=10 ip action=ct_clear,ct(table=13) +table=12,priority=1 action=drop +table=13 ip,ct_state=+trk+est action=goto_table:20 +table=13 ip,ct_state=+trk+new action=ct(commit,table=20) +table=13 ip,ct_state=+trk+inv action=drop +dnl +dnl Output +table=20 ip,nw_src=10.1.1.1 action=set_field:f0:00:00:01:01:01->eth_src,goto_table:21 +table=20 ip,nw_src=10.1.1.2 action=set_field:f0:00:00:01:01:02->eth_src,goto_table:21 +table=20 ip,nw_src=10.254.254.0/24 action=set_field:f0:00:00:01:01:FE->eth_src,goto_table:21 +table=21 ip,nw_dst=10.1.1.1 action=set_field:f0:00:00:01:01:01->eth_dst,output:ovs-p0 +table=21 ip,nw_dst=10.1.1.2 action=set_field:f0:00:00:01:01:02->eth_dst,output:ovs-p1 +]) + +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) + +dnl non-FIP case +NS_CHECK_EXEC([at_ns1], [echo "foobar" |nc $NC_EOF_OPT 10.1.1.1 1234]) +OVS_WAIT_UNTIL([[ovs-appctl dpctl/dump-conntrack | sed -e 's/port=[0-9]*/port=/g' -e 's/id=[0-9]*/id=/g' | +grep "tcp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=,dport=),reply=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),protoinfo=(state=TIME_WAIT)" +]]) + +dnl Check that the full session ends as expected (i.e. TIME_WAIT). Otherwise it +dnl means the datapath didn't process the ct_clear action. Ending in SYN_RECV +dnl (OVS maps to ESTABLISHED) means the initial frame was committed, but not a +dnl second time after the FIP translation (because ct_clear didn't occur). +NS_CHECK_EXEC([at_ns1], [echo "foobar" |nc $NC_EOF_OPT 10.254.254.1 1234]) +OVS_WAIT_UNTIL([[ovs-appctl dpctl/dump-conntrack | sed -e 's/port=[0-9]*/port=/g' -e 's/id=[0-9]*/id=/g' | +grep "tcp,orig=(src=10.254.254.2,dst=10.1.1.1,sport=,dport=),reply=(src=10.1.1.1,dst=10.254.254.2,sport=,dport=),protoinfo=(state=TIME_WAIT)" +]]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP + AT_BANNER([802.1ad]) AT_SETUP([802.1ad - vlan_limit]) -- 2.12.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] dpif-netlink-rtnl: Fix ovs_geneve probing after restart
When using the out-of-tree (openvswitch compat) geneve module, the first time oot tunnel probing returns true (correct). Without unloading the geneve module, if the userspace ovs-vswitchd restarts, because the 'geneve_sys_6081' still exists, the probing incorrectly returns false and loads the in-tree (upstream kernel) geneve module. The issue also exists the other way around, where existing geneve module is in-tree and ovs-vswitchd restarts. The patch fixes it by unconditionally removing the tunnel device before the probing. To reproduce the issue, start the ovs > /etc/init.d/openvswitch-switch start > creat a bridge and attach a geneve port using out-of-tree geneve > /etc/init.d/openvswitch-switch restart Fixes: 921c370a9df5 ("dpif-netlink: Probe for out-of-tree tunnels, decides used interface") Signed-off-by: William Tu Cc: Eric Garver Cc: Gurucharan Shetty --- lib/dpif-netlink-rtnl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c index 0c32e7d8ccb4..148ce5ff3a3d 100644 --- a/lib/dpif-netlink-rtnl.c +++ b/lib/dpif-netlink-rtnl.c @@ -448,6 +448,8 @@ dpif_netlink_rtnl_probe_oot_tunnels(void) } name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf); +dpif_netlink_rtnl_destroy(name); + error = dpif_netlink_rtnl_create(tnl_cfg, name, OVS_VPORT_TYPE_GENEVE, "ovs_geneve", (NLM_F_REQUEST | NLM_F_ACK -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Fix ovs_geneve probing after restart
On Thu, Oct 26, 2017 at 12:31:03PM -0700, William Tu wrote: > When using the out-of-tree (openvswitch compat) geneve module, > the first time oot tunnel probing returns true (correct). > Without unloading the geneve module, if the userspace ovs-vswitchd > restarts, because the 'geneve_sys_6081' still exists, the probing > incorrectly returns false and loads the in-tree (upstream kernel) > geneve module. The issue also exists the other way around, where > existing geneve module is in-tree and ovs-vswitchd restarts. > > The patch fixes it by unconditionally removing the tunnel device > before the probing. To reproduce the issue, start the ovs > > /etc/init.d/openvswitch-switch start > > creat a bridge and attach a geneve port using out-of-tree geneve > > /etc/init.d/openvswitch-switch restart > > Fixes: 921c370a9df5 ("dpif-netlink: Probe for out-of-tree tunnels, decides > used interface") > Signed-off-by: William Tu > Cc: Eric Garver > Cc: Gurucharan Shetty > --- > lib/dpif-netlink-rtnl.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c > index 0c32e7d8ccb4..148ce5ff3a3d 100644 > --- a/lib/dpif-netlink-rtnl.c > +++ b/lib/dpif-netlink-rtnl.c > @@ -448,6 +448,8 @@ dpif_netlink_rtnl_probe_oot_tunnels(void) > } > > name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf); > +dpif_netlink_rtnl_destroy(name); > + > error = dpif_netlink_rtnl_create(tnl_cfg, name, > OVS_VPORT_TYPE_GENEVE, > "ovs_geneve", > (NLM_F_REQUEST | NLM_F_ACK > -- > 2.7.4 Thanks for the fix! Acked-by: Eric Garver ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] system-traffic: Fix conntrack tests
Three conntrack system-traffic tests are broken because of a recent change 7827edcaebd8 ("Add dl_type to flow metadata for correct interpretation of conntrack metadata"). It can be reproduced by $ make check-system-userspace TESTSUITEFLAGS='18 19 37' This patch modifies the check messages to fix the breakage. Fixes: 7827edcaebd8 ("Add dl_type to flow metadata for correct interpretation of conntrack metadata") CC: Daniel Alvarez Signed-off-by: Yi-Hung Wei --- tests/system-traffic.at | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 522eaa615834..fd7b6121b04f 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -764,7 +764,7 @@ dnl Check this output. We only see the latter two packets, not the first. AT_CHECK([cat ofctl_monitor.log], [0], [dnl NXT_PACKET_IN2 (xid=0x0): total_len=42 in_port=1 (via action) data_len=42 (unbuffered) udp,vlan_tci=0x,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=1,tp_dst=2 udp_csum:0 -NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=42 ct_state=est|rpl|trk,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,in_port=2 (via action) data_len=42 (unbuffered) +NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=42 ct_state=est|rpl|trk,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,ip,in_port=2 (via action) data_len=42 (unbuffered) udp,vlan_tci=0x,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=2,tp_dst=1 udp_csum:0 ]) @@ -810,7 +810,7 @@ dnl Check this output. We only see the latter two packets, not the first. AT_CHECK([cat ofctl_monitor.log], [0], [dnl NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=42 in_port=1 (via action) data_len=42 (unbuffered) udp,vlan_tci=0x,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=1,tp_dst=2 udp_csum:0 -NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=42 ct_state=new|trk,ct_nw_src=10.1.1.2,ct_nw_dst=10.1.1.1,ct_nw_proto=17,ct_tp_src=2,ct_tp_dst=1,in_port=2 (via action) data_len=42 (unbuffered) +NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=42 ct_state=new|trk,ct_nw_src=10.1.1.2,ct_nw_dst=10.1.1.1,ct_nw_proto=17,ct_tp_src=2,ct_tp_dst=1,ip,in_port=2 (via action) data_len=42 (unbuffered) udp,vlan_tci=0x,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=2,tp_dst=1 udp_csum:0 ]) @@ -1589,11 +1589,11 @@ AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 2 resubmit\(,0\) 'e64c473528c9c dnl Check this output. We only see the latter two packets, not the first. AT_CHECK([cat ofctl_monitor.log], [0], [dnl -NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=75 ct_state=inv|trk,in_port=2 (via action) data_len=75 (unbuffered) +NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=75 ct_state=inv|trk,ip,in_port=2 (via action) data_len=75 (unbuffered) icmp,vlan_tci=0x,dl_src=c6:f5:4e:cb:72:db,dl_dst=f6:4c:47:35:28:c9,nw_src=172.16.0.4,nw_dst=172.16.0.3,nw_tos=192,nw_ecn=0,nw_ttl=64,icmp_type=3,icmp_code=3 icmp_csum:da49 -NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=47 ct_state=new|trk,ct_nw_src=172.16.0.1,ct_nw_dst=172.16.0.2,ct_nw_proto=17,ct_tp_src=41614,ct_tp_dst=,in_port=1 (via action) data_len=47 (unbuffered) +NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=47 ct_state=new|trk,ct_nw_src=172.16.0.1,ct_nw_dst=172.16.0.2,ct_nw_proto=17,ct_tp_src=41614,ct_tp_dst=,ip,in_port=1 (via action) data_len=47 (unbuffered) udp,vlan_tci=0x,dl_src=e6:4c:47:35:28:c9,dl_dst=c6:f9:4e:cb:72:db,nw_src=172.16.0.1,nw_dst=172.16.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=41614,tp_dst= udp_csum:2096 -NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=75 ct_state=rel|rpl|trk,ct_nw_src=172.16.0.1,ct_nw_dst=172.16.0.2,ct_nw_proto=17,ct_tp_src=41614,ct_tp_dst=,in_port=2 (via action) data_len=75 (unbuffered) +NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=75 ct_state=rel|rpl|trk,ct_nw_src=172.16.0.1,ct_nw_dst=172.16.0.2,ct_nw_proto=17,ct_tp_src=41614,ct_tp_dst=,ip,in_port=2 (via action) data_len=75 (unbuffered) icmp,vlan_tci=0x,dl_src=c6:f9:4e:cb:72:db,dl_dst=e6:4c:47:35:28:c9,nw_src=172.16.0.2,nw_dst=172.16.0.1,nw_tos=192,nw_ecn=0,nw_ttl=64,icmp_type=3,icmp_code=3 icmp_csum:553f ]) -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb-idl: fix index row setting with references.
On Sat, Oct 14, 2017 at 11:53:23PM -0700, Han Zhou wrote: > IDL index should be able to be used without having to be in a > transaction. However, current implementation leads to crash if > a reference type column is being set in an index row for querying > purpose when it is not in a transaction. It is because of the > uninitialized arcs and unnecessary updates of the arcs. This patch > fixes it by telling the column parsing function whether it is for > index row or not, so that when parsing index row, the arcs are not > updated. A new test case is added to cover this scenario. > > Signed-off-by: Han Zhou Thank you for the bug fix. I agree that this fix should go in. I see at least one place where "update" is misspelled "udpate". Did you consider whether there is a way to distinguish an index row from other rows, without adding a new member? It might be possible to simply consider the UUID, since an index row has UUID zero and other rows do not. If this is possible, then I think I'd prefer that approach. The following incremental fixes some style violations identified by "checkpatch". --8<--cut here-->8-- diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index 3636a0e09e0e..451172cdcc34 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -2683,8 +2683,8 @@ do_idl_compound_index_with_ref(struct ovs_cmdl_context *ctx) struct ovsdb_idl_index *index; index = ovsdb_idl_create_index(idl, &idltest_table_simple3, "uref"); -ovsdb_idl_index_add_column(index, &idltest_simple3_col_uref, OVSDB_INDEX_ASC, - NULL); +ovsdb_idl_index_add_column(index, &idltest_simple3_col_uref, + OVSDB_INDEX_ASC, NULL); ovsdb_idl_get_initial_snapshot(idl); @@ -2706,7 +2706,7 @@ do_idl_compound_index_with_ref(struct ovs_cmdl_context *ctx) printf("%03d: After add to other table + set of strong ref\n", step++); dump_simple3(idl, myRow, step++); -myRow2 = (struct idltest_simple4*)idltest_simple4_first(idl); +myRow2 = (struct idltest_simple4 *) idltest_simple4_first(idl); printf("%03d: check simple4: %s\n", step++, myRow2 ? "not empty" : "empty"); @@ -2714,7 +2714,7 @@ do_idl_compound_index_with_ref(struct ovs_cmdl_context *ctx) struct idltest_simple3 *equal; equal = idltest_simple3_index_init_row(idl, &idltest_table_simple3); -myRow2 = (struct idltest_simple4*)idltest_simple4_first(idl); +myRow2 = (struct idltest_simple4 *) idltest_simple4_first(idl); idltest_simple3_index_set_uref(equal, &myRow2, 1); printf("%03d: Query using index with reference\n", step++); IDLTEST_SIMPLE3_FOR_EACH_EQUAL (myRow, &cursor, equal) { @@ -2732,7 +2732,7 @@ do_idl_compound_index_with_ref(struct ovs_cmdl_context *ctx) printf("%03d: After delete\n", step++); dump_simple3(idl, myRow, step++); -myRow2 = (struct idltest_simple4*)idltest_simple4_first(idl); +myRow2 = (struct idltest_simple4 *) idltest_simple4_first(idl); printf("%03d: check simple4: %s\n", step++, myRow2 ? "not empty" : "empty"); ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2] Introduce Emeritus Committer status.
From: Russell Bryant This patch introduces an Emeritus status for OVS committers. An Emeritus Committer is recognized as having made a significant impact to the project and having been a committer in the past. It is intended as an option for those that do not currently have the time or interest to fulfill committer responsibilities based on their current responsibilities. While in this status, they are not included in voting for governance purposes. An emeritus committer may be re-instated as a full committer at any time. The OVS committers voted approval of this change. See documentation contents for full details. Suggested-by: Ethan J. Jackson Signed-off-by: Russell Bryant Signed-off-by: Ben Pfaff --- v1->v2: - Deleted the previous requirements for inactivity revocation, which are now supplanted by emeritus status. - Wordsmithing. - Obtained committer approval via voting. Documentation/automake.mk | 1 + Documentation/index.rst| 3 +- .../internals/committer-emeritus-status.rst| 63 ++ .../internals/committer-grant-revocation.rst | 63 ++ Documentation/internals/index.rst | 1 + MAINTAINERS.rst| 14 - 6 files changed, 84 insertions(+), 61 deletions(-) create mode 100644 Documentation/internals/committer-emeritus-status.rst diff --git a/Documentation/automake.mk b/Documentation/automake.mk index 630fdf197b7c..3be185414928 100644 --- a/Documentation/automake.mk +++ b/Documentation/automake.mk @@ -81,6 +81,7 @@ DOC_SOURCE = \ Documentation/internals/index.rst \ Documentation/internals/authors.rst \ Documentation/internals/bugs.rst \ + Documentation/internals/committer-emeritus-status.rst \ Documentation/internals/committer-grant-revocation.rst \ Documentation/internals/committer-responsibilities.rst \ Documentation/internals/documentation.rst \ diff --git a/Documentation/index.rst b/Documentation/index.rst index 17b7b7a0e8a0..c737a6f6c238 100644 --- a/Documentation/index.rst +++ b/Documentation/index.rst @@ -108,7 +108,8 @@ Learn more about the Open vSwitch project and about how you can contribute: - **Maintaining:** :doc:`internals/maintainers` | :doc:`internals/committer-responsibilities` | - :doc:`internals/committer-grant-revocation` + :doc:`internals/committer-grant-revocation` | + :doc:`internals/committer-emeritus-status` - **Documentation:** :doc:`internals/contributing/documentation-style` | :doc:`Building Open vSwitch Documentation ` | diff --git a/Documentation/internals/committer-emeritus-status.rst b/Documentation/internals/committer-emeritus-status.rst new file mode 100644 index ..b2589ac6729e --- /dev/null +++ b/Documentation/internals/committer-emeritus-status.rst @@ -0,0 +1,63 @@ +.. + Licensed under the Apache License, Version 2.0 (the "License"); you may + not use this file except in compliance with the License. You may obtain + a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + License for the specific language governing permissions and limitations + under the License. + + Convention for heading levels in Open vSwitch documentation: + + === Heading 0 (reserved for the title in a document) + --- Heading 1 + ~~~ Heading 2 + +++ Heading 3 + ''' Heading 4 + + Avoid deeper levels because they do not render well. + +== +Emeritus Status for OVS Committers +== + +OVS committers are nominated and elected based on their impact on the Open +vSwitch project. Over time, as committers' responsibilities change, some may +become unable or uninterested in actively participating in project governance. +Committer "emeritus" status provides a way for committers to take a leave of +absence from OVS governance responsibilities. The following guidelines clarify +the process around the emeritus status for committers: + +* A committer may choose to transition from active to emeritus, or from + emeritus to active, by sending an email to the committers mailing list. + +* If a committer hasn't been heard from in 6 months, and does not respond to + reasonable attempts to contact him or her, the other committers can vote as a + majority to transition the committer from active to emeritus. (If the + committer resurfaces, he or she can transition back to active by sending an + email to the committers mailing list.) + +* Emeritus committers may stay on the committers mailing list to continue to + follow any discussions there. + +*
Re: [ovs-dev] [PATCH v2] Introduce Emeritus Committer status.
Acked-by: Ethan J. Jackson On Thu, Oct 26, 2017 at 2:33 PM, Ben Pfaff wrote: > From: Russell Bryant > > This patch introduces an Emeritus status for OVS committers. An > Emeritus Committer is recognized as having made a significant impact > to the project and having been a committer in the past. It is > intended as an option for those that do not currently have the time or > interest to fulfill committer responsibilities based on their current > responsibilities. While in this status, they are not included in > voting for governance purposes. > > An emeritus committer may be re-instated as a full committer at any > time. > > The OVS committers voted approval of this change. > > See documentation contents for full details. > > Suggested-by: Ethan J. Jackson > Signed-off-by: Russell Bryant > Signed-off-by: Ben Pfaff > --- > v1->v2: > - Deleted the previous requirements for inactivity revocation, > which are now supplanted by emeritus status. > - Wordsmithing. > - Obtained committer approval via voting. > > Documentation/automake.mk | 1 + > Documentation/index.rst| 3 +- > .../internals/committer-emeritus-status.rst| 63 > ++ > .../internals/committer-grant-revocation.rst | 63 > ++ > Documentation/internals/index.rst | 1 + > MAINTAINERS.rst| 14 - > 6 files changed, 84 insertions(+), 61 deletions(-) > create mode 100644 Documentation/internals/committer-emeritus-status.rst > > diff --git a/Documentation/automake.mk b/Documentation/automake.mk > index 630fdf197b7c..3be185414928 100644 > --- a/Documentation/automake.mk > +++ b/Documentation/automake.mk > @@ -81,6 +81,7 @@ DOC_SOURCE = \ > Documentation/internals/index.rst \ > Documentation/internals/authors.rst \ > Documentation/internals/bugs.rst \ > + Documentation/internals/committer-emeritus-status.rst \ > Documentation/internals/committer-grant-revocation.rst \ > Documentation/internals/committer-responsibilities.rst \ > Documentation/internals/documentation.rst \ > diff --git a/Documentation/index.rst b/Documentation/index.rst > index 17b7b7a0e8a0..c737a6f6c238 100644 > --- a/Documentation/index.rst > +++ b/Documentation/index.rst > @@ -108,7 +108,8 @@ Learn more about the Open vSwitch project and about how > you can contribute: > > - **Maintaining:** :doc:`internals/maintainers` | >:doc:`internals/committer-responsibilities` | > - :doc:`internals/committer-grant-revocation` > + :doc:`internals/committer-grant-revocation` | > + :doc:`internals/committer-emeritus-status` > > - **Documentation:** :doc:`internals/contributing/documentation-style` | >:doc:`Building Open vSwitch Documentation ` | > diff --git a/Documentation/internals/committer-emeritus-status.rst > b/Documentation/internals/committer-emeritus-status.rst > new file mode 100644 > index ..b2589ac6729e > --- /dev/null > +++ b/Documentation/internals/committer-emeritus-status.rst > @@ -0,0 +1,63 @@ > +.. > + Licensed under the Apache License, Version 2.0 (the "License"); you may > + not use this file except in compliance with the License. You may obtain > + a copy of the License at > + > + http://www.apache.org/licenses/LICENSE-2.0 > + > + Unless required by applicable law or agreed to in writing, software > + distributed under the License is distributed on an "AS IS" BASIS, > WITHOUT > + WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See > the > + License for the specific language governing permissions and limitations > + under the License. > + > + Convention for heading levels in Open vSwitch documentation: > + > + === Heading 0 (reserved for the title in a document) > + --- Heading 1 > + ~~~ Heading 2 > + +++ Heading 3 > + ''' Heading 4 > + > + Avoid deeper levels because they do not render well. > + > +== > +Emeritus Status for OVS Committers > +== > + > +OVS committers are nominated and elected based on their impact on the Open > +vSwitch project. Over time, as committers' responsibilities change, some may > +become unable or uninterested in actively participating in project > governance. > +Committer "emeritus" status provides a way for committers to take a leave of > +absence from OVS governance responsibilities. The following guidelines > clarify > +the process around the emeritus status for committers: > + > +* A committer may choose to transition from active to emeritus, or from > + emeritus to active, by sending an email to the committers mailing list. > + > +* If a committer hasn't been heard from in 6 months, and does not respond to > + reasonable attempts to contact him or her, the other committers can vote > as a > +
Re: [ovs-dev] [PATCH] faq: Better document how to add vendor extensions.
On Mon, Oct 9, 2017 at 10:33 AM, Ben Pfaff wrote: > Signed-off-by: Ben Pfaff > --- > Documentation/faq/contributing.rst | 44 > +++--- > include/openvswitch/ofp-errors.h | 4 +++- > 2 files changed, 34 insertions(+), 14 deletions(-) > > diff --git a/Documentation/faq/contributing.rst > b/Documentation/faq/contributing.rst > index 6dfc8bc4d436..d59376cd615c 100644 > --- a/Documentation/faq/contributing.rst > +++ b/Documentation/faq/contributing.rst > @@ -33,22 +33,28 @@ Q: How do I implement a new OpenFlow message? > as needed. (If you configure with ``--enable-Werror``, as described in > :doc:`/intro/install/general`, then it is impossible to miss any > warnings.) > > -If you need to add an OpenFlow vendor extension message for a vendor that > -doesn't yet have any extension messages, then you will also need to edit > -``build-aux/extract-ofp-msgs``. > +To add an OpenFlow vendor extension message (aka experimenter message) > for > +a vendor that doesn't yet have any extension messages, you will also need > +to edit ``build-aux/extract-ofp-msgs`` and at least ``ofphdrs_decode()`` > +and ``ofpraw_put__()`` in ``lib/ofp-msgs.c``. OpenFlow doesn't > standardize > +vendor extensions very well, so it's hard to make the process simpler > than > +that. (If you have a choice of how to design your vendor extension > +messages, it will be easier if you make them resemble the ONF and OVS > +extension messages.) > > Q: How do I add support for a new field or header? > > A: Add new members for your field to ``struct flow`` in ``lib/flow.h``, > and > add new enumerations for your new field to ``enum mf_field_id`` in > -``lib/meta-flow.h``, following the existing pattern. Also, add support > to Instead of ``lib/meta-flow.h``, maybe ``include/openvswitch/meta-flow.h``? Otherwise, looks good to me. Acked-by: Yi-Hung Wei ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] Introduce Emeritus Committer status.
Acked-by: Justin Pettit --Justin > On Oct 26, 2017, at 2:33 PM, Ben Pfaff wrote: > > From: Russell Bryant > > This patch introduces an Emeritus status for OVS committers. An > Emeritus Committer is recognized as having made a significant impact > to the project and having been a committer in the past. It is > intended as an option for those that do not currently have the time or > interest to fulfill committer responsibilities based on their current > responsibilities. While in this status, they are not included in > voting for governance purposes. > > An emeritus committer may be re-instated as a full committer at any > time. > > The OVS committers voted approval of this change. > > See documentation contents for full details. > > Suggested-by: Ethan J. Jackson > Signed-off-by: Russell Bryant > Signed-off-by: Ben Pfaff > --- > v1->v2: > - Deleted the previous requirements for inactivity revocation, >which are now supplanted by emeritus status. > - Wordsmithing. > - Obtained committer approval via voting. > > Documentation/automake.mk | 1 + > Documentation/index.rst| 3 +- > .../internals/committer-emeritus-status.rst| 63 ++ > .../internals/committer-grant-revocation.rst | 63 ++ > Documentation/internals/index.rst | 1 + > MAINTAINERS.rst| 14 - > 6 files changed, 84 insertions(+), 61 deletions(-) > create mode 100644 Documentation/internals/committer-emeritus-status.rst > > diff --git a/Documentation/automake.mk b/Documentation/automake.mk > index 630fdf197b7c..3be185414928 100644 > --- a/Documentation/automake.mk > +++ b/Documentation/automake.mk > @@ -81,6 +81,7 @@ DOC_SOURCE = \ > Documentation/internals/index.rst \ > Documentation/internals/authors.rst \ > Documentation/internals/bugs.rst \ > + Documentation/internals/committer-emeritus-status.rst \ > Documentation/internals/committer-grant-revocation.rst \ > Documentation/internals/committer-responsibilities.rst \ > Documentation/internals/documentation.rst \ > diff --git a/Documentation/index.rst b/Documentation/index.rst > index 17b7b7a0e8a0..c737a6f6c238 100644 > --- a/Documentation/index.rst > +++ b/Documentation/index.rst > @@ -108,7 +108,8 @@ Learn more about the Open vSwitch project and about how > you can contribute: > > - **Maintaining:** :doc:`internals/maintainers` | > :doc:`internals/committer-responsibilities` | > - :doc:`internals/committer-grant-revocation` > + :doc:`internals/committer-grant-revocation` | > + :doc:`internals/committer-emeritus-status` > > - **Documentation:** :doc:`internals/contributing/documentation-style` | > :doc:`Building Open vSwitch Documentation ` | > diff --git a/Documentation/internals/committer-emeritus-status.rst > b/Documentation/internals/committer-emeritus-status.rst > new file mode 100644 > index ..b2589ac6729e > --- /dev/null > +++ b/Documentation/internals/committer-emeritus-status.rst > @@ -0,0 +1,63 @@ > +.. > + Licensed under the Apache License, Version 2.0 (the "License"); you may > + not use this file except in compliance with the License. You may obtain > + a copy of the License at > + > + http://www.apache.org/licenses/LICENSE-2.0 > + > + Unless required by applicable law or agreed to in writing, software > + distributed under the License is distributed on an "AS IS" BASIS, > WITHOUT > + WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See > the > + License for the specific language governing permissions and limitations > + under the License. > + > + Convention for heading levels in Open vSwitch documentation: > + > + === Heading 0 (reserved for the title in a document) > + --- Heading 1 > + ~~~ Heading 2 > + +++ Heading 3 > + ''' Heading 4 > + > + Avoid deeper levels because they do not render well. > + > +== > +Emeritus Status for OVS Committers > +== > + > +OVS committers are nominated and elected based on their impact on the Open > +vSwitch project. Over time, as committers' responsibilities change, some may > +become unable or uninterested in actively participating in project > governance. > +Committer "emeritus" status provides a way for committers to take a leave of > +absence from OVS governance responsibilities. The following guidelines > clarify > +the process around the emeritus status for committers: > + > +* A committer may choose to transition from active to emeritus, or from > + emeritus to active, by sending an email to the committers mailing list. > + > +* If a committer hasn't been heard from in 6 months, and does not respond to > + reasonable attempts to contact him or her, the other committers can vote > as a > + majority to trans
Re: [ovs-dev] DNS support feature (was: Re: DNS support options)
Thanks Mark for your reply. There is one more thing. If we bring DNS into play, we may need a mechanism to watch for changes of ip addresses that were already resolved and being used. Thanks, Yifeng On Thu, Oct 26, 2017 at 12:10 PM, Mark Michelson wrote: > On Wed, Oct 25, 2017 at 4:16 PM Yifeng Sun wrote: > >> I feel that unbound stands out in the available open source DNS resolver. >> >> Below is the summary for unbound: >> * The actual resolving work is done by a background process or thread. A >> background process or thread seems unavoidable. Linux's getaddrinfo_a >> clones a thread similarly. >> > * It is ported on Linux, BSD, Windows, MacOS/X and Solaris/SPARC. This is >> good because OVS runs on a large range of platforms. >> > * It complies to the standard, with optional DNSSEC support. Some of its >> features may not be needed in our case. >> * The unbound context is thread-safe. Its internal locks may bring some >> overhead. But since the DNS resolving is not frequent in OVS, I suppose >> this small overhead is not an issue. >> >> Unbound looks like a good option. Another option is to create a >> background thread which processes DNS resolving requests from the main >> thread and sends back the resulting events to the main thread. This method >> is quite simple and straightforward. >> >> The above are what I got so far. Please give your thoughts, thanks a lot. >> > > If portability to all of the systems you mentioned in your second bullet > point is important, then you can rule out a couple of options: > * getaddrinfo_a is a GNU extension and is only available with glibc > * The resolver functions[1] are a BSD specification so they'd be available > on most platforms, but not on Windows. I don't personally recommend these > because of the need to manually parse the DNS responses you receive. > > That leaves two options: > * Run a background thread uses getaddrinfo() to perform resolution. > * Use a third-party library (like unbound). > > Of these two options, I feel like the third-party library is the better > option. The only downside I can think of is the extra dependency for OVS. > And as far as what third-party library to use, I was the one that suggested > unbound in the first place, so obviously I'm fine with using it :) > > Mark > > [1] http://man7.org/linux/man-pages/man3/resolver.3.html > > >> >> Below is the link for original discussion: >> https://mail.openvswitch.org/pipermail/ovs-dev/2017-August/337038.html >> >> >> >> On Wed, Oct 25, 2017 at 2:11 PM, Ben Pfaff wrote: >> >>> Hello everyone, please allow me to introduce Yifeng Sun (CCed), who >>> recently joined VMware's Open vSwitch team. I've asked Yifeng to start >>> out by working on DNS support for Open vSwitch. Yifeng, can you tell us >>> about what you've discovered so far, based on this thread from August >>> that I'm reviving, and your tentative conclusions? >>> >>> Thanks, >>> >>> Ben. >>> >> >> ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] PAYMENT UPDATE.
United States Money Gram Office, Address:300 7th St SW, Houston, Texas 20024, USA Houston, Texas, United States Phone: +1 916 581 2055 Dear Customer, Sir,How are you doing together with your family? I hope everything is moving normally as you want. Sir, it has being a while I heard from you. I email you severally regarding the transfer of your funds and none was replied till date. Can you kindly tell me what's going on? I am asking you this so that I will know your arrangement over this your transfer "whether you will want these funds to be completely transferred to you as it was programmed or to cancel it and return it back to Federal Government Treasury". I am waiting for your response and please do not hesitate to reply back as it is from your response that I will know whether you still interested on receiving those fund. Remember: Your payment file will be cancel and divert to government purse if you don't need these funds. So make sure you reply back immediately you receive this email and tell me if you still need those funds to be transfer to you... Use Below information to send the $220 for the securing of the required Form in order to proceed with your transfer as scheduled. RECEIVER NAME:: MICHAEL HILAND LOCATION:: ECKERT COLORADO USA AMOUN $220 Please send the fee through Money Gram office and get back to us with transfer slip in other to proceed on your transfer. Feel free to call Postal code for more clarifications. Your rapid response is highly needed. Here is the payment information we send on your name but due to what I been telling you concerning the paper, Our head Office Put it stop down till the paper is obtain and present to them. Remember, no one can pick it because it was press stop down by Our Head Office as I told you before. Sender's Name:.Corass smith Country:...Usa Reference #:.92169792 Amount:...$2,500 Note: Once you send the money, the clearance document of your funds will be obtained as well as its Government approval documents being secured too. Then we will without wasting time remit your first payment and email you the info to go to any Money Gram Office and pick it up peacefully. I await for your reply with the payment details of the $220 so that we will fix the error for you to pick up the first payment. Yours Sincere, Mr. William Wilson, Managing Director (Foreign Dept) Tell Phone: +1 916 581 2055 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Finanzas para no Financieros
Buenas Tardes: Este mes de Octubre le invitamos a adquirir una de nuestras pólizas de Capacitación online, los cuáles constan de 12 Temas que son utilizables durante 3 meses, las 24 hrs del día, las veces que usted así lo requiera. En Específico le ofrecemos el plan de Contabilidad y Finanzas que consta de 12 temas. 1. Principios Esenciales De La Administración De Empresas. 2. Guía Práctica De Finanzas Para No Financieros: Lenguaje 100% Empresarial. 3. Cómo Organizar Y Controlar Los Procedimientos De Contabilidad General. 4. Cómo Organizar Y Optimizar Costos/Presupuestos/Planeación De Utilidades (Puntos De Equilibrio). 5. Recomendaciones Críticas Para Organizar El Área Fiscal. 6. Herramientas Y Estrategias En Las Funciones Del Contralor. 7. Cómo Incrementar Tus Ventas, Administrando Tu Crédito Y Cuentas Por Cobrar. 8. Control Y Ahorro En Las Cuentas Por Pagar. 9. Administración Del Capital De Trabajo. 10. Auditoría Y Control Interno Para La Prevención De Fraudes. 11. Las Funciones Del Gerente Financiero. 12. Lo Que Todo Director General Debe Saber Sobre Finanzas Y La Administración De Su Compañía. Para que usted recibá la información completa de este plan, le solicitamos por favor enviarnos la palabra Contabilidad junto los siguientes datos: Nombre: Empresa: Teléfono: Y le haremos llegar el Temario Completo para que usted revise más a detalle. centro telefónico: 018002129393 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] recent OVS upload (was: Re: Bug#878249: Please package >= 2.7.0)
On Fri, Oct 20, 2017 at 08:56:24PM +0200, Thomas Goirand wrote: > I don't mind becoming a co-maintainer, though there's a few things that > need to change to facilitate packaging. First of all, I'd like to switch > to a Git packaging workflow, using git-buildpackage. The debian folder > upstream here should be removed: > > https://github.com/openvswitch/ovs/tree/master/debian > > otherwise, it will clash with what we have in Alioth. Then I'll push > openvswitch in the Alioth git repository under /git/third-party. > > Also, I will remove the debian/automake.mk and the d/copyright hack. > Indeed, Debian needs list of copyright holders, not authors. > > Is that fine for you? I'm starting... Thanks for doing an upload. Open vSwitch users expect to be able to build packages for whatever version of Open vSwitch they're using, which is most easily satisfied by keeping the Debian packaging with the rest of the tree. Any other solution makes it harder to keep the code in sync with the packaging. Removing the debian directory would frustrate this. I'm not a fan of the "dfsg" suffix because it implies that DFSG-noncompliant material was removed. There is nothing DFSG-noncompliant about the Debian directory, it is simply inconvenient for the way you prefer to maintain the packaging. Would you mind choosing a different suffix? I see a number of failed builds here: https://buildd.debian.org/status/package.php?p=openvswitch&suite=experimental Let me analyze them: * mips, powerpc, and ppc64 should be fixed by this commit that is already on branch-2.8: https://github.com/openvswitch/ovs/commit/2906ff5e7eb1fb39b497dc05e471 * m68k is because of looser alignment rules than on other platforms. I don't care much about m68k, and it's not a Debian required platform, so I don't plan to fix this. * sparc64 failures are due to bus errors. I would like to investigate, but I don't know how, because there is only one sparc64 machine listed at https://db.debian.org/machines.cgi, and that machine appears to be down (it is not accepting SSH connections at least when I tried just now). * The ppc64el failure is a hang during the testsuite. Test 2332, which appears to be "ovn -- icmp_reply: 1 HVs, 2 LSs, 1 lport/LS, 1 LR", hung. I will try to reproduce and fix this. Even if we do not fix it, it might not recur in later runs, because it indicates a race condition in the testsuite. (This is almost certainly a bug in the testsuite rather than in OVS itself.) It has been my practice to package the tip of whatever release branch we're using, for example in this case to package from the tip of branch-2.8. OVS release branches only accept bug fixes, so this works well for getting bug fixes that have not yet made it into an "official" release. In this case, this would have picked up the fix that caused three of the builds to fail while running the testsuite. Thanks again, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PKG-Openstack-devel] Bug#878249: recent OVS upload
Hi Ben, On 10/27/2017 12:45 AM, Ben Pfaff wrote: > Thanks for doing an upload. > > Open vSwitch users expect to be able to build packages for whatever > version of Open vSwitch they're using, which is most easily satisfied by > keeping the Debian packaging with the rest of the tree. Any other > solution makes it harder to keep the code in sync with the packaging. > Removing the debian directory would frustrate this. You can simply rename it as "debian-upstream" or something. The other way is to push the debian folder in a separate packaging branch. I by the way saw a lot of differences between the Debian packaging and the upstream one, which is a major issue, and the main reason why having a debian folder upstream is a bad idea: there's 2 source of "truth", and that should be avoided. > I'm not a fan of the "dfsg" suffix because it implies that > DFSG-noncompliant material was removed. There is nothing > DFSG-noncompliant about the Debian directory, it is simply inconvenient > for the way you prefer to maintain the packaging. Which is why I wrote a debian/README.source explaining the facts. Is that enough for you? Alternatively, I could use "debian1", but I don't think a lot of people will even notice, and even less, understand the difference. > I see a number of failed builds here: > https://buildd.debian.org/status/package.php?p=openvswitch&suite=experimental > > Let me analyze them: > > * mips, powerpc, and ppc64 should be fixed by this commit that is > already on branch-2.8: > https://github.com/openvswitch/ovs/commit/2906ff5e7eb1fb39b497dc05e471 I can incorporate this patch, no pb. > * m68k is because of looser alignment rules than on other platforms. I > don't care much about m68k, and it's not a Debian required platform, > so I don't plan to fix this. Right, we shall not care. > * sparc64 failures are due to bus errors. I would like to investigate, > but I don't know how, because there is only one sparc64 machine listed > at https://db.debian.org/machines.cgi, and that machine appears to be > down (it is not accepting SSH connections at least when I tried just > now). Sparc64 isn't an official port either. See here: https://www.debian.org/ports/ I don't think we should care. > * The ppc64el failure is a hang during the testsuite. Test 2332, which > appears to be "ovn -- icmp_reply: 1 HVs, 2 LSs, 1 lport/LS, 1 LR", > hung. I will try to reproduce and fix this. Even if we do not fix > it, it might not recur in later runs, because it indicates a race > condition in the testsuite. (This is almost certainly a bug in the > testsuite rather than in OVS itself.) In such a case, I'd strongly suggest removing this unit test until further investigation. Is that ok for you too? > It has been my practice to package the tip of whatever release branch > we're using, for example in this case to package from the tip of > branch-2.8. OVS release branches only accept bug fixes, so this works > well for getting bug fixes that have not yet made it into an "official" > release. In this case, this would have picked up the fix that caused > three of the builds to fail while running the testsuite. Well, in such a case, I'd suggest upstream to release more bugfix releases then! :) > Thanks again, My pleasure. I hope the other changes I made are ok with you. Did you read the debian/changelog? I removed lots of binary packages that were not technically needed, because that's best practice in Debian (ie: the FTP masters recommend this). All of them have a Provides: equivalent. This also simplifies packaging a lot (ie: all /usr/bin things and manpages are going into openvswitch-common, etc.). BTW, I always wanted to know: is there a way to do VXLan with OpenStack and the OpenVSwitch plugin? Cheers, Thomas Goirand (zigo) ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] dpif-netdev: Initialize new rxqs in port_reconfigure().
valgrind reported use of uninitialized data in port_reconfigure(), which was due to xrealloc() not initializing the newly added data, combined with dp_netdev_rxq_set_intrvl_cycles() reading 'intrvl_idx' from the added data. This avoids the warning. Signed-off-by: Ben Pfaff --- lib/dpif-netdev.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index d5eb8305c8a2..1418175bdfef 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -3285,9 +3285,14 @@ port_reconfigure(struct dp_netdev_port *port) port->txq_used = xcalloc(netdev_n_txq(netdev), sizeof *port->txq_used); for (i = 0; i < netdev_n_rxq(netdev); i++) { +bool new_queue = i >= last_nrxq; +if (new_queue) { +memset(&port->rxqs[i], 0, sizeof port->rxqs[i]); +} + port->rxqs[i].port = port; -if (i >= last_nrxq) { -/* Only reset cycle stats for new queues */ + +if (new_queue) { dp_netdev_rxq_set_cycles(&port->rxqs[i], RXQ_CYCLES_PROC_CURR, 0); dp_netdev_rxq_set_cycles(&port->rxqs[i], RXQ_CYCLES_PROC_HIST, 0); for (unsigned j = 0; j < PMD_RXQ_INTERVAL_MAX; j++) { -- 2.10.2 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v8 4/6] netdev-dpdk: manage failure in mempool name creation.
On 10/19/17, 9:56 AM, "ovs-dev-boun...@openvswitch.org on behalf of antonio.fische...@intel.com" wrote: In case a mempool name could not be generated log a message and return a null mempool pointer to the caller. CC: Mark B Kavanagh CC: Darrell Ball CC: Ciara Loftus CC: Kevin Traynor CC: Aaron Conole Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each port.") Signed-off-by: Antonio Fischetti --- lib/netdev-dpdk.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index dc1e9c3..6fc6e1b 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -502,6 +502,9 @@ dpdk_mp_name(struct dpdk_mp *dmp) int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u", h, dmp->socket_id, dmp->mtu, dmp->mp_size); if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) { I see you copied me on an earlier version, so I’ll add one comment. Given MTU size restriction for dpdk and rte max queues and max buf desc enforcement, can this condition be met ? I think there are 11 remaining characters (out of 31) for mp_size and the calculation is dmp->mp_size = dev->requested_n_rxq * dev->requested_rxq_size + dev->requested_n_txq * dev->requested_txq_size + MIN(RTE_MAX_LCORE, dev->requested_n_rxq) * NETDEV_MAX_BURST + MIN_NB_MBUF; I got 9 decimal digits max, assuming my math is correct… Previous filtering for mtu (netdev_dpdk_set_mtu) and max queues (dpdk_eth_dev_init)/max buf desc (dpdk_process_queue_size) enforcement (present and also any future design) should prevent snprintf from having an unexpected return value number of chars, which would mean snprintf should only have an unexpected return value number of chars, if a future coding error were to be introduced. FYI, OVS traditionally deals with other such cases with ovs_assert(), not a VLOG_DBG, since it is easier to find the bug earlier. I believe snprintf at this level should not be catching user config errors. +VLOG_DBG("snprintf returned %d. Failed to generate a mempool " +"name for \"%s\". Hash:0x%x, mtu:%d, mbufs:%u.", +ret, dmp->if_name, h, dmp->mtu, dmp->mp_size); return NULL; } return mp_name; @@ -533,6 +536,10 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool *mp_exists) do { char *mp_name = dpdk_mp_name(dmp); +if (!mp_name) { +rte_free(dmp); +return NULL; +} VLOG_DBG("Port %s: Requesting a mempool of %u mbufs " "on socket %d for %d Rx and %d Tx queues.", -- 2.4.11 ___ dev mailing list d...@openvswitch.org https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=cKHgut93ZGYQzoLXpXQWPqcZhBX3NMdQaRnbNlfvhiU&s=tx4JYNmCFZ-vgGQzTIteThFJN2RSlMtqg34oTwlFEF0&e= ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] race condition in "ovn: l3ha ensure no master bouncing when ovn-controller is restarted" test?
Hi, I've been running the OVN unit tests in a loop with -j10 (to run faster and cause load) today, and one failure that I see is the following: 2364. ovn.at:8454: testing ovn -- ensure one gw controller restart in HA doesn't bounce the master ... creating ovn-sb database creating ovn-nb database starting ovn-northd starting backup ovn-northd adding simulator 'main' adding simulator 'gw1' adding simulator 'gw2' adding simulator 'hv1' OK OK OK OK OK OK 1b7f7753-aea0-4484-a8db-2971b9fb444f 552e4549-6ba5-4509-99a2-7968cb960a15 a7ea2de8-558d-413a-90df-187f58c4f239 ../../tests/ovn.at:8533: grep "Releasing lport" gw1/ovn-controller.log --- /dev/null 2017-07-26 15:46:07.674034656 -0700 +++ /home/blp/nicira/ovs/_build/tests/testsuite.dir/at-groups/2364/stdout 2017-10-26 17:11:07.261985117 -0700 @@ -0,0 +1 @@ +2017-10-27T00:11:07.242Z|00024|binding|INFO|Releasing lport cr-outside from this chassis. ../../tests/ovn.at:8533: exit code was 0, expected 1 2364. ovn.at:8454: 2364. ovn -- ensure one gw controller restart in HA doesn't bounce the master (ovn.at:8454): FAILED (ovn.at:8533) I don't really understand this test, which was introduced by the following commit. Do you guys have any hints as to why this would race under high load? --8<--cut here-->8-- From: "majop...@redhat.com" Date: Thu, 13 Jul 2017 17:02:41 + Subject: [PATCH] ovn: l3ha ensure no master bouncing when ovn-controller is restarted When ovn-controller is restarted, ovn-controller removes the old Chassis entry from the SBDB and a new one is inserted. This cleared the Gateway_Chassis chassis column in the SBDB and then ovn-northd removed the empty-column Gateway_Chassis entry. Such event made the other (non-restarted and master gateway chassis) believe that he was a single (non-HA) gateway, turning off BFD and releasing the port for a tiny time frame causing unnecesary downtime. Signed-off-by: Miguel Angel Ajo Signed-off-by: Russell Bryant --- ovn/northd/ovn-northd.c | 34 +++- tests/ovn.at| 83 + 2 files changed, 103 insertions(+), 14 deletions(-) diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index cfe765f42c9d..a3f138d448ce 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -1684,11 +1684,22 @@ gateway_chassis_equal(const struct nbrec_gateway_chassis *nb_gwc, const struct sbrec_chassis *nb_gwc_c, const struct sbrec_gateway_chassis *sb_gwc) { -return !strcmp(nb_gwc->name, sb_gwc->name) - && !strcmp(nb_gwc_c->name, sb_gwc->chassis->name) - && nb_gwc->priority == sb_gwc->priority - && smap_equal(&nb_gwc->options, &sb_gwc->options) - && smap_equal(&nb_gwc->external_ids, &sb_gwc->external_ids); +bool equal = !strcmp(nb_gwc->name, sb_gwc->name) + && nb_gwc->priority == sb_gwc->priority + && smap_equal(&nb_gwc->options, &sb_gwc->options) + && smap_equal(&nb_gwc->external_ids, &sb_gwc->external_ids); + +if (!equal) { +return false; +} + +/* If everything else matched and we were unable to find the SBDB + * Chassis entry at this time, assume a match and return true. + * This happens when an ovn-controller is restarting and the Chassis + * entry is gone away momentarily */ +return !nb_gwc_c + || (sb_gwc->chassis && !strcmp(nb_gwc_c->name, + sb_gwc->chassis->name)); } static bool @@ -1723,11 +1734,10 @@ sbpb_gw_chassis_needs_update( chassis_lookup_by_name(chassis_index, lrp->gateway_chassis[n]->chassis_name); -if (chassis) { -lrp_gwc_c[lrp_n_gateway_chassis] = chassis; -lrp_gwc[lrp_n_gateway_chassis] = lrp->gateway_chassis[n]; -lrp_n_gateway_chassis++; -} else { +lrp_gwc_c[lrp_n_gateway_chassis] = chassis; +lrp_gwc[lrp_n_gateway_chassis] = lrp->gateway_chassis[n]; +lrp_n_gateway_chassis++; +if (!chassis) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); VLOG_WARN_RL( &rl, "Chassis name %s referenced in NBDB via Gateway_Chassis " @@ -1805,10 +1815,6 @@ copy_gw_chassis_from_nbrp_to_sbpb( const struct sbrec_chassis *chassis = chassis_lookup_by_name(chassis_index, lrp_gwc->chassis_name); -if (!chassis) { -continue; -} - gw_chassis = xrealloc(gw_chassis, (n_gwc + 1) * sizeof *gw_chassis); struct sbrec_gateway_chassis *pb_gwc = diff --git a/tests/ovn.at b/tests/ovn.at index 126d4a67141b..03a9ff465143 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -8080,3 +8080,86 @@ AT_CHECK([grep $garp hv2_b
Re: [ovs-dev] recent OVS upload (was: Re: Bug#878249: Please package >= 2.7.0)
On Thu, Oct 26, 2017 at 03:45:48PM -0700, Ben Pfaff wrote: > I see a number of failed builds here: > https://buildd.debian.org/status/package.php?p=openvswitch&suite=experimental > > Let me analyze them: > > * mips, powerpc, and ppc64 should be fixed by this commit that is > already on branch-2.8: > https://github.com/openvswitch/ovs/commit/2906ff5e7eb1fb39b497dc05e471 > > * m68k is because of looser alignment rules than on other platforms. I > don't care much about m68k, and it's not a Debian required platform, > so I don't plan to fix this. > > * sparc64 failures are due to bus errors. I would like to investigate, > but I don't know how, because there is only one sparc64 machine listed > at https://db.debian.org/machines.cgi, and that machine appears to be > down (it is not accepting SSH connections at least when I tried just > now). > > * The ppc64el failure is a hang during the testsuite. Test 2332, which > appears to be "ovn -- icmp_reply: 1 HVs, 2 LSs, 1 lport/LS, 1 LR", > hung. I will try to reproduce and fix this. Even if we do not fix > it, it might not recur in later runs, because it indicates a race > condition in the testsuite. (This is almost certainly a bug in the > testsuite rather than in OVS itself.) There's now a failure on hppa, too, which bears investigation. I'll try to look at that soon. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev