Re: [ovs-dev] [PATCHv7 1/3] Improved Packet Drop Statistics in OVS

[Anju Thomas]

Thanks Ilya.
I have sent the updated patch.

Regards
Anju

-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com] 
Sent: Thursday, February 07, 2019 7:52 PM
To: Anju Thomas ; Ben Pfaff 
Cc: d...@openvswitch.org
Subject: Re: [PATCHv7 1/3] Improved Packet Drop Statistics in OVS

On 07.02.2019 17:11, Anju Thomas wrote:
> Thanks for comments Ilya. Just one question regarding the steal flag 
> :-
> 
>> +case OVS_ACTION_ATTR_DROP: {
>> +const enum xlate_error *drop_reason = nl_attr_get(a);
>> +if (*drop_reason < XLATE_MAX) {
>> + dp_update_drop_action_counter(*drop_reason, batch->count);
>> +}
>> +dp_packet_delete_batch(batch, steal);
> 
> I'm not sure if we need to honor 'steal' while performing the DROP action, 
> because packets will not be dropped if 'steal == false'.
> 
>  What if there is the drop is within a clone action and there is 
> another output action.In that case , we would not want to delete the batch . 
> Right?

If the drop action is within a clone action, it will drop cloned packets.
i.e. "actions=clone(drop),output:N" should clone packets, drop cloned packets 
and send original packets to port N.

"actions=drop,output:N" should not send anything to N, because all packets 
should be dropped while executing drop action. This list of actions makes no 
practical sense, though.

> 
> Regards
> Anju
> 
> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Wednesday, February 06, 2019 8:48 PM
> To: Anju Thomas ; Ben Pfaff 
> Cc: d...@openvswitch.org
> Subject: Re: [PATCHv7 1/3] Improved Packet Drop Statistics in OVS
> 
> Hi.
> See comments inline.
> 
> Best regards, Ilya Maximets.
> 
> On 06.02.2019 7:01, Anju Thomas wrote:
>>
>> Hi Ben/Ilya,
>>
>> I have addressed the comments in the below patch. Can you tell me if 
>> this is fin> Regards Anju -Original Message-
>> From: Anju Thomas [mailto:anju.tho...@ericsson.com]
>> Sent: Tuesday, January 29, 2019 5:21 PM
>> To: d...@openvswitch.org
>> Cc: Anju Thomas 
>> Subject: [PATCHv7 1/3] Improved Packet Drop Statistics in OVS
>>
>>Currently OVS maintains explicit packet drop/error counters only on port
>>level. Packets that are dropped as part of normal OpenFlow processing are
>>counted in flow stats of “drop” flows or as table misses in table stats.
>>These can only be interpreted by controllers that know the semantics of
>>the configured OpenFlow pipeline. Without that knowledge, it is impossible
>>for an OVS user to obtain e.g. the total number of packets dropped due to
>>OpenFlow rules.
>>
>>Furthermore, there are numerous other reasons for which packets can be
>>dropped by OVS slow path that are not related to the OpenFlow pipeline.
>>The generated datapath flow entries include a drop action to avoid further
>>expensive upcalls to the slow path, but subsequent packets dropped by the
>>datapath are not accounted anywhere.
>>
>>Finally, the datapath itself drops packets in certain error situations.
>>Also, these drops are today not accounted for.
>>
>>This makes it difficult for OVS users to monitor packet drop in an OVS
>>instance and to alert a management system in case of a unexpected increase
>>of such drops. Also OVS trouble-shooters face difficulties in analysing
>>packet drops.
>>
>>With this patch we implement following changes to address the issues
>>mentioned above.
>>
>>1. Identify and account all the silent packet drop scenarios
>>
>>2. Display these drops in ovs-appctl coverage/show
>>
>>A detailed presentation on this was presented at OvS conference 2017 and
>>link for the corresponding presentation is available at:
>>
>>
>> https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-
>> d
>> ata-plane-in-ovs-82280329
>>
>>Co-authored-by: Rohith Basavaraja 
>>Co-authored-by: Keshav Gupta 
>>Signed-off-by: Anju Thomas 
>>Signed-off-by: Rohith Basavaraja 
>>Signed-off-by: Keshav Gupta 
> 
> 
> You still have this strange shift to the right by 3 spaces in commit-message.
> 
>> ---
>>  datapath/linux/compat/include/linux/openvswitch.h |  1 +
>>  lib/dpif-netdev.c | 39 ++-
>>  lib/dpif.c|  7 ++
>>  lib/dpif.h|  3 +
>>  lib/netdev-dpdk.c |  4 ++
>>  lib/odp-execute.c | 81 
>> +++
>>  lib/odp-execute.h |  2 +
>>  lib/odp-util.c| 10 ++-
>>  ofproto/ofproto-dpif-ipfix.c  |  1 +
>>  ofproto/ofproto-dpif-sflow.c  |  1 +
>>  ofproto/ofproto-dpif-xlate.c  | 31 +
>>  ofproto/ofproto-dpif-xlate.h  |  4 ++
>>  ofproto/ofproto-dpif.c   

Re: [ovs-dev] [PATCH v3 1/1] acinclude: Also use LIBS from dpkg pkg-config

[Christian Ehrhardt]

On Fri, Feb 8, 2019 at 4:55 AM Ben Pfaff  wrote:
>
> On Thu, Feb 07, 2019 at 12:00:39PM +0100, Christian Ehrhardt wrote:
> > [...]
> >
> > >  case "$with_dpdk" in
> > >yes)
> > >  DPDK_AUTO_DISCOVER="true"
> > > -PKG_CHECK_MODULES([DPDK], [libdpdk],
> > > -  [DPDK_INCLUDE="$DPDK_CFLAGS"],
> > > -  [DPDK_INCLUDE="-I/usr/local/include/dpdk 
> > > -I/usr/include/dpdk"])
> > > +PKG_CHECK_MODULES_STATIC([DPDK], [libdpdk],
> > > + [DPDK_INCLUDE="$DPDK_CFLAGS", 
> > > DPDK_LIB="$DPDK_LIBS"],
> > > + 
> > > [DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk", 
> > > DPDK_LIB="-ldpdk"])
> > >  ;;
> > >*)
> > >  DPDK_AUTO_DISCOVER="false"
> >
> > While working fine in all my builds (and it seems on travis now) I got
> > reports of the statements above creating a colon in the assignment on
> > some builds - thanks James (on CC now).
> > It was adding a trailing colon to the FLAGS which broke his build.
> >
> > I wanted to ask the more experienced autoconf users if that makes any sense?
>
> I don't see any colons above.  I do see commas.

commas I meant, sorry :-/

> They look weird to me.
> In Bourne shell, spaces are used to separate assignments, not commas.  I
> would remove them.
>
> Other white space, like new-lines, works too.

Thanks Ben,
in V4 sent yesterday I used newlines - so that should be good now I hope.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCHv8 1/3] Improved Packet Drop Statistics in OVS

[Anju Thomas]

Currently OVS maintains explicit packet drop/error counters only on port
level. Packets that are dropped as part of normal OpenFlow processing are
counted in flow stats of “drop” flows or as table misses in table stats.
These can only be interpreted by controllers that know the semantics of
the configured OpenFlow pipeline. Without that knowledge, it is impossible
for an OVS user to obtain e.g. the total number of packets dropped due to
OpenFlow rules.

Furthermore, there are numerous other reasons for which packets can be
dropped by OVS slow path that are not related to the OpenFlow pipeline.
The generated datapath flow entries include a drop action to avoid further
expensive upcalls to the slow path, but subsequent packets dropped by the
datapath are not accounted anywhere.

Finally, the datapath itself drops packets in certain error situations.
Also, these drops are today not accounted for.

This makes it difficult for OVS users to monitor packet drop in an OVS
instance and to alert a management system in case of a unexpected increase
of such drops. Also OVS trouble-shooters face difficulties in analysing
packet drops.

With this patch we implement following changes to address the issues
mentioned above.

1. Identify and account all the silent packet drop scenarios

2. Display these drops in ovs-appctl coverage/show

A detailed presentation on this was presented at OvS conference 2017 and
link for the corresponding presentation is available at:

https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329

Co-authored-by: Rohith Basavaraja 
Co-authored-by: Keshav Gupta 
Signed-off-by: Anju Thomas 
Signed-off-by: Rohith Basavaraja 
Signed-off-by: Keshav Gupta 
---
 datapath/linux/compat/include/linux/openvswitch.h |   1 +
 lib/dpif-netdev.c |  44 -
 lib/dpif.c|   7 +
 lib/dpif.h|   3 +
 lib/odp-execute.c |  81 +-
 lib/odp-util.c|   9 ++
 ofproto/ofproto-dpif-ipfix.c  |   1 +
 ofproto/ofproto-dpif-sflow.c  |   1 +
 ofproto/ofproto-dpif-xlate.c  | 101 +++-
 ofproto/ofproto-dpif-xlate.h  |   3 +
 ofproto/ofproto-dpif.c|   8 +
 ofproto/ofproto-dpif.h|   7 +-
 tests/automake.mk |   3 +-
 tests/dpif-netdev.at  |   8 +
 tests/drop-stats.at   | 189 ++
 tests/ofproto-dpif.at |   2 +-
 tests/testsuite.at|   1 +
 tests/tunnel-push-pop.at  |  28 +++-
 tests/tunnel.at   |  14 +-
 19 files changed, 465 insertions(+), 46 deletions(-)
 create mode 100644 tests/drop-stats.at

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 9b087f1..559b296 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -938,6 +938,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_POP_NSH,  /* No argument. */
OVS_ACTION_ATTR_METER,/* u32 meter number. */
OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*.  */
+   OVS_ACTION_ATTR_DROP, /* Drop action. */
 
 #ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 0f57e3f..9d03de4 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -100,6 +100,17 @@ enum { MAX_METERS = 65536 };/* Maximum number of 
meters. */
 enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
 enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
 
+COVERAGE_DEFINE(datapath_drop_meter);
+COVERAGE_DEFINE(datapath_drop_upcall_error);
+COVERAGE_DEFINE(datapath_drop_lock_error);
+COVERAGE_DEFINE(datapath_drop_userspace_action_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_push_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_pop_error);
+COVERAGE_DEFINE(datapath_drop_recirc_error);
+COVERAGE_DEFINE(datapath_drop_invalid_port);
+COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
+COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
+
 /* Protects against changes to 'dp_netdevs'. */
 static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;
 
@@ -5644,6 +5655,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct 
dp_packet_batch *packets_,
 band->packet_count += 1;
 band->byte_count += dp_packet_size(packet);
 
