Re: [ovs-dev] [PATCH v2] openvswitch: Trim off padding before L3+ netfilter processing
On Fri, Dec 22, 2017 at 4:39 PM, Ed Swierk wrote: > On Fri, Dec 22, 2017 at 3:31 PM, Pravin Shelar wrote: >> On Thu, Dec 21, 2017 at 7:17 AM, Ed Swierk >> wrote: >>> IPv4 and IPv6 packets may arrive with lower-layer padding that is not >>> included in the L3 length. For example, a short IPv4 packet may have >>> up to 6 bytes of padding following the IP payload when received on an >>> Ethernet device. In the normal IPv4 receive path, ip_rcv() trims the >>> packet to ip_hdr->tot_len before invoking netfilter hooks (including >>> conntrack and nat). >>> >>> In the IPv6 receive path, ip6_rcv() does the same using >>> ipv6_hdr->payload_len. Similarly in the br_netfilter receive path, >>> br_validate_ipv4() and br_validate_ipv6() trim the packet to the L3 >>> length before invoking NF_INET_PRE_ROUTING hooks. >>> >>> In the OVS conntrack receive path, ovs_ct_execute() pulls the skb to >>> the L3 header but does not trim it to the L3 length before calling >>> nf_conntrack_in(NF_INET_PRE_ROUTING). When nf_conntrack_proto_tcp >>> encounters a packet with lower-layer padding, nf_checksum() fails and >>> logs "nf_ct_tcp: bad TCP checksum". While extra zero bytes don't >>> affect the checksum, the length in the IP pseudoheader does. That >>> length is based on skb->len, and without trimming, it doesn't match >>> the length the sender used when computing the checksum. >>> >>> The assumption throughout nf_conntrack and nf_nat is that skb->len >>> reflects the length of the L3 header and payload, so there is no need >>> to refer back to ip_hdr->tot_len or ipv6_hdr->payload_len. >>> >>> This change brings OVS into line with other netfilter users, trimming >>> IPv4 and IPv6 packets prior to L3+ netfilter processing. >>> >>> Signed-off-by: Ed Swierk >>> --- >>> v2: >>> - Trim packet in nat receive path as well as conntrack >>> - Free skb on error >>> --- >>> net/openvswitch/conntrack.c | 34 ++ >>> 1 file changed, 34 insertions(+) >>> >>> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c >>> index b27c5c6..1bdc78f 100644 >>> --- a/net/openvswitch/conntrack.c >>> +++ b/net/openvswitch/conntrack.c >>> @@ -703,6 +703,33 @@ static bool skb_nfct_cached(struct net *net, >>> return ct_executed; >>> } >>> >>> +/* Trim the skb to the L3 length. Assumes the skb is already pulled to >>> + * the L3 header. The skb is freed on error. >>> + */ >>> +static int skb_trim_l3(struct sk_buff *skb) >>> +{ >>> + unsigned int nh_len; >>> + int err; >>> + >>> + switch (skb->protocol) { >>> + case htons(ETH_P_IP): >>> + nh_len = ntohs(ip_hdr(skb)->tot_len); >>> + break; >>> + case htons(ETH_P_IPV6): >>> + nh_len = ntohs(ipv6_hdr(skb)->payload_len) >>> + + sizeof(struct ipv6hdr); >>> + break; >>> + default: >>> + nh_len = skb->len; >>> + } >>> + >>> + err = pskb_trim_rcsum(skb, nh_len); >>> + if (err) >> This should is unlikely. > > I'll add unlikely(). > >>> + kfree_skb(skb); >>> + >>> + return err; >>> +} >>> + >> This looks like a generic function, it probably does not belong to OVS >> code base. > > Indeed. I'll move it to skbuff.c, unless you have a better idea. > >>> #ifdef CONFIG_NF_NAT_NEEDED >>> /* Modelled after nf_nat_ipv[46]_fn(). >>> * range is only used for new, uninitialized NAT state. >>> @@ -715,8 +742,12 @@ static int ovs_ct_nat_execute(struct sk_buff *skb, >>> struct nf_conn *ct, >>> { >>> int hooknum, nh_off, err = NF_ACCEPT; >>> >>> + /* The nat module expects to be working at L3. */ >>> nh_off = skb_network_offset(skb); >>> skb_pull_rcsum(skb, nh_off); >>> + err = skb_trim_l3(skb); >>> + if (err) >>> + return err; >>> >> ct-nat is executed within ct action, so I do not see why you you call >> skb-trim again from ovs_ct_nat_execute(). >> ovs_ct_execute() trim should take care of the skb. > > I see. Doesn't that mean that skb_pull_rcsum() is also unnecessary in > ovs_ct_nat_execute(), as ovs_ct_execute() has already pulled the skb > to the L3 header? > Yes, It looks redundant. but lets address it in separate patch. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] Pravin Shelar
On Tue, Jan 2, 2018 at 11:36 AM, David Miller wrote: > From: Pravin Shelar > Date: Thu, 28 Dec 2017 15:47:39 -0800 > >> Thanks Joe for the patch. But it is corrupted. I will send updated patch >> soon. > > I'm still waiting for this, just FYI :) I sent the patch. Thanks. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netdev: Refactor datapath flow cache
Hi Yipeng, Thanks for the reply. On Tue, Jan 2, 2018 at 10:45 AM, Wang, Yipeng1 wrote: > Hi, William, > > Thanks for the comment. You are right. If the RSS hash does not consider the > fields that matter, the situation you mentioned could happen. > > There are two design options for CD as you may find, when the signatures > collide, we could either replace the existing entry (the current design), or > still insert the entry into the cache. If we chose the second design, I think > the situation you mentioned could be alleviated. We chose the first one > mostly because of its simplicity and speed for the hit case. For example, if > we allow multiple entries with the same signature stay in one bucket, then > the lookup function needs to iterate all the entries in a bucket to find all > the matches (for scalar version). And additional loops and variables are > required to iterate all the matches. We expected to see some percentage of > throughput influence for cache hit cases. I think the cost of having multiple entries with the same signature is too high, basically the CD lookup time increase from O(1) to O(n), where n is the bucket size. > > But as you suggested, if the situation you mentioned is very common in real > use cases, and RSS does not consider the vlan id, we could choose to not > overwrite. Another option is to reduce the insertion rate (or turn off CD) as > CD's miss ratio is high (this is similar to the EMC conditional insertion). > Then the 100% miss ratio case can be alleviated. This is an easy change for > CD. Or we could use software hash together with RSS to consider vlan tag, > this could benefit EMC too I guess. > > There are many design options and trade-offs but we eventually want to have a > design that work for most use cases. I don't have any traffic dataset, but I would assume it's pretty common that multiple tunneling protocols are deployed. That said, the RSS hash, which is based on outer-header 5-tuple, might have little difference and cause high collision when flows try to match fields such as vxlan vni, or geneve metadata field. Matching the inner packets requires recirculation, so the rss of inner 5-tuple come from cpu, and I guess the CD's hit rate is higher for inner packets. The DFC (datapath flow cache) patch seems to have similar drawbacks? The fundamental issue seems to be the choice of hash function (RSS), which only covers 5-tuple. Can we configure the rss hash to hash on more fields when subtables uses more than 5-tuple? Regards, William ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] API for connecting ovs bridge to manage packets
sorry. I just realize the rconn.h had become a public APIs a month ago :) Thanks Zhenyu Gao 2018-01-03 9:21 GMT+08:00 Gao Zhenyu : > Thanks for the suggestion. I will try to make them become regular APIs and > submit patch later. > > Thanks > Zhenyu Gao > > 2018-01-03 0:37 GMT+08:00 Ben Pfaff : > >> On Fri, Dec 29, 2017 at 11:39:16AM +0800, Gao Zhenyu wrote: >> > As you know ovn consumes rconn functions in pinctrl.c to connect >> the >> > bridge. But the rconn functions are private functions. I mean those >> > functions are not exposed in include/openvswitch/*.h file, so they are >> not >> > regular APIs, right? >> > So do you know if we get a way to connect the bridge but not using >> > rconn functions? >> > Then I can make a controller to manage the IN/OUT packets without >> > compiling whole ovs. >> >> I don't see any real obstacle to making rconn a public API. Would that >> be the best solution for you? >> > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] API for connecting ovs bridge to manage packets
Thanks for the suggestion. I will try to make them become regular APIs and submit patch later. Thanks Zhenyu Gao 2018-01-03 0:37 GMT+08:00 Ben Pfaff : > On Fri, Dec 29, 2017 at 11:39:16AM +0800, Gao Zhenyu wrote: > > As you know ovn consumes rconn functions in pinctrl.c to connect > the > > bridge. But the rconn functions are private functions. I mean those > > functions are not exposed in include/openvswitch/*.h file, so they are > not > > regular APIs, right? > > So do you know if we get a way to connect the bridge but not using > > rconn functions? > > Then I can make a controller to manage the IN/OUT packets without > > compiling whole ovs. > > I don't see any real obstacle to making rconn a public API. Would that > be the best solution for you? > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [no-slow 4/6] ofproto: Add 'ofproto_uuid' and 'ofp_in_port' to user action cookie.
> On Jan 2, 2018, at 10:13 AM, Ben Pfaff wrote: > > On Thu, Dec 21, 2017 at 02:25:13PM -0800, Justin Pettit wrote: >> Previously, the ofproto instance and OpenFlow port have been derived >> based on the datapath port number. This change explicitly declares them >> both, which will be helpful in future commits that no longer can depend >> on having a unique datapath port (e.g., a source port that represents >> the controller). >> >> Signed-off-by: Justin Pettit > > Some checkpatch warnings look legit: > > WARNING: Line length is >79-characters long > #34 FILE: lib/odp-util.c:457: > && cookie.header.type == USER_ACTION_COOKIE_FLOW_SAMPLE) > { > > WARNING: Line length is >79-characters long > #259 FILE: ofproto/ofproto-dpif-upcall.c:1101: > = > ofproto_dpif_lookup_by_uuid(&upcall->cookie.header.ofproto_uuid); > > WARNING: Line length is >79-characters long > #307 FILE: ofproto/ofproto-dpif-upcall.c:2072: > ofp_in_port, odp_actions, smid, cmid, > &ofproto->uuid); > > There's a stray " in this line in odp-util.h: > +enum user_action_cookie_type type; /* One of USER_ACTION_COOKIE_*" */ I addressed those issues. > Maybe you like the way you did it, which is fine, but an alternative way > to arrange user_action_cookie would be to make it a struct with the > standard header followed by embedding further structs inside an > anonymous union, like below. Then you could omit lots of ".header." > stuff although you'd need to s/union/struct/ in other places. > > /* user_action_cookie is passed as argument to OVS_ACTION_ATTR_USERSPACE. */ > struct user_action_cookie { >enum user_action_cookie_type type; /* One of USER_ACTION_COOKIE_*" */ >ofp_port_t ofp_in_port;/* OpenFlow in port, or OFPP_NONE. */ >struct uuid ofproto_uuid; /* UUID of ofproto-dpif. */ > >union { >struct { >/* USER_ACTION_COOKIE_SFLOW. */ >ovs_be16 vlan_tci; /* Destination VLAN TCI. */ >uint32_t output;/* SFL_FLOW_SAMPLE_TYPE 'output' value. */ >} sflow; > >struct { >/* USER_ACTION_COOKIE_SLOW_PATH. */ >uint32_t reason;/* enum slow_path_reason. */ >} slow_path; > >struct { >/* USER_ACTION_COOKIE_FLOW_SAMPLE. */ >uint16_t probability; /* Sampling probability. */ >uint32_t collector_set_id; /* ID of IPFIX collector set. */ >uint32_t obs_domain_id; /* Observation Domain ID. */ >uint32_t obs_point_id; /* Observation Point ID. */ >odp_port_t output_odp_port; /* The output odp port. */ >enum nx_action_sample_direction direction; >} flow_sample; > >struct { >/* USER_ACTION_COOKIE_IPFIX. */ >odp_port_t output_odp_port; /* The output odp port. */ >} ipfix; >}; > }; > > It looks like the shortest user_action_cookie variation is 24 bytes, the > longest is 48. It may not be worth it to treat user_action_cookie as > variable-length now. I think that it was done this way to better > tolerate old datapaths that had a short, fixed maximum length for > userdata. It would be simpler to just always ship sizeof(union > user_action_cookie) bytes to the datapath and always require > sizeof(union user_action_cookie) back. You could then skip a lot of > userdata_len checks and updates for each type of cookie. These are good suggestions. I want to send it as a separate patch because it contains more subtlety than I'd want to do as an incremental. > Acked-by: Ben Pfaff Thanks! --Justin ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2] lib: Make error messages more useful
On 1/2/2018 11:52 AM, Ben Pfaff wrote: On Thu, Dec 28, 2017 at 01:52:41PM -0800, Greg Rose wrote: There are many "opening datapath" error messages and when one occurs it is impossible to know just from the log message which of the "opening datapath" errors occurred. Add a helper function that incorporates the calling function's name to help identify where the datapath open error occurred. Signed-off-by: Greg Rose Thanks for working on improving OVS error messages. Users don't want to see the details of OVS code, whether those details are line numbers or function names, except maybe when they're at wits' end anyway (e.g. assertion failures, debug-level log messages). To better distinguish error messages, we should provide something meaningful to the user. That could be the name of a dpctl or unixctl command, or some other indication of why the code was opening the datapath (e.g. "to add an interface"). Hmm. Yeah, that's why I was including the datapath name in the original patch. The helper function I was envisioning would have included the initial call to parsed_dpif_open(), since that's such a common step before reporting an error. OK, if you feel it is still worth pursuing then I'll use the approach you suggest. Thanks, - Greg Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 0/3] Initial support for new SIP Alg.
Tiago Lam writes: > This patch-set is an initial approach at implementing the new SIP Alg, > mentioned by Aaron at [1]. Thanks for this work, Tiago! > I'm mostly interested in getting to know your thoughts of how this is > headed. There are a couple of points that are worth bringing up: > - As mentioned in patches 1/3 and 2/3, this is still a preliminary > implementation, and some work will be needed to move away from some > assuptions, like assuming the SIP traffic is always going over IPv4 > and TCP; > - At the moment, the sip state is being stored in the conn struct. I > followed the example of seq_skew_dir here, which is also stored there, > but realise this is not ideal. It seems storing it somewhere agnostic > will be ideal in the future, to avoid polluting that struct with > different Alg's details; > - The SIP helpers functions and structures are in conntrack-sip.h and > conntrack-sip.c. This can create confusion when comparing to > conntrack-tcp.c and other protocols since SIP is an Alg and is at a > different level. > > With regards to testing, for now, this has been tested manually, by > setting up the flows mentioned in patch 2/3 and having two VMs connected > to OvS, both using SIPp to simulate real traffic both ways. I'm going to > have a look at how this can be automated and added to > tests/system-traffic.at, together with the rest of the already existing > tests. Please do. I would consider the test-suite an important part for acceptance. > [1] [CONNTRACK] Discussions at OvS 2017: > https://mail.openvswitch.org/pipermail/ovs-dev/2017-November/341089.html > > Tiago Lam (3): > Conntrack: Add new API for future SIP Alg. > Conntrack: Add initial support for new SIP Alg. > Conntrack: Support asymmetric RTP port for SIP. > > include/openvswitch/ofp-actions.h | 4 + > lib/automake.mk | 2 + > lib/conntrack-private.h | 2 + > lib/conntrack-sip.c | 491 > ++ > lib/conntrack-sip.h | 123 ++ > lib/conntrack.c | 254 +++- > lib/ofp-parse.c | 5 + > ofproto/ofproto-dpif-xlate.c | 3 + > 8 files changed, 883 insertions(+), 1 deletion(-) > create mode 100644 lib/conntrack-sip.c > create mode 100644 lib/conntrack-sip.h ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2] lib: Make error messages more useful
On Thu, Dec 28, 2017 at 01:52:41PM -0800, Greg Rose wrote: > There are many "opening datapath" error messages and when one occurs > it is impossible to know just from the log message which of the > "opening datapath" errors occurred. Add a helper function that > incorporates the calling function's name to help identify where > the datapath open error occurred. > > Signed-off-by: Greg Rose Thanks for working on improving OVS error messages. Users don't want to see the details of OVS code, whether those details are line numbers or function names, except maybe when they're at wits' end anyway (e.g. assertion failures, debug-level log messages). To better distinguish error messages, we should provide something meaningful to the user. That could be the name of a dpctl or unixctl command, or some other indication of why the code was opening the datapath (e.g. "to add an interface"). The helper function I was envisioning would have included the initial call to parsed_dpif_open(), since that's such a common step before reporting an error. Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [no-slow 2/6] ofproto-dpif: Reorganize upcall handling.
> On Dec 28, 2017, at 3:22 PM, Gregory Rose wrote: > > SFAICT it emulates exactly what the system-traffic.at test 001 does. And it > works fine... /shrug. > > What distribution, kernel, etc are you using for your check-kmod testing? > I'll try to emulate that > exactly and then see if I can get similar results. I'm using Ubuntu 16.04 with kernel 4.4.0-104-generic. I sent you a link on our Slack channel to the internal tester that runs different OSs. It fails a few of tests, but they're the same ones that fail on master. (We need to address those, but they shouldn't be related to my patches.) --Justin ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] Pravin Shelar
From: Pravin Shelar Date: Thu, 28 Dec 2017 15:47:39 -0800 > Thanks Joe for the patch. But it is corrupted. I will send updated patch soon. I'm still waiting for this, just FYI :) ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] lex: Fix parsing of long tokens.
When a token is longer than the built-in 256-byte buffer, a buffer is malloc()'d but it was not properly null-terminated. Found by afl-fuzz. Reported-by: Bhargava Shastry Signed-off-by: Ben Pfaff --- ovn/lib/lex.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ovn/lib/lex.c b/ovn/lib/lex.c index 6f2b570f5c65..2f49af0e91e2 100644 --- a/ovn/lib/lex.c +++ b/ovn/lib/lex.c @@ -89,7 +89,7 @@ lex_token_strcpy(struct lex_token *token, const char *s, size_t length) ? token->buffer : xmalloc(length + 1)); memcpy(token->s, s, length); -token->buffer[length] = '\0'; +token->s[length] = '\0'; } void -- 2.10.2 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [no-slow 2/6] ofproto-dpif: Reorganize upcall handling.
On 12/28/2017 3:22 PM, Gregory Rose wrote: On 12/21/2017 2:25 PM, Justin Pettit wrote: - This reduces the number of times upcall cookies are processed. - It separate true miss calls from slow-path actions. The reorganization will also be useful for a future commit. Signed-off-by: Justin Pettit Another clue - after applying this patch I begin seeing this error message in classify_upcall() in ofproto_dpif_upcall.c. 2018-01-02T18:56:58.442Z|1|ofproto_dpif_upcall(handler18)|WARN|invalid user cookie of type 0 and size 4 This occurs even in the manual test I outlined below. After this occurs in the 'make check-kmod TESTSUITEFLAGS="1"' test I'm running I also then see these errors: 2018-01-02T18:40:02.717Z|00039|netdev_linux|WARN|ovs-p1: removing policing failed: No such device 2018-01-02T18:40:02.718Z|00040|ofproto|WARN|br0: cannot get STP status on nonexistent port 2 2018-01-02T18:40:02.718Z|00041|ofproto|WARN|br0: cannot get RSTP status on nonexistent port 2 2018-01-02T18:40:02.719Z|00042|bridge|INFO|bridge br0: deleted interface ovs-p1 on port 2 2018-01-02T18:40:02.723Z|00043|bridge|WARN|could not open network device ovs-p1 (No such device) 2018-01-02T18:40:02.742Z|00044|bridge|INFO|bridge br0: deleted interface ovs-p0 on port 1 2018-01-02T18:40:02.746Z|00045|bridge|WARN|could not open network device ovs-p1 (No such device) 2018-01-02T18:40:02.750Z|00046|bridge|WARN|could not open network device ovs-p0 Still digging... - Greg Justin, The problem I'm seeing arises when this patch is applied. I'm puzzled though because if I create a script to do the exact same thing as the test I see no problems. Here is my script: ovs-ofctl add-flow br0 "actions=normal" ip netns add at_ns0 ip netns add at_ns1 ip link add p0 type veth peer name ovs-p0 ip link set p0 netns at_ns0 ip link set dev ovs-p0 up ovs-vsctl add-port br0 ovs-p0 -- set interface ovs-p0 external-ids:iface-id="p0" ip netns exec at_ns0 ip addr add 10.1.1.1/24 dev p0 ip netns exec at_ns0 ip link set dev p0 up ip link add p1 type veth peer name ovs-p1 ip link set p1 netns at_ns1 ip link set dev ovs-p1 up ovs-vsctl add-port br0 ovs-p1 -- set interface ovs-p1 external-ids:iface-id="p1" ip netns exec at_ns1 ip addr add 10.1.1.2/24 dev p1 ip netns exec at_ns1 ip link set dev p1 up ip netns exec at_ns0 ping -q -c 3 -i 0.3 -w 2 10.1.1.2 ip netns exec at_ns0 ping -s 1600 -q -c 3 -i 0.3 -w 2 10.1.1.2 ip netns exec at_ns0 ping -s 3200 -q -c 3 -i 0.3 -w 2 10.1.1.2 ovs-vsctl del-port br0 ovs-p1 ovs-vsctl del-port br0 ovs-p0 ip netns del at_ns0 ip netns del at_ns1 ovs-ofctl del-flows br0 SFAICT it emulates exactly what the system-traffic.at test 001 does. And it works fine... /shrug. What distribution, kernel, etc are you using for your check-kmod testing? I'll try to emulate that exactly and then see if I can get similar results. Thanks, - Greg --- ofproto/ofproto-dpif-upcall.c | 91 +-- 1 file changed, 45 insertions(+), 46 deletions(-) diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 02cf5415bc3d..46b75fe35a2b 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -183,6 +183,7 @@ struct udpif { enum upcall_type { BAD_UPCALL, /* Some kind of bug somewhere. */ MISS_UPCALL, /* A flow miss. */ + SLOW_PATH_UPCALL, /* Slow path upcall. */ SFLOW_UPCALL, /* sFlow sample. */ FLOW_SAMPLE_UPCALL, /* Per-flow sampling. */ IPFIX_UPCALL /* Per-bridge sampling. */ @@ -210,8 +211,7 @@ struct upcall { uint16_t mru; /* If !0, Maximum receive unit of fragmented IP packet */ - enum dpif_upcall_type type; /* Datapath type of the upcall. */ - const struct nlattr *userdata; /* Userdata for DPIF_UC_ACTION Upcalls. */ + enum upcall_type type; /* Type of the upcall. */ const struct nlattr *actions; /* Flow actions in DPIF_UC_ACTION Upcalls. */ bool xout_initialized; /* True if 'xout' must be uninitialized. */ @@ -235,6 +235,8 @@ struct upcall { size_t key_len; /* Datapath flow key length. */ const struct nlattr *out_tun_key; /* Datapath output tunnel key. */ + union user_action_cookie cookie; + uint64_t odp_actions_stub[1024 / 8]; /* Stub for odp_actions. */ }; @@ -367,7 +369,8 @@ static int ukey_acquire(struct udpif *, const struct dpif_flow *, static void ukey_delete__(struct udpif_key *); static void ukey_delete(struct umap *, struct udpif_key *); static enum upcall_type classify_upcall(enum dpif_upcall_type type, - const struct nlattr *userdata); + const struct nlattr *userdata, + union user_action_cookie *cookie); stati
Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support
On Tue, Jan 02, 2018 at 08:13:12AM +, Jan Scheurich wrote: > > -Original Message- > > From: ovs-dev-boun...@openvswitch.org > > [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of SatyaValli > > Sent: Monday, 01 January, 2018 11:59 > > To: d...@openvswitch.org > > Cc: Manasa Cherukupally ; Surya Muttamsetty > > ; Pavani Panthagada > > ; SatyaValli ; Lavanya > > Harivelam > > Subject: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry > > Statistics Support > > > > From: SatyaValli > > > > This Patch provides implementation Existing flow entry statistics are > > redefined as standard OXS(OpenFlow Extensible Statistics) fields for > > displaying the arbitrary flow stats.The existing Flow Stats were renamed > > as Flow Description. > > > > To support this implementation below messages are newly added > > > > OFPRAW_OFPT15_FLOW_REMOVED, > > OFPRAW_OFPST15_FLOW_REQUEST, > > OFPRAW_OFPST15_FLOW_DESC_REQUEST, > > OFPRAW_OFPST15_AGGREGATE_REQUEST, > > OFPRAW_OFPST15_FLOW_REPLY, > > OFPRAW_OFPST15_FLOW_DESC_REPLY, > > OFPRAW_OFPST15_AGGREGATE_REPLY, > > > > The current commit adds support for the new feature in flow statistics > > multipart messages,aggregate multipart messages and OXS support for flow > > removal message, individual flow description messages. > > > > "ovs-ofctl dump-flows" needs to be provided with the arbitrary OXS fields > > for displaying the desired flow stats. > > > > Below are Commands to display OXS stats field wise > > > > Flow Statistics Multipart > > ovs-ofctl dump-flows -O OpenFlow15 idle_time > > ovs-ofctl dump-flows -O OpenFlow15 packet_count > > ovs-ofctl dump-flows -O OpenFlow15 byte_count > > This would break backward compatibility for one of the most frequently used > OVS CLI commands. Why don't you introduce a new command such as "ovs-ofctl > dump-flow-stats" for the new OXS stats? I think you might be misinterpreting the meaning here. It doesn't appear to break compatibility, at least not in a major way, since it doesn't do a lot of updates to the tests that would otherwise be required. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Actualización para los retos en 2018
Estrategias efectivas para sus finanzas Reformas y estrategias fiscales 2018 19 de enero- Mtro. David Lozada Martínez - 9am-3pm El panorama económico nacional previsto para 2018 se encuentra sujeto a eventos que podrían impactar negativamente la economía mexicana, como que algún miembro del TLCAN decida abandonar el tratado, un menor dinamismo de la economía de Estados Unidos de América o de la economía mundial, una elevada volatilidad en los mercados financieros internacionales, una menor plataforma de producción de petróleo a la prevista y un incremento de las tensiones geopolíticas. BENEFICIOS DE ASISTIR: - Conocerá las principales disposiciones fiscales en la Ley de Ingresos del 2018. - Identificará la forma en que se debe cumplir con el compartir información de las empresas de subcontratación de personal. - Preverá el impacto de la actualización de las tarifas de ISR para personas físicas. - Sabrá utilizar todos los tipos de CFDI para cada tipo de operación. - Entenderá el nuevo procedimiento para cancelar un CFDI. ¿Requiere la información a la Brevedad? responda este email con la palabra: Reformas + nombre - teléfono - correo. centro telefónico:018002120744 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 05/12] openvswitch: drop unneeded newline
From: Julia Lawall Date: Wed, 27 Dec 2017 15:51:38 +0100 > OVS_NLERR prints a newline at the end of the message string, so the > message string does not need to include a newline explicitly. Done > using Coccinelle. > > Signed-off-by: Julia Lawall Applied. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] ovs-vsctl: show DPDK version
Matteo Croce writes: > On Tue, Jan 2, 2018 at 6:08 PM, Aaron Conole wrote: >> Hi Matteo, >> >> Matteo Croce writes: >> >>> Show DPDK version if Open vSwitch is compiled with DPDK support. >>> Add a `dpdk_version` column in the datamodel, change ovs-ctl and ovs-dev.py >>> to populate the field with the right value. >>> >>> Signed-off-by: Matteo Croce >>> --- >> >> Sorry I missed some of the development for this - I think it's better to >> have the dpdk version retrieved via an appctl command. I can't think of >> a need for this information to be persistent - if it's for historical >> information when debugging, we can put it in with the logs when dpdk is >> started. The user cannot influence DPDK version dynamically - it is not >> just read-only, but it's read-only from a compile time decision. I >> might be misunderstanding the point of putting this in the DB, though? > > Hi Aaron, > > I was threating dpdk_version the same way as ovs_version. Ahh, okay. I had forgotten that ovs_version (and other information) was in the db as well. Sorry for that - next time I'll have coffee before reviewing. :) > ovs_version is saved into the DB by ovs-ctl and it isn't read-only either. > What about putting it only in `ovs-vswitchd --version` output and not > into the DB? What about using the datapath_version field? That field already gets populated, and I think it may be okay to modify the return value of dpif_netdev_get_datapath_version function when dpdk is enabled. Just a thought. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] faq: Correct location of flow.h.
On Tue, Jan 02, 2018 at 10:14:42AM -0800, Gregory Rose wrote: > On 1/2/2018 8:31 AM, Ben Pfaff wrote: > >Reported-by: Alan Kayahan > >Signed-off-by: Ben Pfaff > >--- > > Documentation/faq/contributing.rst | 10 +- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > >diff --git a/Documentation/faq/contributing.rst > >b/Documentation/faq/contributing.rst > >index e4a37708fbca..d5226f4f7f7b 100644 > >--- a/Documentation/faq/contributing.rst > >+++ b/Documentation/faq/contributing.rst > >@@ -45,11 +45,11 @@ Q: How do I implement a new OpenFlow message? > > 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 > >-``include/openvswitch/meta-flow.h``, following the existing pattern. If > >-the field uses a new OXM class, add it to OXM_CLASSES in > >-``build-aux/extract-ofp-fields``. Also, add support to > >+A: Add new members for your field to ``struct flow`` in > >+``include/openvswitch/flow.h``, and add new enumerations for your new > >field > >+to ``enum mf_field_id`` in ``include/openvswitch/meta-flow.h``, > >following > >+the existing pattern. If the field uses a new OXM class, add it to > >+OXM_CLASSES in ``build-aux/extract-ofp-fields``. Also, add support to > > ``miniflow_extract()`` in ``lib/flow.c`` for extracting your new field > > from > > a packet into struct miniflow, and to ``nx_put_raw()`` in > > ``lib/nx-match.c`` to output your new field in OXM matches. Then > > recompile > > LGTM > > Reviewed-by: Greg Rose Thanks, applied to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netdev: Refactor datapath flow cache
Hi, William, Thanks for the comment. You are right. If the RSS hash does not consider the fields that matter, the situation you mentioned could happen. There are two design options for CD as you may find, when the signatures collide, we could either replace the existing entry (the current design), or still insert the entry into the cache. If we chose the second design, I think the situation you mentioned could be alleviated. We chose the first one mostly because of its simplicity and speed for the hit case. For example, if we allow multiple entries with the same signature stay in one bucket, then the lookup function needs to iterate all the entries in a bucket to find all the matches (for scalar version). And additional loops and variables are required to iterate all the matches. We expected to see some percentage of throughput influence for cache hit cases. But as you suggested, if the situation you mentioned is very common in real use cases, and RSS does not consider the vlan id, we could choose to not overwrite. Another option is to reduce the insertion rate (or turn off CD) as CD's miss ratio is high (this is similar to the EMC conditional insertion). Then the 100% miss ratio case can be alleviated. This is an easy change for CD. Or we could use software hash together with RSS to consider vlan tag, this could benefit EMC too I guess. There are many design options and trade-offs but we eventually want to have a design that work for most use cases. Thanks Yipeng >-Original Message- > >Hi Yipeng, > >I like the idea of using Cuckoo Distributer, but I also have some >doubts about the RSS hash collision. (Sorry for jumping into the >discussion.) > >If there are rules using fields outside the 5-tuple, for example: >rule1 match: 5-tuple and vlan_id >= 1024 >rule2 match: 5-tuple and vlan_id < 1024 >Then rule1 will have it's unique mask in subtable1 and rule2 has >another mask in subtable2. > >For packets with the same 5-tuple, but different vlan_id, they will >collide into the same CD hash table, and potentially pointing to the >wrong subtable. Since the current OVS-CD implementation (if I >understand correctly) will overwrite the existing CD hash entry, if >the traffic have patterns like one packet matching rule1 and another >matching rule2, and so on, then the lookup miss for rule1 will insert >entry pointing to subtable1, and the second packet targeting rule2 >will miss again and insert entry pointing to subtable2, and so on. In >the end the miss rate will be 100% in such a worst case? > >Regards, >William ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] ovs-vsctl: show DPDK version
On Tue, Jan 02, 2018 at 06:18:03PM +0100, Matteo Croce wrote: > On Tue, Jan 2, 2018 at 6:08 PM, Aaron Conole wrote: > > Hi Matteo, > > > > Matteo Croce writes: > > > >> Show DPDK version if Open vSwitch is compiled with DPDK support. > >> Add a `dpdk_version` column in the datamodel, change ovs-ctl and ovs-dev.py > >> to populate the field with the right value. > >> > >> Signed-off-by: Matteo Croce > >> --- > > > > Sorry I missed some of the development for this - I think it's better to > > have the dpdk version retrieved via an appctl command. I can't think of > > a need for this information to be persistent - if it's for historical > > information when debugging, we can put it in with the logs when dpdk is > > started. The user cannot influence DPDK version dynamically - it is not > > just read-only, but it's read-only from a compile time decision. I > > might be misunderstanding the point of putting this in the DB, though? > > Hi Aaron, > > I was threating dpdk_version the same way as ovs_version. > ovs_version is saved into the DB by ovs-ctl and it isn't read-only either. > What about putting it only in `ovs-vswitchd --version` output and not > into the DB? One reason that ovs_version is in the database is because a controller might need to know and the controller can't necessarily run "ovs-vswitchd --version" since it might be on a different host. I don't know whether a controller needs to know the DPDK version. I assumed that it did and that that was the motivation for the commit. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [no-slow 6/6] ofproto-dpif: Don't slow-path controller actions with pause.
On Thu, Dec 21, 2017 at 02:25:15PM -0800, Justin Pettit wrote: > A previous patch removed slow-pathing for controller actions with the > exception of ones that specified "pause". This commit removes that > restriction so that no controller actions are slow-pathed. > > Signed-off-by: Justin Pettit One worthwhile checkpatch warning: WARNING: Line length is >79-characters long #175 FILE: ofproto/ofproto-dpif-xlate.c:4372: ctx->xin->flow.in_port.ofp_port); Another valuable simplification, thanks again. Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [no-slow 5/6] ofproto-dpif: Don't slow-path controller actions.
On Thu, Dec 21, 2017 at 02:25:14PM -0800, Justin Pettit wrote: > Controller actions have become more commonly used for purposes other > than just making forwarding decisions (e.g., packet logging). A packet > that needs to be copied to the controller and forwarded would always be > sent to ovs-vswitchd to be handled, which could negatively affect > performance and cause heavier CPU utilization in ovs-vswitchd. > > This commit changes the behavior so that OpenFlow controller actions > become userspace datapath actions while continuing to let packet > forwarding and manipulation continue to be handled by the datapath > directly. > > This patch still slow-paths controller actions with the "pause" flag > set. A future patch will stop slow-pathing these pause actions as > well. > > Signed-off-by: Justin Pettit checkpatch says: WARNING: Line length is >79-characters long #22 FILE: lib/odp-util.c:482: && cookie.header.type == USER_ACTION_COOKIE_CONTROLLER) { This is missing code from parse_odp_userspace_action() that should be able to parse the formatted actions. I would add a test to "OVS datapath actions parsing and formatting - valid forms" in tests/odp.at for parsing and formatting the new kind of userspace action. This is a nice simplification! I thought it would make the code bigger and more complicated, but it shrank it (at least in the worst areas) and made it simpler. Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [no-slow 1/6] ofproto-dpif: Add ability to look up an ofproto by UUID.
> On Jan 2, 2018, at 8:40 AM, Ben Pfaff wrote: > > On Thu, Dec 21, 2017 at 02:25:10PM -0800, Justin Pettit wrote: >> This will have callers in the future. >> >> Signed-off-by: Justin Pettit > > The new comment in struct ofproto_dpif only refers to one of the > hmap_nodes. > > Acked-by: Ben Pfaff Thanks. I added a comment. --Justin ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] faq: Correct location of flow.h.
On 1/2/2018 8:31 AM, Ben Pfaff wrote: Reported-by: Alan Kayahan Signed-off-by: Ben Pfaff --- Documentation/faq/contributing.rst | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/faq/contributing.rst b/Documentation/faq/contributing.rst index e4a37708fbca..d5226f4f7f7b 100644 --- a/Documentation/faq/contributing.rst +++ b/Documentation/faq/contributing.rst @@ -45,11 +45,11 @@ Q: How do I implement a new OpenFlow message? 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 -``include/openvswitch/meta-flow.h``, following the existing pattern. If -the field uses a new OXM class, add it to OXM_CLASSES in -``build-aux/extract-ofp-fields``. Also, add support to +A: Add new members for your field to ``struct flow`` in +``include/openvswitch/flow.h``, and add new enumerations for your new field +to ``enum mf_field_id`` in ``include/openvswitch/meta-flow.h``, following +the existing pattern. If the field uses a new OXM class, add it to +OXM_CLASSES in ``build-aux/extract-ofp-fields``. Also, add support to ``miniflow_extract()`` in ``lib/flow.c`` for extracting your new field from a packet into struct miniflow, and to ``nx_put_raw()`` in ``lib/nx-match.c`` to output your new field in OXM matches. Then recompile LGTM Reviewed-by: Greg Rose ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [no-slow 4/6] ofproto: Add 'ofproto_uuid' and 'ofp_in_port' to user action cookie.
On Thu, Dec 21, 2017 at 02:25:13PM -0800, Justin Pettit wrote: > Previously, the ofproto instance and OpenFlow port have been derived > based on the datapath port number. This change explicitly declares them > both, which will be helpful in future commits that no longer can depend > on having a unique datapath port (e.g., a source port that represents > the controller). > > Signed-off-by: Justin Pettit Some checkpatch warnings look legit: WARNING: Line length is >79-characters long #34 FILE: lib/odp-util.c:457: && cookie.header.type == USER_ACTION_COOKIE_FLOW_SAMPLE) { WARNING: Line length is >79-characters long #259 FILE: ofproto/ofproto-dpif-upcall.c:1101: = ofproto_dpif_lookup_by_uuid(&upcall->cookie.header.ofproto_uuid); WARNING: Line length is >79-characters long #307 FILE: ofproto/ofproto-dpif-upcall.c:2072: ofp_in_port, odp_actions, smid, cmid, &ofproto->uuid); There's a stray " in this line in odp-util.h: +enum user_action_cookie_type type; /* One of USER_ACTION_COOKIE_*" */ Maybe you like the way you did it, which is fine, but an alternative way to arrange user_action_cookie would be to make it a struct with the standard header followed by embedding further structs inside an anonymous union, like below. Then you could omit lots of ".header." stuff although you'd need to s/union/struct/ in other places. /* user_action_cookie is passed as argument to OVS_ACTION_ATTR_USERSPACE. */ struct user_action_cookie { enum user_action_cookie_type type; /* One of USER_ACTION_COOKIE_*" */ ofp_port_t ofp_in_port;/* OpenFlow in port, or OFPP_NONE. */ struct uuid ofproto_uuid; /* UUID of ofproto-dpif. */ union { struct { /* USER_ACTION_COOKIE_SFLOW. */ ovs_be16 vlan_tci; /* Destination VLAN TCI. */ uint32_t output;/* SFL_FLOW_SAMPLE_TYPE 'output' value. */ } sflow; struct { /* USER_ACTION_COOKIE_SLOW_PATH. */ uint32_t reason;/* enum slow_path_reason. */ } slow_path; struct { /* USER_ACTION_COOKIE_FLOW_SAMPLE. */ uint16_t probability; /* Sampling probability. */ uint32_t collector_set_id; /* ID of IPFIX collector set. */ uint32_t obs_domain_id; /* Observation Domain ID. */ uint32_t obs_point_id; /* Observation Point ID. */ odp_port_t output_odp_port; /* The output odp port. */ enum nx_action_sample_direction direction; } flow_sample; struct { /* USER_ACTION_COOKIE_IPFIX. */ odp_port_t output_odp_port; /* The output odp port. */ } ipfix; }; }; It looks like the shortest user_action_cookie variation is 24 bytes, the longest is 48. It may not be worth it to treat user_action_cookie as variable-length now. I think that it was done this way to better tolerate old datapaths that had a short, fixed maximum length for userdata. It would be simpler to just always ship sizeof(union user_action_cookie) bytes to the datapath and always require sizeof(union user_action_cookie) back. You could then skip a lot of userdata_len checks and updates for each type of cookie. Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] ovs-vsctl: show DPDK version
On Tue, Jan 2, 2018 at 6:08 PM, Aaron Conole wrote: > Hi Matteo, > > Matteo Croce writes: > >> Show DPDK version if Open vSwitch is compiled with DPDK support. >> Add a `dpdk_version` column in the datamodel, change ovs-ctl and ovs-dev.py >> to populate the field with the right value. >> >> Signed-off-by: Matteo Croce >> --- > > Sorry I missed some of the development for this - I think it's better to > have the dpdk version retrieved via an appctl command. I can't think of > a need for this information to be persistent - if it's for historical > information when debugging, we can put it in with the logs when dpdk is > started. The user cannot influence DPDK version dynamically - it is not > just read-only, but it's read-only from a compile time decision. I > might be misunderstanding the point of putting this in the DB, though? Hi Aaron, I was threating dpdk_version the same way as ovs_version. ovs_version is saved into the DB by ovs-ctl and it isn't read-only either. What about putting it only in `ovs-vswitchd --version` output and not into the DB? -- Matteo Croce per aspera ad upstream ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] ovs-vsctl: show DPDK version
Hi Matteo, Matteo Croce writes: > Show DPDK version if Open vSwitch is compiled with DPDK support. > Add a `dpdk_version` column in the datamodel, change ovs-ctl and ovs-dev.py > to populate the field with the right value. > > Signed-off-by: Matteo Croce > --- Sorry I missed some of the development for this - I think it's better to have the dpdk version retrieved via an appctl command. I can't think of a need for this information to be persistent - if it's for historical information when debugging, we can put it in with the logs when dpdk is started. The user cannot influence DPDK version dynamically - it is not just read-only, but it's read-only from a compile time decision. I might be misunderstanding the point of putting this in the DB, though? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [no-slow 3/6] ofp-actions: Add action "debug_slow" for testing slow-path.
On Thu, Dec 21, 2017 at 02:25:12PM -0800, Justin Pettit wrote: > It isn't otherwise useful and in fact hurts performance so it's disabled > without --enable-dummy. > > An upcoming commit will make use of this. > > Signed-off-by: Justin Pettit Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [no-slow 2/6] ofproto-dpif: Reorganize upcall handling.
On Thu, Dec 21, 2017 at 02:25:11PM -0800, Justin Pettit wrote: > - This reduces the number of times upcall cookies are processed. > - It separate true miss calls from slow-path actions. > > The reorganization will also be useful for a future commit. > > Signed-off-by: Justin Pettit Assuming you can figure out what Greg is seeing: Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [no-slow 1/6] ofproto-dpif: Add ability to look up an ofproto by UUID.
On Thu, Dec 21, 2017 at 02:25:10PM -0800, Justin Pettit wrote: > This will have callers in the future. > > Signed-off-by: Justin Pettit The new comment in struct ofproto_dpif only refers to one of the hmap_nodes. Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] API for connecting ovs bridge to manage packets
On Fri, Dec 29, 2017 at 11:39:16AM +0800, Gao Zhenyu wrote: > As you know ovn consumes rconn functions in pinctrl.c to connect the > bridge. But the rconn functions are private functions. I mean those > functions are not exposed in include/openvswitch/*.h file, so they are not > regular APIs, right? > So do you know if we get a way to connect the bridge but not using > rconn functions? > Then I can make a controller to manage the IN/OUT packets without > compiling whole ovs. I don't see any real obstacle to making rconn a public API. Would that be the best solution for you? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] faq: Correct location of flow.h.
Reported-by: Alan Kayahan Signed-off-by: Ben Pfaff --- Documentation/faq/contributing.rst | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/faq/contributing.rst b/Documentation/faq/contributing.rst index e4a37708fbca..d5226f4f7f7b 100644 --- a/Documentation/faq/contributing.rst +++ b/Documentation/faq/contributing.rst @@ -45,11 +45,11 @@ Q: How do I implement a new OpenFlow message? 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 -``include/openvswitch/meta-flow.h``, following the existing pattern. If -the field uses a new OXM class, add it to OXM_CLASSES in -``build-aux/extract-ofp-fields``. Also, add support to +A: Add new members for your field to ``struct flow`` in +``include/openvswitch/flow.h``, and add new enumerations for your new field +to ``enum mf_field_id`` in ``include/openvswitch/meta-flow.h``, following +the existing pattern. If the field uses a new OXM class, add it to +OXM_CLASSES in ``build-aux/extract-ofp-fields``. Also, add support to ``miniflow_extract()`` in ``lib/flow.c`` for extracting your new field from a packet into struct miniflow, and to ``nx_put_raw()`` in ``lib/nx-match.c`` to output your new field in OXM matches. Then recompile -- 2.10.2 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovn-nbctl: Add lrp's gateway chassis information in show command
On Tue, Jan 02, 2018 at 06:13:27PM +0530, nusid...@redhat.com wrote: > From: Numan Siddique > > Having this information in the show command will be helpful. > > Signed-off-by: Numan Siddique Applied, thanks! ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] master now "frozen" for forking
Hello everyone. We are at the point in the release cycle where traditionally we would fork a branch from master for release. We have tried a slightly different approach a few times and I'd like to propose that we do it again. Instead of forking immediately, I propose that we "freeze" master so that only bug fixes and previously posted feature enhancements go in, until approximately January 15, and then fork branch-2.9 from master. Unless anyone has a serious objection, let's do this. Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] OVS DPDK: dpdk_merge pull request
Thanks! I merged this to master. On Wed, Dec 20, 2017 at 10:38:19PM +, Stokes, Ian wrote: > Hi Ben, > > The following changes since commit fa233667eecac83c68662702622f4d0c66d2364b: > > bond: Fix bug that writes to freed memory (2017-12-20 09:36:39 -0800) > > are available in the git repository at: > > https://github.com/istokes/ovs dpdk_merge > > for you to fetch changes up to cc4891f39d574986948ac87280cfe9017fe17a39: > > dpif-netdev: Count sent packets and batches. (2017-12-20 21:07:46 +) > > > Ilya Maximets (8): > vswitchd: Document netdev-dpdk commands. > netdev-dpdk: Add debug appctl to get mempool information. > docs: Fix table view for VM config in dpdk howto. > dpif-netdev: Keep latest measured time for PMD thread. > dpif-netdev: Output packet batching. > netdev: Remove unused may_steal. > netdev: Remove useless cutlen. > dpif-netdev: Count sent packets and batches. > > Documentation/howto/dpdk.rst | 22 +- > NEWS | 2 ++ > lib/automake.mk | 1 + > lib/dpif-netdev.c| 244 > +++- > lib/netdev-bsd.c | 6 ++--- > lib/netdev-dpdk-unixctl.man | 14 > lib/netdev-dpdk.c| 84 > + > lib/netdev-dummy.c | 6 ++--- > lib/netdev-linux.c | 8 +++ > lib/netdev-provider.h| 7 +++--- > lib/netdev.c | 12 -- > lib/netdev.h | 2 +- > manpages.mk | 2 ++ > vswitchd/ovs-vswitchd.8.in | 1 + > 14 files changed, 290 insertions(+), 121 deletions(-) > create mode 100644 lib/netdev-dpdk-unixctl.man > > Thanks > Ian ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [Cluster-devel] [PATCH 00/12] drop unneeded newline
On Tue, 2018-01-02 at 15:00 +0100, Julia Lawall wrote: > On Tue, 2 Jan 2018, Bob Peterson wrote: > > - Original Message - > > > - Original Message - > > > > > Still, the GFS2 and DLM code has a plethora of broken-up printk messages, > > and I don't like the thought of re-combining them all. > > Actually, the point of the patch was to remove the unnecessary \n at the > end of the string, because log_print will add another one. If you prefer > to keep the string broken up, I can resend the patch in that form, but > without the unnecessary \n. Please combine any user-visible strings into a single line for which the unneeded newline is dropped since these strings are modified anyway by your patch. Thanks, Bart. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [Cluster-devel] [PATCH 00/12] drop unneeded newline
On Tue, 2 Jan 2018, Bart Van Assche wrote: > On Tue, 2018-01-02 at 15:00 +0100, Julia Lawall wrote: > > On Tue, 2 Jan 2018, Bob Peterson wrote: > > > - Original Message - > > > > - Original Message - > > > > > > > Still, the GFS2 and DLM code has a plethora of broken-up printk messages, > > > and I don't like the thought of re-combining them all. > > > > Actually, the point of the patch was to remove the unnecessary \n at the > > end of the string, because log_print will add another one. If you prefer > > to keep the string broken up, I can resend the patch in that form, but > > without the unnecessary \n. > > Please combine any user-visible strings into a single line for which the > unneeded newline is dropped since these strings are modified anyway by > your patch. That is what the submitted patch (2/12 specifically) did. julia ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] tests: Add system-dpdk-testsuite
Hi Cian, Thank you for your comment. I've uploaded version V2 of the patch which resolves your findings. Best regards, Marcin -Original Message- From: Ferriter, Cian Sent: Wednesday, December 20, 2017 6:16 PM To: Rybka, MarcinX ; d...@openvswitch.org Cc: Rybka, MarcinX Subject: RE: [ovs-dev] [PATCH] tests: Add system-dpdk-testsuite Hi Marcin, Thanks for this patch on testing! I have applied it to the latest master and done some testing. Please find some feedback from my testing below: I have both an ixgbe and an i40e device on my platform, and was able to test with both. I can confirm that all three tests in this initial testsuite are passing on my platform when either an ixgbe, i40e or both devices are bound to DPDK (bound to igb_uio). When no devices are bound to DPDK, the first test passes, while the second two are skipped. One question I have with this behavior is about whether the "Add vhost-user-client port" test should be skipped when there are no devices bound to DPDK? Could this test still run regardless of there being no physical devices available? Maybe this doesn't make sense, since it is mentioned in the documentation for this patch that a DPDK supported NIC is required, but I was just wondering if there was a hard dependency here? I also have one comment, inline below, on the documentation. Let me know what you think. Thanks, Cian > -Original Message- > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > boun...@openvswitch.org] On Behalf Of marcinx.ry...@intel.com > Sent: 14 December 2017 09:07 > To: d...@openvswitch.org > Cc: Rybka, MarcinX > Subject: [ovs-dev] [PATCH] tests: Add system-dpdk-testsuite > > From: Marcin Rybka > > New OVS-DPDK testsuite, which can be launched via `make check-dpdk`, > tests OVS using a DPDK datapath. The testsuite contains already initial tests: > 1. EAL init > 2. Add standard DPDK PHY port > 3. Add vhost-user-client port > > Signed-off-by: Marcin Rybka > --- > Documentation/topics/testing.rst | 19 > tests/automake.mk| 17 +++ > tests/system-dpdk-macros.at | 54 > > tests/system-dpdk-testsuite.at | 25 +++ > tests/system-dpdk.at | 66 > > 5 files changed, 181 insertions(+) > create mode 100644 tests/system-dpdk-macros.at create mode 100644 > tests/system-dpdk-testsuite.at create mode 100644 > tests/system-dpdk.at > > diff --git a/Documentation/topics/testing.rst > b/Documentation/topics/testing.rst > index 1ecda00..e197a4c 100644 > --- a/Documentation/topics/testing.rst > +++ b/Documentation/topics/testing.rst > @@ -293,6 +293,25 @@ To invoke the datapath testsuite with the > userspace datapath, run:: > > The results of the testsuite are in ``tests/system-userspace-testsuite.dir``. > > +DPDK datapath > +' > + > +To test :doc:`/intro/install/dpdk` (i.e., the build was configured > +with ``--with-dpdk``,the ``DPDK`` is installed), run the testsuite > +and generate a report by using the ``check-dpdk`` target:: > + > +$ make check-dpdk > + > +To see a list of all the available tests, run:: > + > +$ make check-dpdk TESTSUITEFLAGS=--list > + > +These tests require a `DPDK supported NIC`_ and proper DPDK variables > +(``DPDK_DIR`` and ``DPDK_BUILD``). Moreover you need to load the > +required modules and bind the NIC to the kernel driver. [Cian Ferriter] In my testing it looks like the NIC had to be bound to the DPDK driver (igb_uio). Should the above be changed to mention this? > + > +.. _DPDK supported NIC: http://dpdk.org/doc/nics > + > Kernel datapath > ''' > > diff --git a/tests/automake.mk b/tests/automake.mk index > ad5362f..90c9b15 > 100644 > --- a/tests/automake.mk > +++ b/tests/automake.mk > @@ -5,10 +5,12 @@ EXTRA_DIST += \ > $(SYSTEM_KMOD_TESTSUITE_AT) \ > $(SYSTEM_USERSPACE_TESTSUITE_AT) \ > $(SYSTEM_OFFLOADS_TESTSUITE_AT) \ > + $(SYSTEM_DPDK_TESTSUITE_AT) \ > $(TESTSUITE) \ > $(SYSTEM_KMOD_TESTSUITE) \ > $(SYSTEM_USERSPACE_TESTSUITE) \ > $(SYSTEM_OFFLOADS_TESTSUITE) \ > + $(SYSTEM_DPDK_TESTSUITE) \ > tests/atlocal.in \ > $(srcdir)/package.m4 \ > $(srcdir)/tests/testsuite \ > @@ -125,6 +127,12 @@ SYSTEM_OFFLOADS_TESTSUITE_AT = \ > tests/system-offloads-traffic.at \ > tests/system-offloads-testsuite.at > > +SYSTEM_DPDK_TESTSUITE_AT = \ > + tests/system-common-macros.at \ > + tests/system-dpdk-macros.at \ > + tests/system-dpdk-testsuite.at \ > + tests/system-dpdk.at > + > check_SCRIPTS += tests/atlocal > > TESTSUITE = $(srcdir)/tests/testsuite @@ -132,6 +140,7 @@ > TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch > SYSTEM_KMOD_TESTSUITE = $(srcdir)/tests/system-kmod-testsuite > SYSTEM_USERSPACE_TESTSUITE = $(srcdir)/tests/system-userspace- > testsuite SYSTEM_OFFLOADS_TESTSUITE = > $(srcdir)/tests/system-offl
[ovs-dev] [PATCH v2] tests: Add system-dpdk-testsuite
New OVS-DPDK testsuite, which can be launched via `make check-dpdk`, tests OVS using a DPDK datapath. The testsuite contains already initial tests: 1. EAL init 2. Add standard DPDK PHY port 3. Add vhost-user-client port Signed-off-by: Marcin Rybka --- Documentation/topics/testing.rst | 19 tests/automake.mk| 17 +++ tests/system-dpdk-macros.at | 54 + tests/system-dpdk-testsuite.at | 25 tests/system-dpdk.at | 65 5 files changed, 180 insertions(+) create mode 100644 tests/system-dpdk-macros.at create mode 100644 tests/system-dpdk-testsuite.at create mode 100644 tests/system-dpdk.at diff --git a/Documentation/topics/testing.rst b/Documentation/topics/testing.rst index a49336b..74e0d3f 100644 --- a/Documentation/topics/testing.rst +++ b/Documentation/topics/testing.rst @@ -297,6 +297,25 @@ To invoke the datapath testsuite with the userspace datapath, run:: The results of the testsuite are in ``tests/system-userspace-testsuite.dir``. +DPDK datapath +' + +To test :doc:`/intro/install/dpdk` (i.e., the build was configured with +``--with-dpdk``,the ``DPDK`` is installed), run the testsuite and generate +a report by using the ``check-dpdk`` target:: + +$ make check-dpdk + +To see a list of all the available tests, run:: + +$ make check-dpdk TESTSUITEFLAGS=--list + +These tests require a `DPDK supported NIC`_ and proper DPDK variables +(``DPDK_DIR`` and ``DPDK_BUILD``). Moreover you need to load the required +modules and bind the NIC to the DPDK-compatible driver. + +.. _DPDK supported NIC: http://dpdk.org/doc/nics + Kernel datapath ''' diff --git a/tests/automake.mk b/tests/automake.mk index 8157641..7be5712 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -5,10 +5,12 @@ EXTRA_DIST += \ $(SYSTEM_KMOD_TESTSUITE_AT) \ $(SYSTEM_USERSPACE_TESTSUITE_AT) \ $(SYSTEM_OFFLOADS_TESTSUITE_AT) \ + $(SYSTEM_DPDK_TESTSUITE_AT) \ $(TESTSUITE) \ $(SYSTEM_KMOD_TESTSUITE) \ $(SYSTEM_USERSPACE_TESTSUITE) \ $(SYSTEM_OFFLOADS_TESTSUITE) \ + $(SYSTEM_DPDK_TESTSUITE) \ tests/atlocal.in \ $(srcdir)/package.m4 \ $(srcdir)/tests/testsuite \ @@ -126,6 +128,12 @@ SYSTEM_OFFLOADS_TESTSUITE_AT = \ tests/system-offloads-traffic.at \ tests/system-offloads-testsuite.at +SYSTEM_DPDK_TESTSUITE_AT = \ + tests/system-common-macros.at \ + tests/system-dpdk-macros.at \ + tests/system-dpdk-testsuite.at \ + tests/system-dpdk.at + check_SCRIPTS += tests/atlocal TESTSUITE = $(srcdir)/tests/testsuite @@ -133,6 +141,7 @@ TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch SYSTEM_KMOD_TESTSUITE = $(srcdir)/tests/system-kmod-testsuite SYSTEM_USERSPACE_TESTSUITE = $(srcdir)/tests/system-userspace-testsuite SYSTEM_OFFLOADS_TESTSUITE = $(srcdir)/tests/system-offloads-testsuite +SYSTEM_DPDK_TESTSUITE = $(srcdir)/tests/system-dpdk-testsuite DISTCLEANFILES += tests/atconfig tests/atlocal AUTOTEST_PATH = utilities:vswitchd:ovsdb:vtep:tests:$(PTHREAD_WIN32_DIR_DLL):ovn/controller-vtep:ovn/northd:ovn/utilities:ovn/controller @@ -256,6 +265,10 @@ check-offloads: all set $(SHELL) '$(SYSTEM_OFFLOADS_TESTSUITE)' -C tests AUTOTEST_PATH='$(AUTOTEST_PATH)' $(TESTSUITEFLAGS) -j1; \ "$$@" || (test X'$(RECHECK)' = Xyes && "$$@" --recheck) +check-dpdk: all + set $(SHELL) '$(SYSTEM_DPDK_TESTSUITE)' -C tests AUTOTEST_PATH='$(AUTOTEST_PATH)' $(TESTSUITEFLAGS) -j1; \ + "$$@" || (test X'$(RECHECK)' = Xyes && "$$@" --recheck) + clean-local: test ! -f '$(TESTSUITE)' || $(SHELL) '$(TESTSUITE)' -C tests --clean @@ -284,6 +297,10 @@ $(SYSTEM_OFFLOADS_TESTSUITE): package.m4 $(SYSTEM_TESTSUITE_AT) $(SYSTEM_OFFLOAD $(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o $@.tmp $@.at $(AM_V_at)mv $@.tmp $@ +$(SYSTEM_DPDK_TESTSUITE): package.m4 $(SYSTEM_TESTSUITE_AT) $(SYSTEM_DPDK_TESTSUITE_AT) $(COMMON_MACROS_AT) + $(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o $@.tmp $@.at + $(AM_V_at)mv $@.tmp $@ + # The `:;' works around a Bash 3.2 bug when the output is not writeable. $(srcdir)/package.m4: $(top_srcdir)/configure.ac $(AM_V_GEN):;{ \ diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at new file mode 100644 index 000..cbd0f69 --- /dev/null +++ b/tests/system-dpdk-macros.at @@ -0,0 +1,54 @@ +# OVS_DPDK_PRE_CHECK() +# +# Check prerequisites for DPDK tests. Following settings are checked: +# - Hugepages +# - UIO driver +# +m4_define([OVS_DPDK_PRE_CHECK], + [dnl Check Hugepages + AT_CHECK([cat /proc/meminfo], [], [stdout]) + AT_CHECK([grep HugePages_ stdout], [], [stdout]) + AT_CHECK([mount], [], [stdout]) + AT_CHECK([grep 'hugetlbfs' stdout], [], [stdout], []) + + dnl Check if VFIO or UIO driver is loaded + AT_CHECK([lsmod | grep -E "igb_uio|vfio"
Re: [ovs-dev] [PATCH] netdev-dpdk: fix port addition for ports sharing same PCI id
ping ... On Thu, Dec 21, 2017 at 12:03:36AM +0800, Yuanhan Liu wrote: > Some NICs have only one PCI address associated with multiple ports. This > patch extends the dpdk-devargs option's format to cater for such devices. > > To achieve that, this patch uses a new syntax that will be adapted and > implemented in future DPDK release (likely, v18.05): > http://dpdk.org/ml/archives/dev/2017-December/084234.html > > And since it's the DPDK duty to parse the (complete and full) syntax > and this patch is more likely to serve as an intermediate workaround, > here I take a simpler and shorter syntax from it (note it's allowed to > have only one category being provided): > class=eth,mac=00:11:22:33:44:55:66 > > Also, old compatibility is kept. Users can still go on with using the > PCI id to add a port (if that's enough for them). Meaning, this patch > will not break anything. > > This patch is basically based on the one from Ciara: > https://mail.openvswitch.org/pipermail/ovs-dev/2017-October/339496.html > > Cc: Loftus Ciara > Cc: Thomas Monjalon > Cc: Kevin Traynor > Signed-off-by: Yuanhan Liu > --- > lib/netdev-dpdk.c | 77 > --- > 1 file changed, 62 insertions(+), 15 deletions(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index 45fcc74..4e5cc25 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -1205,30 +1205,77 @@ netdev_dpdk_lookup_by_port_id(dpdk_port_t port_id) > return NULL; > } > > +static int > +netdev_dpdk_str_to_ether(const char *mac, struct ether_addr *ea) > +{ > +unsigned int bytes[6]; > +int i; > + > +if (sscanf(mac, "%x:%x:%x:%x:%x:%x", > + &bytes[0], &bytes[1], &bytes[2], > + &bytes[3], &bytes[4], &bytes[5]) != 6) { > +return -1; > +} > + > +for (i = 0; i < 6; i++) { > +ea->addr_bytes[i] = bytes[i]; > +} > + > +return 0; > +} > + > +static dpdk_port_t > +netdev_dpdk_get_port_by_mac(const char *mac_str) > +{ > +int i; > +struct ether_addr mac; > +struct ether_addr port_mac; > + > +netdev_dpdk_str_to_ether(mac_str, &mac); > +for (i = 0; i < RTE_MAX_ETHPORTS; i++) { > +if (!rte_eth_dev_is_valid_port(i)) { > +continue; > +} > + > +rte_eth_macaddr_get(i, &port_mac); > +if (is_same_ether_addr(&mac, &port_mac)) { > +return i; > +} > +} > + > +return DPDK_ETH_PORT_ID_INVALID; > +} > + > static dpdk_port_t > netdev_dpdk_process_devargs(struct netdev_dpdk *dev, > const char *devargs, char **errp) > { > -/* Get the name up to the first comma. */ > -char *name = xmemdup0(devargs, strcspn(devargs, ",")); > +char *name; > dpdk_port_t new_port_id = DPDK_ETH_PORT_ID_INVALID; > > -if (rte_eth_dev_get_port_by_name(name, &new_port_id) > -|| !rte_eth_dev_is_valid_port(new_port_id)) { > -/* Device not found in DPDK, attempt to attach it */ > -if (!rte_eth_dev_attach(devargs, &new_port_id)) { > -/* Attach successful */ > -dev->attached = true; > -VLOG_INFO("Device '%s' attached to DPDK", devargs); > -} else { > -/* Attach unsuccessful */ > -new_port_id = DPDK_ETH_PORT_ID_INVALID; > -VLOG_WARN_BUF(errp, "Error attaching device '%s' to DPDK", > - devargs); > +if (strncmp(devargs, "class=eth,mac=", 14) == 0) { > +new_port_id = netdev_dpdk_get_port_by_mac(&devargs[14]); > +} else { > +name = xmemdup0(devargs, strcspn(devargs, ",")); > +if (rte_eth_dev_get_port_by_name(name, &new_port_id) > +|| !rte_eth_dev_is_valid_port(new_port_id)) { > +/* Device not found in DPDK, attempt to attach it */ > +if (!rte_eth_dev_attach(devargs, &new_port_id)) { > +/* Attach successful */ > +dev->attached = true; > +VLOG_INFO("Device '%s' attached to DPDK", devargs); > +} else { > +/* Attach unsuccessful */ > +new_port_id = DPDK_ETH_PORT_ID_INVALID; > +} > } > +free(name); > +} > + > +if (new_port_id == DPDK_ETH_PORT_ID_INVALID) { > +VLOG_WARN_BUF(errp, "Error attaching device '%s' to DPDK", devargs); > } > > -free(name); > return new_port_id; > } > > -- > 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [Cluster-devel] [PATCH 00/12] drop unneeded newline
On Tue, 2 Jan 2018, Bob Peterson wrote: > - Original Message - > | - Original Message - > | | Drop newline at the end of a message string when the printing function > adds > | | a newline. > | > | Hi Julia, > | > | NACK. > | > | As much as it's a pain when searching the source code for output strings, > | this patch set goes against the accepted Linux coding style document. See: > | > | > https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings > | > | Regards, > | > | Bob Peterson > | > | > Hm. I guess I stand corrected. The document reads: > > "However, never break user-visible strings such as printk messages, because > that breaks the ability to grep for them." > > Still, the GFS2 and DLM code has a plethora of broken-up printk messages, > and I don't like the thought of re-combining them all. Actually, the point of the patch was to remove the unnecessary \n at the end of the string, because log_print will add another one. If you prefer to keep the string broken up, I can resend the patch in that form, but without the unnecessary \n. julia ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [Cluster-devel] [PATCH 00/12] drop unneeded newline
- Original Message - | - Original Message - | | Drop newline at the end of a message string when the printing function adds | | a newline. | | Hi Julia, | | NACK. | | As much as it's a pain when searching the source code for output strings, | this patch set goes against the accepted Linux coding style document. See: | | https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings | | Regards, | | Bob Peterson | | Hm. I guess I stand corrected. The document reads: "However, never break user-visible strings such as printk messages, because that breaks the ability to grep for them." Still, the GFS2 and DLM code has a plethora of broken-up printk messages, and I don't like the thought of re-combining them all. Regards, Bob Peterson ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [Cluster-devel] [PATCH 00/12] drop unneeded newline
On Tue, 2 Jan 2018, Bob Peterson wrote: > - Original Message - > | Drop newline at the end of a message string when the printing function adds > | a newline. > > Hi Julia, > > NACK. > > As much as it's a pain when searching the source code for output strings, > this patch set goes against the accepted Linux coding style document. See: > > https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings I don't think that's the case: "However, never break user-visible strings such as printk messages, because that breaks the ability to grep for them." julia > > Regards, > > Bob Peterson > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [Cluster-devel] [PATCH 00/12] drop unneeded newline
- Original Message - | Drop newline at the end of a message string when the printing function adds | a newline. Hi Julia, NACK. As much as it's a pain when searching the source code for output strings, this patch set goes against the accepted Linux coding style document. See: https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings Regards, Bob Peterson ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ovn-nbctl: Add lrp's gateway chassis information in show command
From: Numan Siddique Having this information in the show command will be helpful. Signed-off-by: Numan Siddique --- ovn/utilities/ovn-nbctl.c | 9 + 1 file changed, 9 insertions(+) diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c index c920ad878..c9aa2fef4 100644 --- a/ovn/utilities/ovn-nbctl.c +++ b/ovn/utilities/ovn-nbctl.c @@ -614,6 +614,15 @@ print_lr(const struct nbrec_logical_router *lr, struct ds *s) } ds_put_cstr(s, "]\n"); } + +if (lrp->n_gateway_chassis) { +ds_put_cstr(s, "gateway chassis: ["); +for (size_t j = 0; j < lrp->n_gateway_chassis; j++) { +ds_put_format(s, "%s ", lrp->gateway_chassis[j]->chassis_name); +} +ds_chomp(s, ' '); +ds_put_cstr(s, "]\n"); +} } for (size_t i = 0; i < lr->n_nat; i++) { -- 2.14.3 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [OVN][Request] New OVS 2.8 tag
Hi folks, It'd be great if we could have 2.8.2 tag so that we can benefit from some patches that we would require in OVN and its OpenStack integration such as: * OVN: Add external_ids to NAT and Logical_Router_Static_Route tables [0] * ovn-northd; Treat logical ports of router type as always being up [1] * OVN pacemaker: Add the monitor action for Master role [2] * Check flow's dl_type before setting ct_orig_tuple in 'pkt_metadata_from_flow()' [3] Thanks a lot in advance! Daniel [0] https://github.com/openvswitch/ovs/commit/c4c1e1a5af31c65c7b78e37973d5113f64339701 [1] https://github.com/openvswitch/ovs/commit/75ac161b2a1b147ccc60d4cdd7fb9b90193491ba [2] https://github.com/openvswitch/ovs/commit/a07b7dafcf0830340dd12a381c1cb1cd03558c81 [3] https://github.com/openvswitch/ovs/commit/17d98668019271ebae0c6f811371d0dd081ea56a ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support
> -Original Message- > From: ovs-dev-boun...@openvswitch.org > [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of SatyaValli > Sent: Monday, 01 January, 2018 11:59 > To: d...@openvswitch.org > Cc: Manasa Cherukupally ; Surya Muttamsetty > ; Pavani Panthagada > ; SatyaValli ; Lavanya Harivelam > > Subject: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry > Statistics Support > > From: SatyaValli > > This Patch provides implementation Existing flow entry statistics are > redefined as standard OXS(OpenFlow Extensible Statistics) fields for > displaying the arbitrary flow stats.The existing Flow Stats were renamed > as Flow Description. > > To support this implementation below messages are newly added > > OFPRAW_OFPT15_FLOW_REMOVED, > OFPRAW_OFPST15_FLOW_REQUEST, > OFPRAW_OFPST15_FLOW_DESC_REQUEST, > OFPRAW_OFPST15_AGGREGATE_REQUEST, > OFPRAW_OFPST15_FLOW_REPLY, > OFPRAW_OFPST15_FLOW_DESC_REPLY, > OFPRAW_OFPST15_AGGREGATE_REPLY, > > The current commit adds support for the new feature in flow statistics > multipart messages,aggregate multipart messages and OXS support for flow > removal message, individual flow description messages. > > "ovs-ofctl dump-flows" needs to be provided with the arbitrary OXS fields > for displaying the desired flow stats. > > Below are Commands to display OXS stats field wise > > Flow Statistics Multipart > ovs-ofctl dump-flows -O OpenFlow15 idle_time > ovs-ofctl dump-flows -O OpenFlow15 packet_count > ovs-ofctl dump-flows -O OpenFlow15 byte_count This would break backward compatibility for one of the most frequently used OVS CLI commands. Why don't you introduce a new command such as "ovs-ofctl dump-flow-stats" for the new OXS stats? > > Aggregate Flow Statistics Multipart > ovs-ofctl dump-aggregate -O OpenFlow15 packet_count > ovs-ofctl dump-aggregate -O OpenFlow15 byte_count > > Flow Descritption > ovs-ofctl dump-flow-desc -O OpenFlow15 idle_time > ovs-ofctl dump-flow-desc -O OpenFlow15 packet_count > ovs-ofctl dump-flow-desc -O OpenFlow15 byte_count > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev