Re: [ovs-dev] [PATCH v2] openvswitch: Trim off padding before L3+ netfilter processing

2018-01-02 Thread Pravin Shelar
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

2018-01-02 Thread 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

2018-01-02 Thread William Tu
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

2018-01-02 Thread Gao Zhenyu
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

2018-01-02 Thread 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] [no-slow 4/6] ofproto: Add 'ofproto_uuid' and 'ofp_in_port' to user action cookie.

2018-01-02 Thread Justin Pettit


> 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(>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, 
> >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

2018-01-02 Thread Gregory Rose

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.

2018-01-02 Thread Aaron Conole
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

2018-01-02 Thread Ben Pfaff
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.

2018-01-02 Thread Justin Pettit

> 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

2018-01-02 Thread David Miller
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.

2018-01-02 Thread Ben Pfaff
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.

2018-01-02 Thread Gregory Rose

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 

Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support

2018-01-02 Thread Ben Pfaff
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

2018-01-02 Thread Reformas y estrategias fiscales 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

2018-01-02 Thread David Miller
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

2018-01-02 Thread Aaron Conole
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.

2018-01-02 Thread Ben Pfaff
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

2018-01-02 Thread Wang, Yipeng1
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

2018-01-02 Thread Ben Pfaff
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.

2018-01-02 Thread Ben Pfaff
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 1/6] ofproto-dpif: Add ability to look up an ofproto by UUID.

2018-01-02 Thread Justin Pettit


> 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.

2018-01-02 Thread Gregory Rose

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.

2018-01-02 Thread Ben Pfaff
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(>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, >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

2018-01-02 Thread Matteo Croce
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

2018-01-02 Thread Aaron Conole
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.

2018-01-02 Thread Ben Pfaff
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.

2018-01-02 Thread Ben Pfaff
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.

2018-01-02 Thread Ben Pfaff
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

2018-01-02 Thread 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


[ovs-dev] [PATCH] faq: Correct location of flow.h.

2018-01-02 Thread Ben Pfaff
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

2018-01-02 Thread Ben Pfaff
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

2018-01-02 Thread Ben Pfaff
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

2018-01-02 Thread Ben Pfaff
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

2018-01-02 Thread Bart Van Assche
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

2018-01-02 Thread Julia Lawall


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

2018-01-02 Thread Rybka, MarcinX
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
>  

[ovs-dev] [PATCH v2] tests: Add system-dpdk-testsuite

2018-01-02 Thread 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 | 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
+   

Re: [ovs-dev] [PATCH] netdev-dpdk: fix port addition for ports sharing same PCI id

2018-01-02 Thread Yuanhan Liu
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",
> +   [0], [1], [2],
> +   [3], [4], [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, );
> +for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
> +if (!rte_eth_dev_is_valid_port(i)) {
> +continue;
> +}
> +
> +rte_eth_macaddr_get(i, _mac);
> +if (is_same_ether_addr(, _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, _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, _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([14]);
> +} else {
> +name = xmemdup0(devargs, strcspn(devargs, ","));
> +if (rte_eth_dev_get_port_by_name(name, _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, _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

2018-01-02 Thread Julia Lawall


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

2018-01-02 Thread Bob Peterson
- 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

2018-01-02 Thread Julia Lawall


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

2018-01-02 Thread Bob Peterson
- 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

2018-01-02 Thread nusiddiq
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

2018-01-02 Thread Daniel Alvarez Sanchez
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

2018-01-02 Thread Jan Scheurich
> -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