+COVERAGE_INC(datapath_drop_meter);
 dp_packet_delete(packet);
 } else {
 /* Meter accepts packet. */
@@ -6399,6 +6411,7 @@ dfc_processing(struct dp_netdev_pmd_thread *

Re: [ovs-dev] [PATCH v3 1/1] acinclude: Also use LIBS from dpkg pkg-config

[Ben Pfaff]

On Thu, Feb 07, 2019 at 12:00:39PM +0100, Christian Ehrhardt wrote:
> [...]
> 
> >  case "$with_dpdk" in
> >yes)
> >  DPDK_AUTO_DISCOVER="true"
> > -PKG_CHECK_MODULES([DPDK], [libdpdk],
> > -  [DPDK_INCLUDE="$DPDK_CFLAGS"],
> > -  [DPDK_INCLUDE="-I/usr/local/include/dpdk 
> > -I/usr/include/dpdk"])
> > +PKG_CHECK_MODULES_STATIC([DPDK], [libdpdk],
> > + [DPDK_INCLUDE="$DPDK_CFLAGS", 
> > DPDK_LIB="$DPDK_LIBS"],
> > + [DPDK_INCLUDE="-I/usr/local/include/dpdk 
> > -I/usr/include/dpdk", DPDK_LIB="-ldpdk"])
> >  ;;
> >*)
> >  DPDK_AUTO_DISCOVER="false"
> 
> While working fine in all my builds (and it seems on travis now) I got
> reports of the statements above creating a colon in the assignment on
> some builds - thanks James (on CC now).
> It was adding a trailing colon to the FLAGS which broke his build.
> 
> I wanted to ask the more experienced autoconf users if that makes any sense?

I don't see any colons above.  I do see commas.  They look weird to me.
In Bourne shell, spaces are used to separate assignments, not commas.  I
would remove them.

Other white space, like new-lines, works too.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVN: Setting custom data

[Ben Pfaff]

On Thu, Feb 07, 2019 at 02:24:08PM -0500, Mark Michelson wrote:
> The general problem I have is that I'm developing a feature for OVN. When a
> relevant packet arrives, I want to set some data and have that data
> available the entire time that the packet is being processed. My initial
> idea was to use one of reg0-reg9 for this. However, those register values
> get reset to 0 when the packet changes from the ingress to the egress
> pipeline and when the packet moves from one logical datapath to another.
> 
> My question is, is it possible (or acceptable) for me to use one of
> reg0-reg9 as a special purpose register and not to reset its value to 0
> automatically? Or is there some other method that would be appropriate?

The reason that the registers get reset to 0 on transition between
pipelines is that the transition between pipelines is where packets get
transmitted across tunnels from one hypervisor to another.  The
registers don't get transmitted in the tunnels, so they would naturally
get reset to 0.  We want the logical pipeline to be independent of the
physical pipeline, so we reset all the registers even when the packets
are not transmitted over a tunnel.

This could easily be changed, if we want to assume the use of Geneve,
since we can transmit as many registers as we want as Geneve TLVs.  But
that would rule out the use of STT (and further restrict the use of
VXLAN).  In the past, there's been little willingness to do that.

> Here's a more detailed explanation of the problem. OpenShift is migrating
> from using their own custom use of OVS to using OVN (via ovn-kubernetes).
> One feature they currently offer is for any pod in a namespace to be able to
> send a multicast packet and have it reach all other pods in the same
> namespace. The actual multicast destination address is not important. This
> is on purpose so that separate namespaces can use the same multicast
> destination without the worry of collision. They want to maintain this
> feature when switching to OVN.
> 
> You may recall me talking about doing something like this quite some time
> ago, but the actual method has changed a bit since then.
> 
> My approach for implementing this is to create a new northbound table for
> multicast groups. This table contains a list of logical switch ports that
> represent members of the group. In ovn-northd, we do some magic to ensure
> that if any of these logical switch ports are on separate logical switches,
> then appropriate logical flows get installed so that the multicast packet
> attempts to traverse the logical router(s) that separate the logical
> switches.
> 
> When the packet initially arrives on a logical switch, we can use the
> logical input port, coupled with the multicast destination, to determine the
> multicast group that this packet should be sent to. However, once the packet
> reaches a logical router, there's nothing about the packet that we can use
> to determine which multicast group is the appropriate destination. Same
> thing occurs when the packet arrives in a logical switch from a logical
> router.
> 
> My idea for fixing this is to store the multicast group ID in a
> general-purpose register.  But as stated above, if I set this value in a
> register, the register's value will get reset to 0 when the packet reaches
> the logical router pipeline. Hence my questions from the beginning.

It seems like this could be done by using allocating multicast IP
addresses to the groups, and then mapping from multicast IP addresses to
the OVN multicast group.  But I'm really naive about IP multicast so
maybe this doesn't make sense.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-discuss] Geneve remote_ip as flow for OVN hosts

[Ben Pfaff]

On Thu, Feb 07, 2019 at 08:10:48PM +, Venugopal Iyer wrote:
> > Also,  as I mentioned the changes will mean that the ovn-controller will 
> > need the ovn-central
> > to be updated to the changed version as well (i.e. if someone just installs 
> > ovs and ovn-host
> > s/he can't expect it to be backward compatible with the older version of 
> > ovn-sb). Is that
> > acceptable?
> 
> That's not what we usually want.  The OVN upgrade process expects the
> HVs to be upgraded before the central nodes.  If that breaks things,
> especially in the case where the deployment is only using a single
> interface per HV, it's a problem.
> 
> What would it take to retain compatibility?
> 
>  That is possible; I have committed the changes to the repo[1].  I have 
> tested 
>  ovn-host upgrade with these changes against an OVN central based on 2.9 
>  (without m-vtep changes).
>  Initially, I was planning to bind the port to an encap even if 
> "encap-ip" external_ids
>  is not configured; so when the updated ovn-host comes up, it'll try to 
> bind the
>  port to an encap and the transaction failure caused the port bindings to 
> fail. Implicit
>  binding is not needed, probably not preferred either. So, an updated 
> ovn-host
>  should work with a prev version of SB, however, the OVN central will be
>  updated too, right? Else, if one configures "encap-ip" subsequently on 
> an updated 
>  ovn-host to work with m-vtep, we'll hit the same issue.
>  [1]https://github.com/iyervl/nv-ovs

Of course we want users to upgrade the entire system.  We just need to
make sure that it's possible to upgrade one piece at a time in an order
that ensures that the system isn't broken by a partial upgrade.  The
specified order for OVN is to upgrade the HVs first, then the central
node.  (Although apparently some people want to do it in the other
order, which is currently a problem.)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH v1 3/3] OVN ACL: Allow a user to input ct.label value for an acl

[Ben Pfaff]

That's helpful.

On Thu, Feb 07, 2019 at 09:32:33PM +, Ankur Sharma wrote:
> Hi Ben,
> 
> My bad, should have explained the use case as well.
> 
> Following is the use case I started with:
> a. By associating identifier with connection tracker we can facilitate 
> debugging from connection tracker side.
> b. For example, if a system has multiple OVN ACLs (Hundreds or Thousands), 
> and we see an unexpected FLOW getting established.
>  We can debug it from connection tracker side to see which "Allow" ACL, 
> let the packet passthrough.
> 
> 
> I will update the documentation with this use case as well.
> Please let me know your thoughts.
> 
> Regards,
> Ankur
> 
> -Original Message-
> From: Ben Pfaff  
> Sent: Wednesday, February 6, 2019 6:11 PM
> To: Ankur Sharma 
> Cc: ovs-dev@openvswitch.org
> Subject: Re: [ovs-dev] [RFC PATCH v1 3/3] OVN ACL: Allow a user to input 
> ct.label value for an acl
> 
> I'd like to hear some kind of overall use case here.  Sure, you can use it to 
> identify an OVN ACL, or a security group, or anything else.  How does that 
> contribute to a larger system?  There should be a hint to the reader about 
> how and why to use it.
> 
> On Wed, Feb 06, 2019 at 10:06:46PM +, Ankur Sharma wrote:
> > Reason for using 128 bits:
> > a. Connection tracker has only 2 fields for metadata, ct.mark(32 bits) and 
> > ct.label(128 bits).
> > b. Purpose of this series is to ensure that we use smaller field, i.e  
> > ct.mark for flags and use the bigger field, i.e ct.label for associating 
> > metadata with the ct entry.
> > c. An example of metadata could be a value which maps ct entry to 
> > corresponding OVN ACL or Security Group or both.
> > d. Yes, I agree that 128 could more than sufficient for most of the cases, 
> > but unless we see a use case of dividing ct.label in subfields, i thought 
> > we can leverage on full 128 bits.
> > This keeps implementation simple and  also keeps the interpretation of a 
> > connection tracking entry simple.
> > 
> > Please let me know, if it sounds reasonable.
> > 
> > Thanks
> > 
> > Regards,
> > Ankur
> > 
> > -Original Message-
> > From: Ben Pfaff 
> > Sent: Tuesday, February 5, 2019 1:40 PM
> > To: Ankur Sharma 
> > Cc: ovs-dev@openvswitch.org
> > Subject: Re: [ovs-dev] [RFC PATCH v1 3/3] OVN ACL: Allow a user to 
> > input ct.label value for an acl
> > 
> > On Fri, Jan 11, 2019 at 12:16:39AM +, Ankur Sharma wrote:
> > > This patch allows user to associate a value with acl, which will be 
> > > assigned to ct.label of the corresponding connection tracking entry.
> > > 
> > > This value can be used to map a ct entry with corresponding OVN ACL 
> > > or higher level constructs like security group.
> > > 
> > > signed-off-by: Ankur Sharma 
> > 
> > Thanks for the patch!
> > 
> > Please capitalize the "S" in "Signed-off-by".
> > 
> > This adds a column in ovn-sb.ovsschema, so it should increment the minor 
> > version (the y in x.y.z).
> > 
> > The documentation for the new column explains what it does, but it does not 
> > explain the purpose.  Why would a user set this column?  What are its 
> > effects?
> > 
> > The column is a string, but its value is an integer.  Maybe that is because 
> > OVSDB integer columns are limited to 64 bits, but this value can be 128 
> > bits.  That is a very large space.  What is the reason that so much space 
> > should be dedicated to this identifier?  Even 64 bits is more identifiers 
> > than any deployment will ever use, so there must be some other reason.
> > 
> > Please do not use // comments.
> > 
> > Please document the new option in the ovn-sbctl manpage.
> > 
> > Please add a NEWS item for the new feature.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH v1 3/3] OVN ACL: Allow a user to input ct.label value for an acl

[Ankur Sharma]

Hi Ben,

My bad, should have explained the use case as well.

Following is the use case I started with:
a. By associating identifier with connection tracker we can facilitate 
debugging from connection tracker side.
b. For example, if a system has multiple OVN ACLs (Hundreds or Thousands), and 
we see an unexpected FLOW getting established.
 We can debug it from connection tracker side to see which "Allow" ACL, let 
the packet passthrough.


I will update the documentation with this use case as well.
Please let me know your thoughts.

Regards,
Ankur

-Original Message-
From: Ben Pfaff  
Sent: Wednesday, February 6, 2019 6:11 PM
To: Ankur Sharma 
Cc: ovs-dev@openvswitch.org
Subject: Re: [ovs-dev] [RFC PATCH v1 3/3] OVN ACL: Allow a user to input 
ct.label value for an acl

I'd like to hear some kind of overall use case here.  Sure, you can use it to 
identify an OVN ACL, or a security group, or anything else.  How does that 
contribute to a larger system?  There should be a hint to the reader about how 
and why to use it.

On Wed, Feb 06, 2019 at 10:06:46PM +, Ankur Sharma wrote:
> Reason for using 128 bits:
> a. Connection tracker has only 2 fields for metadata, ct.mark(32 bits) and 
> ct.label(128 bits).
> b. Purpose of this series is to ensure that we use smaller field, i.e  
> ct.mark for flags and use the bigger field, i.e ct.label for associating 
> metadata with the ct entry.
> c. An example of metadata could be a value which maps ct entry to 
> corresponding OVN ACL or Security Group or both.
> d. Yes, I agree that 128 could more than sufficient for most of the cases, 
> but unless we see a use case of dividing ct.label in subfields, i thought we 
> can leverage on full 128 bits.
> This keeps implementation simple and  also keeps the interpretation of a 
> connection tracking entry simple.
> 
> Please let me know, if it sounds reasonable.
> 
> Thanks
> 
> Regards,
> Ankur
> 
> -Original Message-
> From: Ben Pfaff 
> Sent: Tuesday, February 5, 2019 1:40 PM
> To: Ankur Sharma 
> Cc: ovs-dev@openvswitch.org
> Subject: Re: [ovs-dev] [RFC PATCH v1 3/3] OVN ACL: Allow a user to 
> input ct.label value for an acl
> 
> On Fri, Jan 11, 2019 at 12:16:39AM +, Ankur Sharma wrote:
> > This patch allows user to associate a value with acl, which will be 
> > assigned to ct.label of the corresponding connection tracking entry.
> > 
> > This value can be used to map a ct entry with corresponding OVN ACL 
> > or higher level constructs like security group.
> > 
> > signed-off-by: Ankur Sharma 
> 
> Thanks for the patch!
> 
> Please capitalize the "S" in "Signed-off-by".
> 
> This adds a column in ovn-sb.ovsschema, so it should increment the minor 
> version (the y in x.y.z).
> 
> The documentation for the new column explains what it does, but it does not 
> explain the purpose.  Why would a user set this column?  What are its effects?
> 
> The column is a string, but its value is an integer.  Maybe that is because 
> OVSDB integer columns are limited to 64 bits, but this value can be 128 bits. 
>  That is a very large space.  What is the reason that so much space should be 
> dedicated to this identifier?  Even 64 bits is more identifiers than any 
> deployment will ever use, so there must be some other reason.
> 
> Please do not use // comments.
> 
> Please document the new option in the ovn-sbctl manpage.
> 
> Please add a NEWS item for the new feature.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-discuss] Geneve remote_ip as flow for OVN hosts

[Venugopal Iyer]

Sorry, Ben, for the delay, just wanted to test out the compatibility, please 
see response 
inline prefixed with 

Let me know if you have further questions.

thanks,

-venu


From: Ben Pfaff 
Sent: Wednesday, February 6, 2019 10:15 AM
To: Venugopal Iyer
Cc: Guru Shetty; Leonid Grossman; d...@openvswitch.org
Subject: Re: [ovs-discuss] [ovs-dev] Geneve remote_ip as flow for OVN hosts

It looks like we fell off ovs-dev somehow.  I've added it back.

On Wed, Feb 06, 2019 at 05:50:39PM +, Venugopal Iyer wrote:
> Thanks, Ben!
>
> I missed and {} for an if; have fixed it; thanks for catching it!
>
> As for the TODOS:
>  - ovn/controller/chassis.c was something I just wanted to check, but not 
> needed, so have
> removed it.
>
> - ovn/controller/physical.c is not relevant, so removed it.
>
> - ovn/utitilities/ovnn-sbctl.c was to add a cmd to bind a encap to  a port. I 
> am not
>   planning to do that now since it will work even if we don't explicitly bind 
> (it will select
>   the preferred encap to the chassis it is bound to). If we see a need to 
> provide a
>   lsp-encap-bind (or somesuch) we can add it later. Let me know if that works.

OK, thanks.

> BTW, I wanted to check if it is possible to rename "ovn-chassis-id" to 
> "ovn-tunnel-id"
> since that is more appropriate. Not sure if that'll cause any grief to 
> existing tools/scripts
> that specifically look for ovn-chassis-id.

Good names are important but we don't want to change names if it causes
compatibility problems.  However...

> Also,  as I mentioned the changes will mean that the ovn-controller will need 
> the ovn-central
> to be updated to the changed version as well (i.e. if someone just installs 
> ovs and ovn-host
> s/he can't expect it to be backward compatible with the older version of 
> ovn-sb). Is that
> acceptable?

That's not what we usually want.  The OVN upgrade process expects the
HVs to be upgraded before the central nodes.  If that breaks things,
especially in the case where the deployment is only using a single
interface per HV, it's a problem.

What would it take to retain compatibility?

 That is possible; I have committed the changes to the repo[1].  I have 
tested 
 ovn-host upgrade with these changes against an OVN central based on 2.9 
 (without m-vtep changes).
 Initially, I was planning to bind the port to an encap even if "encap-ip" 
external_ids
 is not configured; so when the updated ovn-host comes up, it'll try to 
bind the
 port to an encap and the transaction failure caused the port bindings to 
fail. Implicit
 binding is not needed, probably not preferred either. So, an updated 
ovn-host
 should work with a prev version of SB, however, the OVN central will be
 updated too, right? Else, if one configures "encap-ip" subsequently on an 
updated 
 ovn-host to work with m-vtep, we'll hit the same issue.
 [1]https://github.com/iyervl/nv-ovs

> I haven't updated the test suite (to add a couple of tests for m-vtep); i.e 
> to make sure the
> port binding is done correctly. What is the process to proceed with 
> integrating the changes;
> is it required that the tests be committed at the same time the changes are?
>
> I have  pushed the changes to https://github.com/iyervl/nv-ovs after cleaning 
> up the
> comments and adding the missing {}.
>
> Once the changes look fine, I'll squash the commits and also add the detailed 
> commit
> and let you know.
>
> thanks for your time and help!
>
> -venu
>
> 
> From: Ben Pfaff 
> Sent: Tuesday, February 5, 2019 12:55 PM
> To: Venugopal Iyer
> Cc: Guru Shetty; Leonid Grossman
> Subject: Re: [ovs-discuss] [ovs-dev] Geneve remote_ip as flow for OVN hosts
>
> Thanks.
>
> I looked over this again now.  There are multiple TODO items still in
> it.  Do you intend to fix them?
>
> The version in ovn-sb.ovsschema should be updated, probably to 2.1.0.
>
> I noticed one missing {} around an 'if' block.
>
> I'd appreciate a detailed commit message for the series (which I imagine
> will ultimately be squashed into one patch for commit).
>
> Thanks again!

---
This email message is for the sole use of the intended recipient(s) and may 
contain
confidential information.  Any unauthorized review, use, disclosure or 
distribution
is prohibited.  If you are not the intended recipient, please contact the 
sender by
reply email and destroy all copies of the original message.
---
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] OVN: Setting custom data

[Mark Michelson]

Hi everyone,

The general problem I have is that I'm developing a feature for OVN. 
When a relevant packet arrives, I want to set some data and have that 
data available the entire time that the packet is being processed. My 
initial idea was to use one of reg0-reg9 for this. However, those 
register values get reset to 0 when the packet changes from the ingress 
to the egress pipeline and when the packet moves from one logical 
datapath to another.


My question is, is it possible (or acceptable) for me to use one of 
reg0-reg9 as a special purpose register and not to reset its value to 0 
automatically? Or is there some other method that would be appropriate?




Here's a more detailed explanation of the problem. OpenShift is 
migrating from using their own custom use of OVS to using OVN (via 
ovn-kubernetes). One feature they currently offer is for any pod in a 
namespace to be able to send a multicast packet and have it reach all 
other pods in the same namespace. The actual multicast destination 
address is not important. This is on purpose so that separate namespaces 
can use the same multicast destination without the worry of collision. 
They want to maintain this feature when switching to OVN.


You may recall me talking about doing something like this quite some 
time ago, but the actual method has changed a bit since then.


My approach for implementing this is to create a new northbound table 
for multicast groups. This table contains a list of logical switch ports 
that represent members of the group. In ovn-northd, we do some magic to 
ensure that if any of these logical switch ports are on separate logical 
switches, then appropriate logical flows get installed so that the 
multicast packet attempts to traverse the logical router(s) that 
separate the logical switches.


When the packet initially arrives on a logical switch, we can use the 
logical input port, coupled with the multicast destination, to determine 
the multicast group that this packet should be sent to. However, once 
the packet reaches a logical router, there's nothing about the packet 
that we can use to determine which multicast group is the appropriate 
destination. Same thing occurs when the packet arrives in a logical 
switch from a logical router.


My idea for fixing this is to store the multicast group ID in a 
general-purpose register.  But as stated above, if I set this value in a 
register, the register's value will get reset to 0 when the packet 
reaches the logical router pipeline. Hence my questions from the beginning.


Thanks everyone,
Mark Michelson
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Hi

[KShan]

Greetings,

Our company have a business proposal to share with you, please kindly
respond back if you need more information regarding the proposal.

Sincerely,

Kim Shandong
h...@sanshime.com

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Openvswitch| RSA Conference 2019 -Leads

[Stacy Brown]

Hi,

 

Attendees-list of RSA Conference 2019 (March 4th to 8th  2019) is
available-for-purchase with contact name, company name, job title, business
email-address, phone number and mailing address etc...

 

Let me know if you'd be interested in hearing more about it.

 

Thanks,
Stacy Brown
Marketing Executive

PS : If you have any other list requirements that we can service may be
based on the target job titles, industries, countries, Event attendees list
, Technology install base, you sell into.

If you don't want to include yourself in our mailing list, please reply back
"No thanks" in a subject line.

 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] odp-util: Do not rewrite fields with the same values as matched

[0-day Robot]

Bleep bloop.  Greetings Eli Britstein, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Inappropriate bracing around statement
#57 FILE: lib/odp-util.c:7189:
if (size == 0)

ERROR: Inappropriate bracing around statement
#60 FILE: lib/odp-util.c:7192:
if (memcmp(pkey0, pkey1, size) == 0)

ERROR: Inappropriate bracing around statement
#62 FILE: lib/odp-util.c:7194:
else

WARNING: Line is 92 characters long (recommended limit is 79)
#85 FILE: lib/odp-util.c:7250:
#define __OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_ethernet, name), 
sizeof(type)},

WARNING: Line is 88 characters long (recommended limit is 79)
#108 FILE: lib/odp-util.c:7393:
#define __OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_ipv4, name), 
sizeof(type)},

WARNING: Line is 88 characters long (recommended limit is 79)
#130 FILE: lib/odp-util.c:7454:
#define __OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_ipv6, name), 
sizeof(type)},

WARNING: Line is 87 characters long (recommended limit is 79)
#152 FILE: lib/odp-util.c:7514:
#define __OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_arp, name), 
sizeof(type)},

WARNING: Line is 88 characters long (recommended limit is 79)
#173 FILE: lib/odp-util.c:7555:
#define __OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_icmp, name), 
sizeof(type)},

WARNING: Line is 86 characters long (recommended limit is 79)
#196 FILE: lib/odp-util.c:7626:
#define __OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_nd, name), 
sizeof(type)},

WARNING: Line is 87 characters long (recommended limit is 79)
#217 FILE: lib/odp-util.c:7877:
#define __OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_tcp, name), 
sizeof(type)},

Lines checked: 353, Warnings: 7, Errors: 3


build:
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include 
-I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/odp-execute.lo -MD -MP -MF 
lib/.deps/odp-execute.Tpo -c lib/odp-execute.c -o lib/odp-execute.o
depbase=`echo lib/odp-util.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. 
   -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall 
-Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security 
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align 
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -MT lib/odp-util.lo -MD -MP -MF $depbase.Tpo -c -o lib/odp-util.lo 
lib/odp-util.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include 
-I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/odp-util.lo -MD -MP -MF lib/.deps/odp-util.Tpo 
-c lib/odp-util.c -o lib/odp-util.o
lib/odp-util.c: In function 'commit_set_nd_extensions_action':
lib/odp-util.c:7659:16: error: passing argument 7 of 'commit' from incompatible 
pointer type [-Werror]
&base, &mask, sizeof key, odp_actions)) {
^
lib/odp-util.c:7202:1: note: expected 'struct ovs_key_field_properties *' but 
argument is of type 'struct ofpbuf *'
 commit(enum ovs_key_attr attr, bool use_masked_set,
 ^
lib/odp-util.c:7659:16: error: too few arguments to function 'commit'
&base, &mask, sizeof key, odp_actions)) {
^
lib/odp-util.c:7202:1: note: declared here
 commit(enum ovs_key_attr attr, bool use_masked_set,
 ^
cc1: all warnings being treated as errors
make[2]: *** [lib/odp-util.lo] Error 1
make[2]: Leaving directory 
`/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory 
`/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make: *** [all] Error 2


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Makefiles: Generate datapath ovs key fields macros

[0-day Robot]

Bleep bloop.  Greetings Eli Britstein, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 83 characters long (recommended limit is 79)
#69 FILE: build-aux/extract-odp-netlink-xmacros-h:34:
echo "/* Generated automatically from  -- do not modify! 
*/"

Lines checked: 117, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC 1/3] dpdk: Unify passing of DPDK EAL arguments.

[Flavio Leitner]

On Wed, Feb 06, 2019 at 11:33:04AM +0300, Ilya Maximets wrote:
> On 05.02.2019 23:19, Flavio Leitner wrote:
> > On Tue, Feb 05, 2019 at 06:43:08PM +0300, Ilya Maximets wrote:
> >> Hi Flavio.
> >> Thanks for taking a look.
> >>
> >> On 05.02.2019 15:38, Flavio Leitner wrote:
> >>>
> >>> Hi Ilya,
> >>>
> >>> Thanks for the patch and I think we knew about that pain when we
> >>> exposed the very first parameter. I still remember Aaron struggling
> >>> to find the best path forward. Time flies and here we are.
> >>>
> >>> The problem is that this change is not friendly from the community
> >>> perspective to its users. That is like an exposed API which we should
> >>> avoid break as much as possible.
> >>>
> >>> For instance, there are users (OpenStack) with support life cycle of
> >>> 5+ years that cannot keep up with this kind of change but they expect
> >>> to be able to update to newer OVS.
> >>
> >> Sure, there are users that wants stable API that will never change.
> >> But this is really impossible in practice. I'm working with OpenStack
> >> too and will have to fixup deployment tools with these changes. BTW, from
> >> the whole OpenStack only few deployment projects (like TripleO) will need
> >> to make changes and these changes are not very big.
> > 
> > That's only part of the work. There will be work on QA, documentation
> > and even migration path from one to another. And we can't change the
> > past for existing deployments.
> 
> Sure. But incompatible API changes are almost unavoidable for a young
> projects that wants to be better.

My point here is that the associated impact and cost of this change is
far from trivial.

> > I don't think we should remove the docs if the parameters are there as
> > a first step. I mean, assume an existing deployment, there is a parameter
> > which might be in use but there is no documentation available. That
> > doesn't sound like a good user experience to me.
> 
> Maybe we could save a man pages while removing the guides. There is no much
> information in Documentation/intro/install/dpdk.rst anyway.

Agreed.

> > On another hand, you could introduce the new interface and update the
> > docs to recommend using the new one because the old one will be removed
> > in the future. Warning messages come next, and then finally its removal.
> 
> I'd prefer to have warning messages to be there right from the start to
> push users to migrate to the new interfaces as early as possible.

If it's a WARN level message then I can tell you it will break
environments over here. If that is a INFO level message, then it might
be okay.  Though I still think there could be a period of time where
both could co-exist before we announce/warn about the deprecation of
the current interface.

> How about this:
> 
> First stage (apply now, release in 2.12):
>   - Introduce new interface 'dpdk-options'.
>   - Rewrite installation guide with new interface fully removing the old one.
>   - Add new interface to man pages (vswitch.xml) and mark all the old options
> as deprecated by adding something like: "Deprecated. 'dpdk-options' should
> be used instead. Will be ignored in the future."
>   - Add a runtime deprecation warning if old interface is in use.

^^^ this is bad as an initial step as explained above.

>   - Ignore values of old knobs if 'dpdk-options' provided.

Sounds like a good transition barrier.


> Second stage (release in 2.13 or 2.14, could wait longer if required):
>   - Remove old interfaces wile keeping the warnings. (i.e. values always 
> ignored)
>   - Remove old knobs from the man pages.
> 
> Third stage (optional):
>   - Remove warnings.
> 
> So the main difference from the current patches is delaying removal of the man
> pages to the second stage.
[...] 
> >> We're keeping few sane defaults like providing default lcore and setting 
> >> the
> >> socket-limit if needed. And we'll try to do that in the future. The thing
> >> this patch tries to eliminate is the dependency tracking between different
> >> EAL arguments, i.e. duplicating the work with rte_eal_init() and having too
> >> many configuration knobs with similar meanings what does not work at the
> >> same time.
> >>
> >> Right now there are no critical arguments that user must provide.
> >> So, IMHO, having special configs for them is really unnecessary.
> > 
> > Do you think the defaults work for the majority of DPDK users?
> 
> My personal experience is opposite. Most of my deployments and deployments 
> that
> I had to work with had massive 'dpdk-extra' arguments. These are mostly memory
> tuning options, pci whitelists and various virtual devices. And these 
> 'dpdk-extra'
> strings only grows over time with '--socket-limit', '--legacy-mem',
> '--single-file-segments', '--in-memory', '--file-prefix' and so on.

That is indeed a problem, but moving to 'dpdk-options' will not solve it.

> Defaults are OK if your application is the only application you want to see on
> a system. Otherwise, you need to

[ovs-dev] [PATCH V3 2/2] odp-util: Do not rewrite fields with the same values as matched

[Eli Britstein]

To improve performance and avoid wasting resources for HW offloaded
flows, do not rewrite fields that are matched with the same value.

Signed-off-by: Eli Britstein 
Reviewed-by: Roi Dayan 
---
 lib/odp-util.c| 110 +-
 tests/mpls-xlate.at   |   2 +-
 tests/ofproto-dpif.at |  14 +++
 3 files changed, 108 insertions(+), 18 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 778c00ee8..e7a389c8c 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -43,6 +43,7 @@
 #include "uuid.h"
 #include "openvswitch/vlog.h"
 #include "openvswitch/match.h"
+#include "odp-netlink-xmacros.h"
 
 VLOG_DEFINE_THIS_MODULE(odp_util);
 
@@ -7093,12 +7094,49 @@ commit_odp_tunnel_action(const struct flow *flow, 
struct flow *base,
 }
 }
 
+struct ovs_key_field_properties {
+int offset;
+int size;
+};
+
+/* Compare the keys similary to memcmp, but each field separately.
+ * The offsets and sizes of each field is provided by field_properties
+ * argument.
+ * For fields with the same value, zero out their mask part in order not to
+ * rewrite them as it's unnecessary */
+static bool
+keycmp_mask(const void *key0, const void *key1,
+struct ovs_key_field_properties *field_properties, void *mask)
+{
+int field;
+bool differ = false;
+
+for (field = 0 ; ; field++) {
+int size = field_properties[field].size;
+int offset = field_properties[field].offset;
+char *pkey0 = ((char *)key0) + offset;
+char *pkey1 = ((char *)key1) + offset;
+char *pmask = ((char *)mask) + offset;
+
+if (size == 0)
+break;
+
+if (memcmp(pkey0, pkey1, size) == 0)
+memset(pmask, 0, size);
+else
+differ = true;
+}
+
+return differ;
+}
+
 static bool
 commit(enum ovs_key_attr attr, bool use_masked_set,
const void *key, void *base, void *mask, size_t size,
+   struct ovs_key_field_properties *field_properties,
struct ofpbuf *odp_actions)
 {
-if (memcmp(key, base, size)) {
+if (keycmp_mask(key, base, field_properties, mask)) {
 bool fully_masked = odp_mask_is_exact(attr, mask, size);
 
 if (use_masked_set && !fully_masked) {
@@ -7140,6 +7178,12 @@ commit_set_ether_action(const struct flow *flow, struct 
flow *base_flow,
 bool use_masked)
 {
 struct ovs_key_ethernet key, base, mask;
+struct ovs_key_field_properties ovs_key_ethernet_field_properties[] = {
+#define __OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_ethernet, name), 
sizeof(type)},
+__OVS_KEY_ETHERNET_FIELDS
+{0, 0}
+#undef __OVS_KEY_FIELD
+};
 
 if (flow->packet_type != htonl(PT_ETH)) {
 return;
@@ -7150,7 +7194,8 @@ commit_set_ether_action(const struct flow *flow, struct 
flow *base_flow,
 get_ethernet_key(&wc->masks, &mask);
 
 if (commit(OVS_KEY_ATTR_ETHERNET, use_masked,
-   &key, &base, &mask, sizeof key, odp_actions)) {
+   &key, &base, &mask, sizeof key,
+   ovs_key_ethernet_field_properties, odp_actions)) {
 put_ethernet_key(&base, base_flow);
 put_ethernet_key(&mask, &wc->masks);
 }
@@ -7276,6 +7321,12 @@ commit_set_ipv4_action(const struct flow *flow, struct 
flow *base_flow,
bool use_masked)
 {
 struct ovs_key_ipv4 key, mask, base;
+struct ovs_key_field_properties ovs_key_ipv4_field_properties[] = {
+#define __OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_ipv4, name), 
sizeof(type)},
+__OVS_KEY_IPV4_FIELDS
+{0, 0}
+#undef __OVS_KEY_FIELD
+};
 
 /* Check that nw_proto and nw_frag remain unchanged. */
 ovs_assert(flow->nw_proto == base_flow->nw_proto &&
@@ -7293,7 +7344,7 @@ commit_set_ipv4_action(const struct flow *flow, struct 
flow *base_flow,
 }
 
 if (commit(OVS_KEY_ATTR_IPV4, use_masked, &key, &base, &mask, sizeof key,
-   odp_actions)) {
+   ovs_key_ipv4_field_properties, odp_actions)) {
 put_ipv4_key(&base, base_flow, false);
 if (mask.ipv4_proto != 0) { /* Mask was changed by commit(). */
 put_ipv4_key(&mask, &wc->masks, true);
@@ -7331,6 +7382,12 @@ commit_set_ipv6_action(const struct flow *flow, struct 
flow *base_flow,
bool use_masked)
 {
 struct ovs_key_ipv6 key, mask, base;
+struct ovs_key_field_properties ovs_key_ipv6_field_properties[] = {
+#define __OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_ipv6, name), 
sizeof(type)},
+__OVS_KEY_IPV6_FIELDS
+{0, 0}
+#undef __OVS_KEY_FIELD
+};
 
 /* Check that nw_proto and nw_frag remain unchanged. */
 ovs_assert(flow->nw_proto == base_flow->nw_proto &&
@@ -7349,7 +7406,7 @@ commit_set_ipv6_action(const struct flow *flow, struct 
flow *base_flow,
 }
 
 if (commit(OVS_KEY_ATTR_IPV6, use_masked, &key, &base, &mask, sizeof key,
-   odp_actions)) {
+   

[ovs-dev] [PATCH V3 1/2] Makefiles: Generate datapath ovs key fields macros

[Eli Britstein]

Generate datapath ovs key fields macros as a pre-step of retrieving
field information, with no functional change.

Signed-off-by: Eli Britstein 
Reviewed-by: Roi Dayan 
---
 .gitignore  |  1 +
 build-aux/extract-odp-netlink-xmacros-h | 55 +
 include/automake.mk | 11 +--
 3 files changed, 64 insertions(+), 3 deletions(-)
 create mode 100755 build-aux/extract-odp-netlink-xmacros-h

diff --git a/.gitignore b/.gitignore
index 60e7818c3..9b9ea91c0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -68,6 +68,7 @@ cscope.*
 tags
 _debian
 odp-netlink.h
+odp-netlink-xmacros.h
 OvsDpInterface.h
 /.vagrant/
 testsuite.tmp.orig
diff --git a/build-aux/extract-odp-netlink-xmacros-h 
b/build-aux/extract-odp-netlink-xmacros-h
new file mode 100755
index 0..b91ba3a9a
--- /dev/null
+++ b/build-aux/extract-odp-netlink-xmacros-h
@@ -0,0 +1,55 @@
+#!/bin/bash
+
+hfile=$1
+
+function generate_fields_macros {
+local struct_name=$1
+
+# line_start is the line number where the definition of the struct begin
+# line_end is the line number where the definition of the struct ends
+line_start=`grep -nw $struct_name $hfile | grep { | cut -d ":" -f1`
+num_lines=`tail -n +${line_start} $hfile | grep -n -m1 } | cut -d ":" -f1`
+line_end=$((line_start+num_lines-1))
+
+
+STRUCT=`echo $struct_name | tr [a-z] [A-Z]`
+echo "#define __${STRUCT}_FIELDS \\"
+# for all the field lines, including the terminating }, remove ";" and
+# replace with a macro.
+# 3 awk fields are for struct  
+# 2 awk fields are for  
+# else - terminating macro
+awk -F ";" "NR>${line_start} && NR<=${line_end}"' {print $1}' $hfile |
+awk '{
+if (NF == 3)
+print "__OVS_KEY_FIELD(" $1,$2", " $3 ") \\";
+else if (NF == 2)
+print "__OVS_KEY_FIELD(" $1", " $2 ") \\";
+else
+print "__OVS_KEY_FIELDS_END";
+}'
+}
+
+
+echo "/* Generated automatically from  -- do not 
modify! */"
+echo
+echo
+echo "#ifndef ODP_NETLINK_MACROS_H"
+echo "#define ODP_NETLINK_MACROS_H"
+echo
+echo "#define __OVS_KEY_FIELDS_END"
+echo
+
+generate_fields_macros "ovs_key_ethernet"
+generate_fields_macros "ovs_key_ipv4"
+generate_fields_macros "ovs_key_ipv6"
+generate_fields_macros "ovs_key_tcp"
+generate_fields_macros "ovs_key_udp"
+generate_fields_macros "ovs_key_sctp"
+generate_fields_macros "ovs_key_icmp"
+generate_fields_macros "ovs_key_icmpv6"
+generate_fields_macros "ovs_key_arp"
+generate_fields_macros "ovs_key_nd"
+
+echo
+echo "#endif"
diff --git a/include/automake.mk b/include/automake.mk
index 3f3ed1ccd..b54e77683 100644
--- a/include/automake.mk
+++ b/include/automake.mk
@@ -1,10 +1,15 @@
-BUILT_SOURCES += include/odp-netlink.h
+BUILT_SOURCES += include/odp-netlink.h include/odp-netlink-xmacros.h
 
 include/odp-netlink.h: datapath/linux/compat/include/linux/openvswitch.h \
build-aux/extract-odp-netlink-h
$(AM_V_GEN)sed -f $(srcdir)/build-aux/extract-odp-netlink-h < $< > $@
-EXTRA_DIST += build-aux/extract-odp-netlink-h
-CLEANFILES += include/odp-netlink.h
+
+include/odp-netlink-xmacros.h: include/odp-netlink.h \
+  build-aux/extract-odp-netlink-xmacros-h
+   $(AM_V_GEN)bash -f $(srcdir)/build-aux/extract-odp-netlink-xmacros-h $< 
> $@
+
+EXTRA_DIST += build-aux/extract-odp-netlink-h 
build-aux/extract-odp-netlink-xmacros-h
+CLEANFILES += include/odp-netlink.h include/odp-netlink-xmacros.h
 
 include include/ovn/automake.mk
 include include/openflow/automake.mk
-- 
2.14.5

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH V3 0/2] Do not rewrite fields with the same values as matched

[Eli Britstein]

Hi

This patch set avoids unnecessary rewrite actions to fields with the
same values as matched on.

Patch 1 is a pre-step of generating ovs key fields macros

Patch 2 avoids the unnecessary rewrites and adapts the tests accordingly


Travis link:
https://travis-ci.org/roidayan/ovs/builds/490090058
Appvoyer link:
https://ci.appveyor.com/project/roidayan/ovs/builds/22198603

Thanks,
Eli


Eli Britstein (2):
  Makefiles: Generate datapath ovs key fields macros
  odp-util: Do not rewrite fields with the same values as matched

 .gitignore  |   1 +
 build-aux/extract-odp-netlink-xmacros-h |  55 
 include/automake.mk |  11 +++-
 lib/odp-util.c  | 110 +---
 tests/mpls-xlate.at |   2 +-
 tests/ofproto-dpif.at   |  14 ++--
 6 files changed, 172 insertions(+), 21 deletions(-)
 create mode 100755 build-aux/extract-odp-netlink-xmacros-h

-- 
2.14.5

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] travis: Fix building datapath instead of userspace with DPDK_SHARED.

[Aaron Conole]

Ilya Maximets  writes:

> Current script does not check build of OVS with DPDK.
> It builds datapath instead:
>   https://travis-ci.org/openvswitch/ovs/jobs/489520695
>
> CC: Ian Stokes 
> Fixes: edfe8d263d2e ("travis: Add dpdk shared library build.")
> Signed-off-by: Ilya Maximets 
> ---

Neat catch!

Acked-by: Aaron Conole 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] acinclude: Use NUMA_AWARE_HUGEPAGES too for libnuma check.

[Ilya Maximets]

On 07.02.2019 19:18, Ian Stokes wrote:
> On 2/7/2019 1:00 PM, Ilya Maximets wrote:
>> This fixes build with NUMA_AWARE_HUGEPAGES enabled and VHOST_NUMA
>> disabled. This should not be a usual case. But it's possible to
>> configure DPDK this way.
>>
> 
> Out of interest, with RTE_EAL_NUMA_AWARE_HUGEPAGES defined but not 
> RTE_LIBRTE_VHOST_NUMA, does vhost numa still work as expected?

If RTE_LIBRTE_VHOST_NUMA disabled, all the numa related code from
librte_vhost will be compiled out. So, rte_vhost_get_numa_node()
will always return -1. i.e. we will not be able to reallocate
memory pools according to numa node where VM started.

At the same time eal_memory and eal_memalloc modules will work fine
allocating hugepages and memory from the requested numa nodes.

> 
> Ian
>> Fixes: 5e925ccc2a6f ("netdev-dpdk: DPDK v17.11 upgrade")
>>
>> Signed-off-by: Ilya Maximets 
>> ---
>>   acinclude.m4 | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/acinclude.m4 b/acinclude.m4
>> index c51af246a..e2af4ee16 100644
>> --- a/acinclude.m4
>> +++ b/acinclude.m4
>> @@ -254,7 +254,7 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>>     AC_LANG_PROGRAM(
>>   [
>>     #include 
>> -#if RTE_LIBRTE_VHOST_NUMA
>> +#if defined(RTE_LIBRTE_VHOST_NUMA) || defined(RTE_EAL_NUMA_AWARE_HUGEPAGES)
>>   #error
>>   #endif
>>   ], [])
>>
> 
> 
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] acinclude: Use NUMA_AWARE_HUGEPAGES too for libnuma check.

[Ian Stokes]

On 2/7/2019 1:00 PM, Ilya Maximets wrote:

This fixes build with NUMA_AWARE_HUGEPAGES enabled and VHOST_NUMA
disabled. This should not be a usual case. But it's possible to
configure DPDK this way.



Out of interest, with RTE_EAL_NUMA_AWARE_HUGEPAGES defined but not 
RTE_LIBRTE_VHOST_NUMA, does vhost numa still work as expected?


Ian

Fixes: 5e925ccc2a6f ("netdev-dpdk: DPDK v17.11 upgrade")

Signed-off-by: Ilya Maximets 
---
  acinclude.m4 | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/acinclude.m4 b/acinclude.m4
index c51af246a..e2af4ee16 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -254,7 +254,7 @@ AC_DEFUN([OVS_CHECK_DPDK], [
AC_LANG_PROGRAM(
  [
#include 
-#if RTE_LIBRTE_VHOST_NUMA
+#if defined(RTE_LIBRTE_VHOST_NUMA) || defined(RTE_EAL_NUMA_AWARE_HUGEPAGES)
  #error
  #endif
  ], [])



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Reply for more details

[Miss C L Young]

United Nations Headquarters New York
Address: New York, NY 10017, USA
Phone: +1 (646) 547-2917
Your payment File Number: (wp195uno)

Attention:

My name is Mrs. Carman Lapointe Young Former United Nation Under
Secretary General this is to officially inform you that your name is
among the people that could not be able to receive there compensation
payment of (USD2.5M) Two Million Five Hundred Thousand United State
Dollars during my Time in the office and this payment was approve by
the United Nations Organization. but after the 72nd UN General
Assembly Meeting which was held on Tuesday 19TH 09/2017 with Mr.
Donald J. Trump the President of United State of America and new UN
sectary General Mr. Antonio Guterres and other world leaders they
reach agreement to pay out all the beneficiaries that could not be
able to receive there compensation payment during my time in the
office

Note. United Bank for Africa (UBA) has been appointed by the United
Nations Organizations stake holders to effect the payment to all the
Beneficiaries including you without any further delay so kindly
contact the bank now so that they will direct and instruct you on what
to do and how to receive your payment from them before it will be too
late because this payment will be closing any time soon.

Bank Name:  United Bank for Africa (UBA)
Address: 57 Marina Rd, Lagos Island, Lagos Nigeria
Contact Person: Mr. Tony O. Elumelu
Position: MD/CEO of UBA Plc
E-mail: mr.elumely.tony...@gmail.com
E-mail: mr.elumely.tony...@gmail.com
Phone: +23470-6126-8585

I will advice you to Feel free and contact them and follow there
instructions so that they will direct you on what to do and how to
receive your payment without any hitch

Mrs. Carman Lapointe
Former United Nations Under-Secretary
General for Internal Oversight
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] acinclude: Drop DPDK_EXTRA_LIB variable.

[Ian Stokes]

On 2/7/2019 12:20 PM, Ilya Maximets wrote:

AC_SEARCH_LIBS enables the libraries itself:

   checking for library containing get_mempolicy... -lnuma
   checking for library containing pcap_dump... -lpcap

So, they are available in LIBS. No need to add them twice.

Also, DPDK_EXTRA_LIB doesn't even work, because each check overwrites
the variable instead of appending the new library. It was first time
misused while making libnuma optional and copy-pasted to several places
after that.



Good catch, wasn't aware that the libs were added automatically by 
search, good to know.


I've applied to master and backported to 2.7

Ian
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] travis: Fix building datapath instead of userspace with DPDK_SHARED.

[Ilya Maximets]

Current script does not check build of OVS with DPDK.
It builds datapath instead:
  https://travis-ci.org/openvswitch/ovs/jobs/489520695

CC: Ian Stokes 
Fixes: edfe8d263d2e ("travis: Add dpdk shared library build.")
Signed-off-by: Ilya Maximets 
---
 .travis/linux-build.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index 9d177aa1b..8c931ebc5 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -43,7 +43,7 @@ function install_kernel()
 fi
 
 KERNELSRC=$(pwd)
-if [ ! "$DPDK" ]; then
+if [ ! "$DPDK" ] && [ ! "$DPDK_SHARED" ]; then
 EXTRA_OPTS="--with-linux=$(pwd)"
 fi
 echo "Installed kernel source in $(pwd)"
@@ -106,7 +106,8 @@ configure_ovs $EXTRA_OPTS $*
 make selinux-policy
 
 # Only build datapath if we are testing kernel w/o running testsuite
-if [ "$KERNEL" ] && [ ! "$TESTSUITE" ] && [ ! "$DPDK" ]; then
+if [ "$KERNEL" ] && [ ! "$TESTSUITE" ] && \
+   [ ! "$DPDK" ] && [ ! "$DPDK_SHARED" ]; then
 cd datapath
 fi
 
-- 
2.17.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] HI

[Mathew Dudash]

Hallo

Ich habe diesen Brief schon früher an Sie geschickt, aber ich bin mir nicht 
sicher, ob Sie ihn bekommen haben, ich habe nichts von Ihnen gehört, und 
deshalb wiederhole ich ihn. Ich bin Rechtsanwalt Mathew Hamza. Ich wohne in 
Togo. Ich kontaktiere Sie in Verbindung mit dem Tod meines verstorbenen 
Klienten, der aus Ihrem Land stammt und den gleichen Namen bei sich trägt. Er 
ist mit seinem gesamten Familienmitglied durch einen Autounfall ums Leben 
gekommen. Hinterlassen der Summe von 9.500.000,00 Mio. USD die Bank von Togo. 
Die Bank hat mich gebeten, ihnen einen Angehörigen vorzustellen, aber nach 
einem erfolglosen Versuch, einen Angehörigen meines verstorbenen Klienten zu 
finden, beschloss ich, Sie zu kontaktieren, um als Angehöriger zu stehen und 
dieses Erbschaftsgeld in Anspruch zu nehmen. Wenden Sie sich daher an mich, um 
Ihnen weitere Details und Informationen zur Transaktion zu geben. Ich brauche 
Ihre dringende Antwort


Rechtsanwalt Mathew Hamza
Lomé - Togo
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCHv7 1/3] Improved Packet Drop Statistics in OVS

[Ilya Maximets]

On 07.02.2019 17:11, Anju Thomas wrote:
> Thanks for comments Ilya. Just one question regarding the steal flag :-
> 
>> +case OVS_ACTION_ATTR_DROP: {
>> +const enum xlate_error *drop_reason = nl_attr_get(a);
>> +if (*drop_reason < XLATE_MAX) {
>> + dp_update_drop_action_counter(*drop_reason, batch->count);
>> +}
>> +dp_packet_delete_batch(batch, steal);
> 
> I'm not sure if we need to honor 'steal' while performing the DROP action, 
> because packets will not be dropped if 'steal == false'.
> 
>  What if there is the drop is within a clone action and there is 
> another output action.In that case , we would not want to delete the batch . 
> Right?

If the drop action is within a clone action, it will drop cloned packets.
i.e. "actions=clone(drop),output:N" should clone packets, drop cloned packets
and send original packets to port N.

"actions=drop,output:N" should not send anything to N, because all packets
should be dropped while executing drop action. This list of actions makes
no practical sense, though.

> 
> Regards
> Anju
> 
> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com] 
> Sent: Wednesday, February 06, 2019 8:48 PM
> To: Anju Thomas ; Ben Pfaff 
> Cc: d...@openvswitch.org
> Subject: Re: [PATCHv7 1/3] Improved Packet Drop Statistics in OVS
> 
> Hi.
> See comments inline.
> 
> Best regards, Ilya Maximets.
> 
> On 06.02.2019 7:01, Anju Thomas wrote:
>>
>> Hi Ben/Ilya,
>>
>> I have addressed the comments in the below patch. Can you tell me if 
>> this is fin> Regards Anju -Original Message-
>> From: Anju Thomas [mailto:anju.tho...@ericsson.com]
>> Sent: Tuesday, January 29, 2019 5:21 PM
>> To: d...@openvswitch.org
>> Cc: Anju Thomas 
>> Subject: [PATCHv7 1/3] Improved Packet Drop Statistics in OVS
>>
>>Currently OVS maintains explicit packet drop/error counters only on port
>>level. Packets that are dropped as part of normal OpenFlow processing are
>>counted in flow stats of “drop” flows or as table misses in table stats.
>>These can only be interpreted by controllers that know the semantics of
>>the configured OpenFlow pipeline. Without that knowledge, it is impossible
>>for an OVS user to obtain e.g. the total number of packets dropped due to
>>OpenFlow rules.
>>
>>Furthermore, there are numerous other reasons for which packets can be
>>dropped by OVS slow path that are not related to the OpenFlow pipeline.
>>The generated datapath flow entries include a drop action to avoid further
>>expensive upcalls to the slow path, but subsequent packets dropped by the
>>datapath are not accounted anywhere.
>>
>>Finally, the datapath itself drops packets in certain error situations.
>>Also, these drops are today not accounted for.
>>
>>This makes it difficult for OVS users to monitor packet drop in an OVS
>>instance and to alert a management system in case of a unexpected increase
>>of such drops. Also OVS trouble-shooters face difficulties in analysing
>>packet drops.
>>
>>With this patch we implement following changes to address the issues
>>mentioned above.
>>
>>1. Identify and account all the silent packet drop scenarios
>>
>>2. Display these drops in ovs-appctl coverage/show
>>
>>A detailed presentation on this was presented at OvS conference 2017 and
>>link for the corresponding presentation is available at:
>>
>>
>> https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-d
>> ata-plane-in-ovs-82280329
>>
>>Co-authored-by: Rohith Basavaraja 
>>Co-authored-by: Keshav Gupta 
>>Signed-off-by: Anju Thomas 
>>Signed-off-by: Rohith Basavaraja 
>>Signed-off-by: Keshav Gupta 
> 
> 
> You still have this strange shift to the right by 3 spaces in commit-message.
> 
>> ---
>>  datapath/linux/compat/include/linux/openvswitch.h |  1 +
>>  lib/dpif-netdev.c | 39 ++-
>>  lib/dpif.c|  7 ++
>>  lib/dpif.h|  3 +
>>  lib/netdev-dpdk.c |  4 ++
>>  lib/odp-execute.c | 81 
>> +++
>>  lib/odp-execute.h |  2 +
>>  lib/odp-util.c| 10 ++-
>>  ofproto/ofproto-dpif-ipfix.c  |  1 +
>>  ofproto/ofproto-dpif-sflow.c  |  1 +
>>  ofproto/ofproto-dpif-xlate.c  | 31 +
>>  ofproto/ofproto-dpif-xlate.h  |  4 ++
>>  ofproto/ofproto-dpif.c|  8 +++
>>  ofproto/ofproto-dpif.h|  8 ++-
>>  tests/automake.mk |  3 +-
>>  tests/dpif-netdev.at  | 10 +++
>>  tests/ofproto-dpif.at |  4 +-
>>  tests/testsuite.at 

Re: [ovs-dev] [PATCHv7 1/3] Improved Packet Drop Statistics in OVS

[Anju Thomas]

Thanks for comments Ilya. Just one question regarding the steal flag :-

> +case OVS_ACTION_ATTR_DROP: {
> +const enum xlate_error *drop_reason = nl_attr_get(a);
> +if (*drop_reason < XLATE_MAX) {
> + dp_update_drop_action_counter(*drop_reason, batch->count);
> +}
> +dp_packet_delete_batch(batch, steal);

I'm not sure if we need to honor 'steal' while performing the DROP action, 
because packets will not be dropped if 'steal == false'.

 What if there is the drop is within a clone action and there is another 
output action.In that case , we would not want to delete the batch . Right?

Regards
Anju

-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com] 
Sent: Wednesday, February 06, 2019 8:48 PM
To: Anju Thomas ; Ben Pfaff 
Cc: d...@openvswitch.org
Subject: Re: [PATCHv7 1/3] Improved Packet Drop Statistics in OVS

Hi.
See comments inline.

Best regards, Ilya Maximets.

On 06.02.2019 7:01, Anju Thomas wrote:
> 
> Hi Ben/Ilya,
> 
> I have addressed the comments in the below patch. Can you tell me if 
> this is fin> Regards Anju -Original Message-
> From: Anju Thomas [mailto:anju.tho...@ericsson.com]
> Sent: Tuesday, January 29, 2019 5:21 PM
> To: d...@openvswitch.org
> Cc: Anju Thomas 
> Subject: [PATCHv7 1/3] Improved Packet Drop Statistics in OVS
> 
>Currently OVS maintains explicit packet drop/error counters only on port
>level. Packets that are dropped as part of normal OpenFlow processing are
>counted in flow stats of “drop” flows or as table misses in table stats.
>These can only be interpreted by controllers that know the semantics of
>the configured OpenFlow pipeline. Without that knowledge, it is impossible
>for an OVS user to obtain e.g. the total number of packets dropped due to
>OpenFlow rules.
> 
>Furthermore, there are numerous other reasons for which packets can be
>dropped by OVS slow path that are not related to the OpenFlow pipeline.
>The generated datapath flow entries include a drop action to avoid further
>expensive upcalls to the slow path, but subsequent packets dropped by the
>datapath are not accounted anywhere.
> 
>Finally, the datapath itself drops packets in certain error situations.
>Also, these drops are today not accounted for.
> 
>This makes it difficult for OVS users to monitor packet drop in an OVS
>instance and to alert a management system in case of a unexpected increase
>of such drops. Also OVS trouble-shooters face difficulties in analysing
>packet drops.
> 
>With this patch we implement following changes to address the issues
>mentioned above.
> 
>1. Identify and account all the silent packet drop scenarios
> 
>2. Display these drops in ovs-appctl coverage/show
> 
>A detailed presentation on this was presented at OvS conference 2017 and
>link for the corresponding presentation is available at:
> 
>
> https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-d
> ata-plane-in-ovs-82280329
> 
>Co-authored-by: Rohith Basavaraja 
>Co-authored-by: Keshav Gupta 
>Signed-off-by: Anju Thomas 
>Signed-off-by: Rohith Basavaraja 
>Signed-off-by: Keshav Gupta 


You still have this strange shift to the right by 3 spaces in commit-message.

> ---
>  datapath/linux/compat/include/linux/openvswitch.h |  1 +
>  lib/dpif-netdev.c | 39 ++-
>  lib/dpif.c|  7 ++
>  lib/dpif.h|  3 +
>  lib/netdev-dpdk.c |  4 ++
>  lib/odp-execute.c | 81 
> +++
>  lib/odp-execute.h |  2 +
>  lib/odp-util.c| 10 ++-
>  ofproto/ofproto-dpif-ipfix.c  |  1 +
>  ofproto/ofproto-dpif-sflow.c  |  1 +
>  ofproto/ofproto-dpif-xlate.c  | 31 +
>  ofproto/ofproto-dpif-xlate.h  |  4 ++
>  ofproto/ofproto-dpif.c|  8 +++
>  ofproto/ofproto-dpif.h|  8 ++-
>  tests/automake.mk |  3 +-
>  tests/dpif-netdev.at  | 10 +++
>  tests/ofproto-dpif.at |  4 +-
>  tests/testsuite.at|  1 +
>  tests/tunnel-push-pop.at  | 28 +++-
>  tests/tunnel.at   | 14 +++-
>  20 files changed, 248 insertions(+), 12 deletions(-)
> 
> diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
> b/datapath/linux/compat/include/linux/openvswitch.h
> index 9b087f1..92db378 100644
> --- a/datapath/linux/compat/include/linux/openvswitch.h
> +++ b/datapath/linux/compat/include/linux/openvswitch.h
> @@ -938,6 +938,7 @@ enum ovs_action_attr {

[ovs-dev] [PATCH] acinclude: Use NUMA_AWARE_HUGEPAGES too for libnuma check.

[Ilya Maximets]

This fixes build with NUMA_AWARE_HUGEPAGES enabled and VHOST_NUMA
disabled. This should not be a usual case. But it's possible to
configure DPDK this way.

Fixes: 5e925ccc2a6f ("netdev-dpdk: DPDK v17.11 upgrade")

Signed-off-by: Ilya Maximets 
---
 acinclude.m4 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/acinclude.m4 b/acinclude.m4
index c51af246a..e2af4ee16 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -254,7 +254,7 @@ AC_DEFUN([OVS_CHECK_DPDK], [
   AC_LANG_PROGRAM(
 [
   #include 
-#if RTE_LIBRTE_VHOST_NUMA
+#if defined(RTE_LIBRTE_VHOST_NUMA) || defined(RTE_EAL_NUMA_AWARE_HUGEPAGES)
 #error
 #endif
 ], [])
-- 
2.17.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] acinclude: Drop DPDK_EXTRA_LIB variable.

[Ilya Maximets]

AC_SEARCH_LIBS enables the libraries itself:

  checking for library containing get_mempolicy... -lnuma
  checking for library containing pcap_dump... -lpcap

So, they are available in LIBS. No need to add them twice.

Also, DPDK_EXTRA_LIB doesn't even work, because each check overwrites
the variable instead of appending the new library. It was first time
misused while making libnuma optional and copy-pasted to several places
after that.

Signed-off-by: Ilya Maximets 
---
 acinclude.m4 | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 95241b142..c51af246a 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -242,7 +242,6 @@ AC_DEFUN([OVS_CHECK_DPDK], [
 esac
 
 DPDK_LIB="-ldpdk"
-DPDK_EXTRA_LIB=""
 
 ovs_save_CFLAGS="$CFLAGS"
 ovs_save_LDFLAGS="$LDFLAGS"
@@ -261,7 +260,6 @@ AC_DEFUN([OVS_CHECK_DPDK], [
 ], [])
   ], [],
   [AC_SEARCH_LIBS([get_mempolicy],[numa],[],[AC_MSG_ERROR([unable to find 
libnuma, install the dependency package])])
-   DPDK_EXTRA_LIB="-lnuma"
AC_DEFINE([VHOST_NUMA], [1], [NUMA Aware vHost support detected in 
DPDK.])])
 
 AC_COMPILE_IFELSE([
@@ -274,7 +272,6 @@ AC_DEFUN([OVS_CHECK_DPDK], [
 ], [])
   ], [],
   [AC_SEARCH_LIBS([pcap_dump],[pcap],[],[AC_MSG_ERROR([unable to find 
libpcap, install the dependency package])])
-   DPDK_EXTRA_LIB="-lpcap"
AC_COMPILE_IFELSE([
  AC_LANG_PROGRAM(
[
@@ -297,7 +294,6 @@ AC_DEFUN([OVS_CHECK_DPDK], [
 ], [])
   ], [],
   [AC_SEARCH_LIBS([mnl_attr_put],[mnl],[],[AC_MSG_ERROR([unable to find 
libmnl, install the dependency package])])
-   DPDK_EXTRA_LIB="-lmnl"
AC_DEFINE([DPDK_MNL], [1], [MLX5 PMD detected in DPDK.])])
 
 # On some systems we have to add -ldl to link with dpdk
@@ -310,7 +306,7 @@ AC_DEFUN([OVS_CHECK_DPDK], [
 DPDKLIB_FOUND=false
 save_LIBS=$LIBS
 for extras in "" "-ldl"; do
-LIBS="$DPDK_LIB $extras $save_LIBS $DPDK_EXTRA_LIB"
+LIBS="$DPDK_LIB $extras $save_LIBS"
 AC_LINK_IFELSE(
[AC_LANG_PROGRAM([#include 
  #include ],
-- 
2.17.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v4 1/1] acinclude: Also use LIBS from dpkg pkg-config

[Christian Ehrhardt]

DPDK 18.11 builds using the more modern meson build system no more
provide the -ldpdk linker script. Instead it is expected to use
pkgconfig for linker options as well.

This change will set DPDK_LIB from pkg-config (if pkg-config was
available) and since that already carries the whole-archive flags
around the PMDs skips the further wrapping in more whole-archive
if that is already part of DPDK_LIB.

To work reliable in all environments this needs pkg-config 0.29.1.
We want to be able to use PKG_CHECK_MODULES_STATIC which
is not yet available in 0.24. Therefore update pkg.m4
to pkg-config 0.29.1.

This should be backport-safe as these macro files are all versioned.
autoconf is smart enough to check the version if you have it locally,
and if the system's is higher, it will use that one instead.

Finally make configure.ac use the locally provided pkg.m4 before
calling the PKG_PROG_PKG_CONFIG macro.

Signed-off-by: Christian Ehrhardt 
---
 acinclude.m4 |  21 +++--
 configure.ac |   2 +
 m4/pkg.m4| 217 +--
 3 files changed, 157 insertions(+), 83 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 95241b142..8ada98623 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -223,9 +223,13 @@ AC_DEFUN([OVS_CHECK_DPDK], [
 case "$with_dpdk" in
   yes)
 DPDK_AUTO_DISCOVER="true"
-PKG_CHECK_MODULES([DPDK], [libdpdk],
-  [DPDK_INCLUDE="$DPDK_CFLAGS"],
-  [DPDK_INCLUDE="-I/usr/local/include/dpdk 
-I/usr/include/dpdk"])
+PKG_CHECK_MODULES_STATIC([DPDK], [libdpdk], [
+DPDK_INCLUDE="$DPDK_CFLAGS"
+DPDK_LIB="$DPDK_LIBS"
+ ], [
+DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk"
+DPDK_LIB="-ldpdk"
+ ])
 ;;
   *)
 DPDK_AUTO_DISCOVER="false"
@@ -238,10 +242,10 @@ AC_DEFUN([OVS_CHECK_DPDK], [
DPDK_INCLUDE="-I$DPDK_INCLUDE_PATH/dpdk"
 fi
 DPDK_LIB_DIR="$with_dpdk/lib"
+DPDK_LIB="-ldpdk"
 ;;
 esac
 
-DPDK_LIB="-ldpdk"
 DPDK_EXTRA_LIB=""
 
 ovs_save_CFLAGS="$CFLAGS"
@@ -346,7 +350,14 @@ AC_DEFUN([OVS_CHECK_DPDK], [
 #
 # These options are specified inside a single -Wl directive to prevent
 # autotools from reordering them.
-DPDK_vswitchd_LDFLAGS=-Wl,--whole-archive,$DPDK_LIB,--no-whole-archive
+#
+# OTOH newer versions of dpdk pkg-config (generated with Meson)
+# will already have flagged just the right set of libs with
+# --whole-archive - in those cases do not wrap it once more.
+case "$DPDK_LIB" in
+  *whole-archive*) DPDK_vswitchd_LDFLAGS=$DPDK_LIB;;
+  *) DPDK_vswitchd_LDFLAGS=-Wl,--whole-archive,$DPDK_LIB,--no-whole-archive
+esac
 AC_SUBST([DPDK_vswitchd_LDFLAGS])
 AC_DEFINE([DPDK_NETDEV], [1], [System uses the DPDK module.])
   fi
diff --git a/configure.ac b/configure.ac
index 505e3d041..dc6eebbf5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -28,6 +28,8 @@ AC_PROG_CPP
 AC_PROG_MKDIR_P
 AC_PROG_FGREP
 AC_PROG_EGREP
+
+m4_include([m4/pkg.m4])
 PKG_PROG_PKG_CONFIG
 
 AM_MISSING_PROG([AUTOM4TE], [autom4te])
diff --git a/m4/pkg.m4 b/m4/pkg.m4
index c5b26b52e..82bea96ee 100644
--- a/m4/pkg.m4
+++ b/m4/pkg.m4
@@ -1,29 +1,60 @@
-# pkg.m4 - Macros to locate and utilise pkg-config.-*- Autoconf -*-
-# serial 1 (pkg-config-0.24)
-# 
-# Copyright © 2004 Scott James Remnant .
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful, but
-# WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-# General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
-#
-# As a special exception to the GNU General Public License, if you
-# distribute this file as part of a program that contains a
-# configuration script generated by Autoconf, you may include it under
-# the same distribution terms that you use for the rest of that program.
-
-# PKG_PROG_PKG_CONFIG([MIN-VERSION])
-# --
+dnl pkg.m4 - Macros to locate and utilise pkg-config.   -*- Autoconf -*-
+dnl serial 11 (pkg-config-0.29.1)
+dnl
+dnl Copyright © 2004 Scott James Remnant .
+dnl Copyright © 2012-2015 Dan Nicholson 
+dnl
+dnl This program is free software; you can redistribute it and/or modify
+dnl it under the terms of the GNU General Public License as published by
+dnl the Free Software Foundation; either version 2 of the Licen

[ovs-dev] [PATCH v4 0/1] fix build with DPDK 18.11 without -ldpdk

[Christian Ehrhardt]

DPDK 18.11 can be built using the more modern meson build system.
In that case it no more provides the -ldpdk linker script. Instead
it is expected to use pkgconfig for linker options as well.

FYI here a build log on Ubuntu 19.04 with the three patches applied:
https://launchpadlibrarian.net/409021278/buildlog_ubuntu-disco-amd64.openvswitch_2.11.0~git20190121.4e4f80ec2-0ubuntu1~ppa5_BUILDING.txt.gz

Updates from v3:
- change separation of multiple actions in PKG_CHECK_MODULES_STATIC to
  newlines to avoid trailing commas in some build environments

Updates from v2:
- patches were formerly structured for reviewability, but to guarantee
  travis requirements to pass the build individually and not as a series
  they are now squashed to be acceptable

Updates from v1:
- add updated pkg.m4 to support PKG_CHECK_MODULES_STATIC
- include local pkg.m4 in configure.ac

Christian Ehrhardt (1):
  acinclude: Also use LIBS from dpkg pkg-config

 acinclude.m4 |  21 +++--
 configure.ac |   2 +
 m4/pkg.m4| 217 +--
 3 files changed, 157 insertions(+), 83 deletions(-)

-- 
2.17.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 1/1] acinclude: Also use LIBS from dpkg pkg-config

[Luca Boccassi]

On Thu, 2019-02-07 at 12:00 +0100, Christian Ehrhardt wrote:
> [...]
> 
> >  case "$with_dpdk" in
> >    yes)
> >  DPDK_AUTO_DISCOVER="true"
> > -PKG_CHECK_MODULES([DPDK], [libdpdk],
> > -  [DPDK_INCLUDE="$DPDK_CFLAGS"],
> > -  [DPDK_INCLUDE="-I/usr/local/include/dpdk 
> > -I/usr/include/dpdk"])
> > +PKG_CHECK_MODULES_STATIC([DPDK], [libdpdk],
> > + [DPDK_INCLUDE="$DPDK_CFLAGS",
> > DPDK_LIB="$DPDK_LIBS"],
> > + [DPDK_INCLUDE="-
> > I/usr/local/include/dpdk -I/usr/include/dpdk", DPDK_LIB="-ldpdk"])
> >  ;;
> >    *)
> >  DPDK_AUTO_DISCOVER="false"
> 
> While working fine in all my builds (and it seems on travis now) I
> got
> reports of the statements above creating a colon in the assignment on
> some builds - thanks James (on CC now).
> It was adding a trailing colon to the FLAGS which broke his build.
> 
> I wanted to ask the more experienced autoconf users if that makes any
> sense?
> 
> We wondered if instead of colon actions inside the action sections
> would better be newline [1] separated instead:
> dnl PKG_CHECK_MODULES_STATIC(VARIABLE-PREFIX, MODULES, [ACTION-IF-
> FOUND],
> dnl   [ACTION-IF-NOT-FOUND])
> 
> [1]: https://stackoverflow.com/questions/12735432/how-to-pack-multipl
> e-statements
> 
> Like the following then:
> 
> -+ [DPDK_INCLUDE="$DPDK_CFLAGS",
> DPDK_LIB="$DPDK_LIBS"],
> -+
> [DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk",
> DPDK_LIB="-ldpdk"])
> ++ [DPDK_INCLUDE="$DPDK_CFLAGS"
> ++  DPDK_LIB="$DPDK_LIBS"],
> ++
> [DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk"
> ++  DPDK_LIB="-ldpdk"])
> 
> If that assumption could be backed by some autoconf experience I'd
> make a v4 which splits by newlines instead of the colon.

Hi,

Yes using newlines will work, I've used it before. Or rather should, we
are talking about autoconf after all :-)

https://github.com/zeromq/libzmq/blob/master/configure.ac#L559

-- 
Kind regards,
Luca Boccassi___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS-DPDK public meeting

[Kevin Traynor]

Next meeting Feb 20th 1700 UTC (will cancel if OVS2.11 already released
or no agenda)

Attendees: Aaron, David, Alejandro, Asaf, Frikkie, Ophir, Pieter,
Johann, Ian, Kevin.

===
GENERAL
===
- OVS 2.11 Release
-- No blockers reported, so still heading for
http://docs.openvswitch.org/en/latest/internals/release-process/?highlight=release#release-scheduling

- DPDK Stable Releases
-- 18.11.1-RC1 in about a week
-- Considering validation times, 18.11.1 won't be ready for OVS 2.11
-- Next 17.11.5 release is waiting validation
-- Ian said may be able to do some RC testing with OVS test setup

- OVS CI Status
-- Aaron has some NICs sent by vendors ready for this
-- Servers in UNH, but they will be changed shortly
-- There is a regular call about the DPDK Lab
-- Next meeting is next Tuesday 12th Feb. Details in [1].


PERFORMANCE/FEATURES

- DPDK 18.11 Hugepage Memory limits
-- Patched added and backported to 2.11, to add the socket-limit by
default and so keep the existing default behavior

- Unifying DPDK EAL Arguments
-- Discussion on ML about this proposal to remove the some of the
specific DB entries for DPDK EAL args
-- Would be a user visible change, so good if people can to look at the RFC

- MESON packaging and Build
-- RH using make for DPDK
-- Aaron will try out pkg-config patch


Bugs

- dpif-netdev:fix reload pmd's dead lock
-- Now resolved as being issue with user local code changes
-- https://mail.openvswitch.org/pipermail/ovs-dev/2019-February/355981.html


[1]
When: Tuesday, February 12, 2019 2:00 PM-2:30 PM (UTC+00:00) Dublin,
Edinburgh, Lisbon, London.
Where: Skype Meeting

.
--> Join Skype Meeting
  Trouble Joining? Try Skype Web
App
Join by phone
+1(916)356-2663 (or your local bridge access #) Choose bridge
5.
(Global)   English (United States)
Find a local number

Conference ID: 298060418
 Forgot your dial-in PIN?
|Help


On 07/25/2017 02:25 PM, Kevin Traynor wrote:
> Hi All,
> 
> The OVS-DPDK public meetings have moved to Wednesday's at the same time.
> Everyone is welcome, it's a chance to share status/plans etc.
> 
> It's scheduled for every 2 weeks starting 26th July. Meeting time is
> currently @ 4pm UTC.
> 
> You can connect through Bluejeans or through dial in:
> 
> https://bluejeans.com/139318596
> 
> US: +1.408.740.7256 
> UK: +44.203.608.5256 
> Germany: +49.32.221.091256 
> Ireland: +353.1.697.1256 
> Other numbers at https://www.bluejeans.com/numbers
> Meeting ID: 139318596
> 
> thanks,
> Kevin.
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 1/1] acinclude: Also use LIBS from dpkg pkg-config

[Christian Ehrhardt]

[...]

>  case "$with_dpdk" in
>yes)
>  DPDK_AUTO_DISCOVER="true"
> -PKG_CHECK_MODULES([DPDK], [libdpdk],
> -  [DPDK_INCLUDE="$DPDK_CFLAGS"],
> -  [DPDK_INCLUDE="-I/usr/local/include/dpdk 
> -I/usr/include/dpdk"])
> +PKG_CHECK_MODULES_STATIC([DPDK], [libdpdk],
> + [DPDK_INCLUDE="$DPDK_CFLAGS", 
> DPDK_LIB="$DPDK_LIBS"],
> + [DPDK_INCLUDE="-I/usr/local/include/dpdk 
> -I/usr/include/dpdk", DPDK_LIB="-ldpdk"])
>  ;;
>*)
>  DPDK_AUTO_DISCOVER="false"

While working fine in all my builds (and it seems on travis now) I got
reports of the statements above creating a colon in the assignment on
some builds - thanks James (on CC now).
It was adding a trailing colon to the FLAGS which broke his build.

I wanted to ask the more experienced autoconf users if that makes any sense?

We wondered if instead of colon actions inside the action sections
would better be newline [1] separated instead:

dnl PKG_CHECK_MODULES_STATIC(VARIABLE-PREFIX, MODULES, [ACTION-IF-FOUND],
dnl   [ACTION-IF-NOT-FOUND])

[1]: 
https://stackoverflow.com/questions/12735432/how-to-pack-multiple-statements

Like the following then:

-+ [DPDK_INCLUDE="$DPDK_CFLAGS",
DPDK_LIB="$DPDK_LIBS"],
-+
[DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk",
DPDK_LIB="-ldpdk"])
++ [DPDK_INCLUDE="$DPDK_CFLAGS"
++  DPDK_LIB="$DPDK_LIBS"],
++
[DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk"
++  DPDK_LIB="-ldpdk"])

If that assumption could be backed by some autoconf experience I'd
make a v4 which splits by newlines instead of the colon.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] dpif-netdev:fix reload pmd's dead lock

[Stokes, Ian]

> On 31.01.2019 11:48, Lilijun wrote:
> > This patch fix the dead lock when using dpdk userspace datapath. The
> > problem is described as follows:
> > 1) when add or delete port, the main thread will call
> > reconfigure_datapath() in the function dpif_netdev_run()
> > 2) Here the dp->port_mutex is locked. In dp_netdev_reload_pmd__(), it
> > will notify each pmd to reload.
> > 3) If pmd is doing packet upcall in fast_path_processing() and try to
> > get
> > dp->port_mutex in
> > do_xlate_actions()-->tnl_route_lookup_flow()--
> >dpif_netdev_port_query_by_name().
> > Here pmd get lock failed because the main thread has got the lock in
> > step 2.
> > 4) So the main thread was stuck for waiting pmd to reload done. Now we
> > got a dead lock.
> >
> > Here reload_affected_pmds() may not need to lock dp->port_mutex. So we
> > release the lock temporarily when calling  reload_affected_pmds().
> >
> > Signed-off-by: Lilijun 
> 
> Replying just to keep answers on a list/patchwork consistent.
> The deadlock caused by some local changes done by the user.
> Not possible in upstream master. See discussion for the previous version
> of this patch that is missing in patchwork for some reason:
> 
>https://mail.openvswitch.org/pipermail/ovs-dev/2019-January/355756.html
> 
> Best regards, Ilya Maximets.
> 

Thanks for clarifying Ilya, there was discussion of this at the community 
meeting yesterday but it seems a non-issue now.

Thanks
Ian
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev