[ovs-dev] [PATCH] Typo fix: vswtich -> vswitch.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 
---
 NEWS  | 2 +-
 tests/ofproto-dpif.at | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 4d56eee31900..1df105c41c95 100644
--- a/NEWS
+++ b/NEWS
@@ -28,7 +28,7 @@ Post-v2.12.0
  * Add support for DPDK 19.11.
- RSTP:
  * The rstp_statistics column in Port table will only be updated every
-   stats-update-interval configured in Open_vSwtich table.
+   stats-update-interval configured in Open_vSwitch table.
- OVSDB:
  * When ovsdb-server is running in backup mode, the default value of probe
interval is increased to 60 seconds for the connection to the
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 88acc4b1ab06..ff1cc93707b8 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -7970,7 +7970,7 @@ dl_src=60:66:66:66:66:01 
actions=pop_mpls:0x8847,controller
 AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 
 dnl Packet is dropped because an MPLS PUSH action is applied to a packet with
-dnl 4 MPLS LSEs but ovs-vswtichd can only handle up to 3 MPLS LSEs and thus
+dnl 4 MPLS LSEs but ovs-vswitchd can only handle up to 3 MPLS LSEs and thus
 dnl can't determine the resulting MPLS label after MPLS push/pop actions.
 dnl
 dnl The input is a frame with two MPLS headers which tcpdump -vve shows as:
-- 
2.23.0

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

Re: [ovs-dev] [PATCH] Documentation: Fix building with Python 3.9

[Ben Pfaff]

On Thu, Jan 16, 2020 at 11:20:01AM -0300, Flavio Leitner wrote:
> On Thu, Jan 16, 2020 at 02:21:47PM +0100, Timothy Redaelli wrote:
> > open(), io.open(), codecs.open() and fileinput.FileInput no longer accept 
> > 'U'
> > ("universal newline") in the file mode.
> > This flag was deprecated since Python 3.3.
> > In Python 3, the "universal newline" is used by default when a file is open
> > in text mode.
> > 
> > Reported-at: https://bugzilla.redhat.com/1791681
> > Reported-by: Miro Hrončok 
> > Signed-off-by: Timothy Redaelli 
> > ---
> 
> LGTM
> Acked-by: Flavio Leitner 

Thanks Timothy (and Flavio).  Applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] ovsdb-server: Allow OVSDB clients to specify the UUID for inserted rows.

[Ben Pfaff]

On Thu, Jan 09, 2020 at 07:39:22PM -0800, Han Zhou wrote:
> On Thu, Jan 9, 2020 at 12:48 PM Ben Pfaff  wrote:
> >
> > Requested-by: Leonid Ryzhyk 
> > Signed-off-by: Ben Pfaff 
> > ---
> > v1->v2: Improve test as suggested by Flavio Fernandes.
>
> Acked-by: Han Zhou 

Applied to master, thanks!

> (I also tested with ovsdb-client which works as expected. Maybe we can
> update the xxxctl tools to support specifying uuids for the "create"
> command as well)

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

[ovs-dev] AWB Notification: You have A Package With Us

[TNT EXPRESS INC]

Greetings, 

FYI
Your consignment have been booked via TNT EXPRESS is scheduled for
delivery on January 20. 

But your shipment address is missing please kindly review the shipping
details attached to confirm if every information is correct to enable us
dispatch. 

Attached is the shipping document for clearing and reference purposes 

If you would like to find out about the many ways TNT helps you to track
your shipment, or if you would like to know more about the services
provided by TNT, simply connect to www.tnt.com and select your location
at any time. 

---


This message and any attachment are confidential and may be privileged
or otherwise protected from disclosure. 
If you are not the intended recipient, please telephone or email the
sender and delete this message and any attachment from your system. 
If you are not the intended recipient you must not copy this message or
attachment or disclose the contents to any other person.
Please consider the environmental impact before printing this document
and its attachment(s). 
Print black and white and double-sided where possible.
--
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Happy New Year (82)

[arivumani]

    ...  .. ..

https://bit.ly/2ragBc8








80342 46 762

j4l7ef o164q ezm mq6pi3l 483iqou r 7by1gv2 hi hz06

5kp 5ii8ia qef ql dj5fo m28ovc v6n nzm38


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

Re: [ovs-dev] [PATCHv5] userspace: Add GTP-U support.

[William Tu]

On Tue, Jan 7, 2020 at 2:53 PM Ben Pfaff  wrote:
>
> On Wed, Dec 11, 2019 at 04:24:13PM -0800, William Tu wrote:
> > GTP, GPRS Tunneling Protocol, is a group of IP-based communications
> > protocols used to carry general packet radio service (GPRS) within
> > GSM, UMTS and LTE networks.  GTP protocol has two parts: Signalling
> > (GTP-Control, GTP-C) and User data (GTP-User, GTP-U). GTP-C is used
> > for setting up GTP-U protocol, which is an IP-in-UDP tunneling
> > protocol. Usually GTP is used in connecting between base station for
> > radio, Serving Gateway (S-GW), and PDN Gateway (P-GW).
> >
> > This patch implements GTP-U protocol for userspace datapath,
> > supporting only required header fields and G-PDU message type.
> > See spec in:
> > https://tools.ietf.org/html/draft-hmm-dmm-5g-uplane-analysis-00
> >
> > Signed-off-by: Yi Yang 
> > Co-authored-by: Yi Yang 
> > Signed-off-by: William Tu 
>
> I apologize that I haven't reviewed this new version even though it's
> been almost a month.  In the meantime, there are a few patch conflicts.
> Would you mind rebasing?
>
Thanks Ben. I've sent out the new version.
https://mail.openvswitch.org/pipermail/ovs-dev/2020-January/366907.html
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCHv6] userspace: Add GTP-U support.

[William Tu]

GTP, GPRS Tunneling Protocol, is a group of IP-based communications
protocols used to carry general packet radio service (GPRS) within
GSM, UMTS and LTE networks.  GTP protocol has two parts: Signalling
(GTP-Control, GTP-C) and User data (GTP-User, GTP-U). GTP-C is used
for setting up GTP-U protocol, which is an IP-in-UDP tunneling
protocol. Usually GTP is used in connecting between base station for
radio, Serving Gateway (S-GW), and PDN Gateway (P-GW).

This patch implements GTP-U protocol for userspace datapath,
supporting only required header fields and G-PDU message type.
See spec in:
https://tools.ietf.org/html/draft-hmm-dmm-5g-uplane-analysis-00

Signed-off-by: Yi Yang 
Co-authored-by: Yi Yang 
Signed-off-by: William Tu 
---
v5 -> v6:
  - rebase to master
  - travis: https://travis-ci.org/williamtu/ovs-travis/builds/638083655

v4 -> v5:
  - address Ben and Aaron comments
1) flow_get_metadata, format_flow_tunnel
2) use of ?: in MSVS
3) tun_key_to_attr
4) handling PT_GTPU_MSG packets
5) make gtpu_flags and gtpu_msgtype read-only
  - use be32_to_be64 and be64_to_be32
  - make gtpu_flags as hexadicimal, gtpu_msgtype as decimal
  - remove PT_GTPU_MSG
  Address Roni's comments
  - for non-GPDU msgtype, don't pop header
  - add seq number flags parsing, push/pop header support
  - refactor netdev_tnl_push_udp_header()

v3 -> v4:
  - applied Ben's doc revise
  - increment FLOW_WC_SEQ to 42
  - minor fixes
  - travis: https://travis-ci.org/williamtu/ovs-travis/builds/621289678

v2 -> v3:
  - pick up the code from v2, rebase to master
  - many fixes in code, docs, and add more tests
  - travis: https://travis-ci.org/williamtu/ovs-travis/builds/619799361

v1 -> v2:
  - Add new packet_type PT_GTPU_MSG to handle GTP-U signaling message,
GTP-U signaling message should be steered into a control plane handler
by explicit openflow entry in flow table.
  - Fix gtpu_flags and gptu_msgtype set issue.
  - Verify communication with kernel GTP implementation from Jiannan
Ouyang.
  - Fix unit test issue and make sure all the unit tests can pass.
  - This patch series add GTP-U tunnel support in DPDK userspace,
GTP-U is used for carrying user data within the GPRS core network and
between the radio access network and the core network. The user data
transported can be packets in any of IPv4, IPv6, or PPP formats.
---
 Documentation/faq/configuration.rst   |  13 ++
 Documentation/faq/releases.rst|   1 +
 NEWS  |   3 +
 datapath/linux/compat/include/linux/openvswitch.h |   2 +
 include/openvswitch/flow.h|   4 +-
 include/openvswitch/match.h   |   6 +
 include/openvswitch/meta-flow.h   |  28 
 include/openvswitch/packets.h |   4 +-
 lib/dpif-netlink-rtnl.c   |   5 +
 lib/dpif-netlink.c|   5 +
 lib/flow.c|  28 ++--
 lib/flow.h|   2 +-
 lib/match.c   |  36 -
 lib/meta-flow.c   |  38 +
 lib/meta-flow.xml |  79 ++-
 lib/netdev-native-tnl.c   | 165 --
 lib/netdev-native-tnl.h   |  13 ++
 lib/netdev-vport.c|  25 +++-
 lib/nx-match.c|   8 +-
 lib/odp-util.c| 123 +++-
 lib/odp-util.h|   2 +-
 lib/ofp-match.c   |   2 +-
 lib/packets.h |  68 +
 lib/tnl-ports.c   |   3 +
 ofproto/ofproto-dpif-rid.h|   2 +-
 ofproto/ofproto-dpif-xlate.c  |   3 +-
 tests/ofproto.at  |   2 +-
 tests/tunnel-push-pop.at  |  22 +++
 tests/tunnel.at   |  76 ++
 vswitchd/vswitch.xml  |  24 
 30 files changed, 752 insertions(+), 40 deletions(-)

diff --git a/Documentation/faq/configuration.rst 
b/Documentation/faq/configuration.rst
index ff3b71a5d4ef..4a98740c5d4d 100644
--- a/Documentation/faq/configuration.rst
+++ b/Documentation/faq/configuration.rst
@@ -225,6 +225,19 @@ Q: Does Open vSwitch support IPv6 GRE?
 options:remote_ip=fc00:100::1 \
 options:packet_type=legacy_l2
 
+Q: Does Open vSwitch support GTP-U?
+
+A: Yes. Starting with version 2.13, the Open vSwitch userspace
+datapath supports GTP-U (GPRS Tunnelling Protocol User Plane
+(GTPv1-U)). TEID is set by using tunnel key field.
+
+::
+
+$ ovs-vsctl add-br br0
+$ ovs-vsctl add-port br0 gtpu0 -- \
+   

Re: [ovs-dev] [PATCH v2] tc: handle packet mark of zero

[Ben Pfaff]

On Thu, Jan 16, 2020 at 11:20:20AM +0100, Simon Horman wrote:
> On Thu, Jan 16, 2020 at 04:59:54AM -0500, 0-day Robot wrote:
> > Bleep bloop.  Greetings Simon Horman, 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: Unexpected sign-offs from developers who are not authors or 
> > co-authors or committers: Simon Horman 
> > Lines checked: 44, Warnings: 1, Errors: 0
> 
> Is the correct tag Co-Authored-by rather than Co-Authored as used in
> this patch?

Yes, it should be Co-authored-by.  See
Documentation/internals/contributing/submitting-patches.rst
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] ofctrl.c: Update installed OVS flow cookie when lflow is changed.

[Ben Pfaff]

On Sun, Jan 12, 2020 at 02:48:10PM -0800, Han Zhou wrote:
> On Sun, Jan 12, 2020 at 2:10 PM Han Zhou  wrote:
> >
> > When an old lflow is replaced by a new lflow, if the OVS flows
> > translated by the old and new lflows have same match, ofctrl will
> > update existing OVS flow instead of deleting and adding a new one.
> >
> > However, when updating the existing flows, the cookie was not updated
> > by current implementation, which results in discrepency between lflows
> > and OVS flows, making debugging difficult and confuses tools such as
> > ovn-trace. This patch fixes it.
> >
> > Note: since command OFPFC_MODIFY_STRICT in FLOW_MOD message doesn't
> > support updating flow cookie after OpenFlow 1.1, this patch changes
> > to use OFPFC_ADD command, which effectively modifies existing flow
> > if a match is found.
> >
> > Signed-off-by: Han Zhou 
>
> Hi Ben,
> 
> I need your help to confirm that it is ok to replace OFPFC_MODIFY_STRICT
> with OFPFC_ADD for flow update, so that flow cookies can be updated. It
> seems there is no other way to update cookie after OF1.1.

I think you are right.  The basic issue is that the OF1.1 flow_mod wire
format (which is also used unmodified by all later versions of OpenFlow)
has only one field for a flow cookie.  This means that this field can be
used for matching on a flow cookie or updating a flow cookie, but not
(reasonably) used for both purposes.  "modify_strict" matches on cookie,
so it doesn't update it; "add" sets a cookie, so it doesn't match on it.

> Although I see this in Documentation/topics/design.rst, which mentioned
> that NXM supports updating cookie with OFPFC_MODIFY_STRICT when cookie is
> not 'UNIT64_MAX'.

The NXM wire format does allow for this.  It is just as borked as the
other protocols in terms of the number of cookie fields, that is, just
one.  (That's because the same people designed it as the plain OpenFlow
wire format, with the same lack of insight.)  But later one it was
extended to allow for a special NXM "field" that matches on the cookie.
If this is used, then we magically have two fields instead of one and
both purposes can be satisfied.

> ---
> The following table shows the handling of different protocols when receiving
> ``OFPFC_MODIFY`` and ``OFPFC_MODIFY_STRICT`` messages.  A mask of 0
> indicates
> either an explicit mask of zero or an implicit one by not specifying the
> ``NXM_NX_COOKIE(_W)`` field.
> 
> ==  ==  ==  =  =
> Match   Update   Add on missAdd on miss
> cookie  cookie mask!=0mask==0
> ==  ==  ==  =  =
> OpenFlow 1.0  no yes(add on miss)  (add on miss)
> OpenFlow 1.1 yes  no no yes
> OpenFlow 1.2 yes  no no no
> NXM  yes yes\*   no yes
> ==  ==  ==  =  =
> 
> \* Updates the flow's cookie unless the ``cookie`` field is ``UINT64_MAX``.
> ---
> However, it didn't work when I try using OFPFC_MODIFY_STRICT, even if I set
> fm.cookie =  and fm.cookie_mask = UINT64_MAX. So I had to
> change it to OFPFC_ADD to make it work.

Probably, that's because OVS doesn't really fully support the Nicira
extension flow mod in OpenFlow versions later than 1.0.  I guess that
you were modifying ovn-controller, which uses OpenFlow 1.3.  OVS doesn't
ever generate Nicira extension flow mods in OF1.3.  

In turn, that's because I didn't realize until this very moment that
there were NXM features that are not in OF1.3.  Honestly, I'd rather not
use Nicira extension flow mods outside of OF1.0.  They were supposed to
be a nasty kluge because OF1.0 was inadequate and not evolving quickly
enough.  OF1.3 is pretty good and I don't want to go back to a place
where OVS is a huge pile of extensions on top of the official protocols.

> I am not familiar with OF standard and its implementation in OVS, but the
> change worked well with my tests. Could you help explain what's the side
> effect of using OFPFC_ADD instead of OFPFC_MODIFY_STRICT?

I think it's pretty much what's listed in the table.  The reset of the
counters is the biggest thing.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] netdev_afxdp: Detects combined channels and aborts wrong config

[William Tu]

Thanks for the feedback.

The original problem we faced is that when using AF_XDP on physical device,
users often forgets to setup combined channel (ethool -L eth0 combined N).
Assume the device's default combined channel is 8, and ovs creates only 1 n_rxq,
(ovs-vsctl -- set int eth0 n_rxq=1 ...). Then there is no warning
message shows up,
and device attached to OVS successfully but receives no packet. Because XDP
program is attached to all 8 queues, and only 1 queues has XDP socket
to receives
packets. So this patch adds some warnings.

> >
> > Does this command return maximum possible number of channels or
> > number of currently enabled channels?
> Thanks for review.  This command can return both the maximum possible
> number of channels and the currently enabled channels.
>
>
> > In first case we'll end up with a need to configure huge number of queues.
> > In second case it's not guaranteed that number of channels will not be
> > changed later.  (Does enabling more channels generates if-notifier event?)
> Yes, it generates if-notifier event.

I think when combined channel changed, the device goes through some reset
process, ex: to drain the queues and re-assign queue id.
>
>
> > Another point is why we need a configurable parameter that is allowed to
> > be configured with a single value only?
I don't understand.

> >
> > And there is a possible usecase for not having all the queues attached
> > to OVS.  For example, if you have a custom xdp program loaded, you may
> > redirect specific traffic to fast afxdp queues and other traffic to
> > kernel or system datapath.  This might be useful for quick handling
> > of undesired or malicious traffic.

Yes, that's an interesting use case! This will require loading custom
XDP program,
instead of the built-in one in libbpf.
I will send out new version of the custom XDP program patch.
https://patchwork.ozlabs.org/patch/1199734/

> >
> > However, this still make sense to limit the maximum number of queues
> > with the number of actually available channels instead of having a
> > hardcoded constant (MAX_XSKQ).
>
> I agree that it is a valid use case to attach some queues to OVS and
> attach other XDP programs to the other queues.  In this case, it is
> not appropriate to fail the afxdp setup when n_rxq != # or combined
> channels or # of rx channels.
>
I think we should fail, or at least log error message.
When n_rxq != # combined channels, OVS attaches XDP programs to all
combined channels, but only handle n_rxq queues. Packets will be dropped
at the rest queues. Remember right now, the XDP program attachment is
per-netdev to all queues, and there is no per-queue XDP program support.

> On the other hand, does it make sense to log the current # of combined
> channels and the current # of rx channels in the INFO level?  It could
> be helpful to detect configuration issue on both use cases (attach all
> all the queues to OVS or not).
>
I would live to at least log the n_rxq and combined channels.

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

[ovs-dev] Trámites de capacitación ante la S.T.P.S.

[Secretaría del Trabajo y Previsión Social]

Viernes 07 de Febrero | Horario de 10:00 a 14:00 hrs.  |  (hora del centro de 
México) 

- Guía paso a paso: Trámites de capacitación ante la S.T.P.S. - 

¿De qué hablaremos?

En este webinar te presentamos una guía práctica paso a paso del proceso que 
debemos seguir ante este organismo
 para registrar la capacitación de nuestro personal así como la documentación 
necesaria para cumplir ante una 
supervisión o auditoría. 

¿Qué aprenderás?:

- El participante revisará los conceptos básicos de la capacitación en las 
empresas.
- El participante conocerá el proceso que determina la S.T.P.S. para registrar 
capacitaciones así como los formatos requeridos.  
- El participante recibirá recomendaciones para las supervisiones de la S.T.P.S.
- El participante podrá elaborar y aplicar con éxito un programa de 
capacitación con la información proporcionada.

Solicita información respondiendo a este correo con la palabra STPS junto con 
los siguientes datos:

Nombre:
Correo electrónico:
Número telefónico:

Dirigido a: Supervisores, jefes, directores, gerentes, capacitadores. 
Ejecutivos del área de RRHH, Capacitación o Desarrollo humano.

Números de Atención: 

(045) 55 15 54 66 30 - (045) 55 85567293 - (045) 5530167085 

En caso de que haya recibido este correo sin haberlo solicitado o si desea 
dejar de recibir nuestra promoción favor de responder
con la palabra baja o enviar un correo a bajas@ innovalearn.net


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

Re: [ovs-dev] [PATCH v9 2/2] netdev-afxdp: NUMA-aware memory allocation for XSK related memory

[William Tu]

On Fri, Jan 3, 2020 at 5:13 PM Yi-Hung Wei  wrote:
>
> Currently, the AF_XDP socket (XSK) related memory are allocated by main
> thread in the main thread's NUMA domain.  With the patch that detects
> netdev-linux's NUMA node id, the PMD thread of AF_XDP port will be run on
> the AF_XDP netdev's NUMA domain.  If the net device's NUMA domain
> is different from the main thread's NUMA domain, we will have two
> cross-NUMA memory accesses (netdev <-> memory, memory <-> CPU).
>
> This patch addresses the aforementioned issue by allocating
> the memory in the net device's NUMA domain.
>
> Signed-off-by: Yi-Hung Wei 

Thanks for working on this!
Acked-by: William Tu 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v4 3/3] netdev-dpdk: Add TCP Segmentation Offload support

[Flavio Leitner]

Abbreviated as TSO, TCP Segmentation Offload is a feature which enables
the network stack to delegate the TCP segmentation to the NIC reducing
the per packet CPU overhead.

A guest using vhostuser interface with TSO enabled can send TCP packets
much bigger than the MTU, which saves CPU cycles normally used to break
the packets down to MTU size and to calculate checksums.

It also saves CPU cycles used to parse multiple packets/headers during
the packet processing inside virtual switch.

If the destination of the packet is another guest in the same host, then
the same big packet can be sent through a vhostuser interface skipping
the segmentation completely. However, if the destination is not local,
the NIC hardware is instructed to do the TCP segmentation and checksum
calculation.

It is recommended to check if NIC hardware supports TSO before enabling
the feature, which is off by default. For additional information please
check the tso.rst document.

Signed-off-by: Flavio Leitner 
---
 Documentation/automake.mk  |   1 +
 Documentation/topics/index.rst |   1 +
 Documentation/topics/userspace-tso.rst |  98 +++
 NEWS   |   1 +
 lib/automake.mk|   2 +
 lib/conntrack.c|  29 +-
 lib/dp-packet.h| 186 +++-
 lib/ipf.c  |  32 +-
 lib/netdev-dpdk.c  | 349 +++---
 lib/netdev-linux-private.h |   5 +
 lib/netdev-linux.c | 386 ++---
 lib/netdev-provider.h  |   9 +
 lib/netdev.c   |  78 -
 lib/userspace-tso.c|  48 +++
 lib/userspace-tso.h|  23 ++
 vswitchd/bridge.c  |   2 +
 vswitchd/vswitch.xml   |  17 ++
 17 files changed, 1143 insertions(+), 124 deletions(-)
 create mode 100644 Documentation/topics/userspace-tso.rst
 create mode 100644 lib/userspace-tso.c
 create mode 100644 lib/userspace-tso.h

Changelog:
- v4
 * rebased on top of master (recvmmsg)
 * fixed URL in doc to point to 19.11
 * renamed tso to userspace-tso
 * renamed the option to userspace-tso-enable
 * removed prototype that left over from v2
 * fixed function style declaration
 * renamed dp_packet_hwol_tx_ip_checksum to dp_packet_hwol_tx_ipv4_checksum
 * dp_packet_hwol_tx_ipv4_checksum now checks for PKT_TX_IPV4.
 * account for drops while preping the batch for TX.
 * don't prep the batch for TX if TSO is disabled.
 * simplified setsockopt error checking
 * fixed af_packet_sock error checking to not call setsockopt on
  closed sockets.
 * fixed ol_flags comment.
 * used VLOG_ERR_BUF() to pass error messages.
 * fixed packet leak at netdev_send_prepare_batch()
 * added a coverage counter to account drops while preparing a batch
   at netdev.c
 * fixed netdev_send() to not call ->send() if the batch is empty.
 * fixed packet leak at netdev_push_header and account for the drops.
 * removed DPDK requirement to enable userspace TSO support.
 * fixed parameter documentation in vswitch.xml.
 * renamed tso.rst to userspace-tso.rst and moved to topics/
 * added comments documeting the functions in dp-packet.h
 * fixed dp_packet_hwol_is_tso to check only PKT_TX_TCP_SEG

- v3
 * Improved the documentation.
 * Updated copyright year to 2020.
 * TSO offloaded msg now includes the netdev's name.
 * Added period at the end of all code comments.
 * Warn and drop encapsulation of TSO packets.
 * Fixed travis issue with restricted virtio types.
 * Fixed double headroom allocation in dpdk_copy_dp_packet_to_mbuf()
   which caused packet corruption.
 * Fixed netdev_dpdk_prep_hwol_packet() to unconditionally set
   PKT_TX_IP_CKSUM only for IPv4 packets.


diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index f2ca17bad..22976a3cd 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -57,6 +57,7 @@ DOC_SOURCE = \
Documentation/topics/ovsdb-replication.rst \
Documentation/topics/porting.rst \
Documentation/topics/tracing.rst \
+   Documentation/topics/userspace-tso.rst \
Documentation/topics/windows.rst \
Documentation/howto/index.rst \
Documentation/howto/dpdk.rst \
diff --git a/Documentation/topics/index.rst b/Documentation/topics/index.rst
index 34c4b10e0..08af3a24d 100644
--- a/Documentation/topics/index.rst
+++ b/Documentation/topics/index.rst
@@ -50,5 +50,6 @@ OVS
language-bindings
testing
tracing
+   userspace-tso
idl-compound-indexes
ovs-extensions
diff --git a/Documentation/topics/userspace-tso.rst 
b/Documentation/topics/userspace-tso.rst
new file mode 100644
index 0..893c64839
--- /dev/null
+++ b/Documentation/topics/userspace-tso.rst
@@ -0,0 +1,98 @@
+..
+  Copyright 2020, Red Hat, Inc.
+
+  Licensed under the Apache License, Version 2.0 (the "License"); you may
+  not 

[ovs-dev] [PATCH v4 1/3] dp-packet: preserve headroom when cloning a pkt batch

[Flavio Leitner]

The headroom is useful if the packet needs to insert additional
header, so preserve the original headroom when cloning the batch.

Signed-off-by: Flavio Leitner 
---
 lib/dp-packet.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 3dd59e25d..133942155 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -879,7 +879,11 @@ dp_packet_batch_clone(struct dp_packet_batch *dst,
 
 dp_packet_batch_init(dst);
 DP_PACKET_BATCH_FOR_EACH (i, packet, src) {
-dp_packet_batch_add(dst, dp_packet_clone(packet));
+uint32_t headroom = dp_packet_headroom(packet);
+struct dp_packet *pkt_clone;
+
+pkt_clone  = dp_packet_clone_with_headroom(packet, headroom);
+dp_packet_batch_add(dst, pkt_clone);
 }
 dst->trunc = src->trunc;
 }
-- 
2.24.1

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

[ovs-dev] [PATCH v4 2/3] vhost: Disable multi-segmented buffers

[Flavio Leitner]

There is no support for multi-segmented buffers, so flag
that to vhost library.

Signed-off-by: Flavio Leitner 
---
 lib/netdev-dpdk.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 72b67af1a..d1469f6f2 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1347,6 +1347,9 @@ netdev_dpdk_vhost_construct(struct netdev *netdev)
 dev->vhost_id = xasprintf("%s/%s", dpdk_get_vhost_sock_dir(), name);
 
 dev->vhost_driver_flags &= ~RTE_VHOST_USER_CLIENT;
+
+/* There is no support for multi-segments buffers. */
+dev->vhost_driver_flags |= RTE_VHOST_USER_LINEARBUF_SUPPORT;
 err = rte_vhost_driver_register(dev->vhost_id, dev->vhost_driver_flags);
 if (err) {
 VLOG_ERR("vhost-user socket device setup failure for socket %s\n",
@@ -4952,6 +4955,9 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev 
*netdev)
 /* Register client-mode device. */
 vhost_flags |= RTE_VHOST_USER_CLIENT;
 
+/* There is no support for multi-segments buffers. */
+vhost_flags |= RTE_VHOST_USER_LINEARBUF_SUPPORT;
+
 /* Enable IOMMU support, if explicitly requested. */
 if (dpdk_vhost_iommu_enabled()) {
 vhost_flags |= RTE_VHOST_USER_IOMMU_SUPPORT;
-- 
2.24.1

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

[ovs-dev] [PATCH v4 0/3] Add support for TSO with DPDK

[Flavio Leitner]

Abbreviated as TSO, TCP Segmentation Offload is a feature which enables
the network stack to delegate the TCP segmentation to the NIC reducing
the per packet CPU overhead.

A guest using vhost-user interface with TSO enabled can send TCP packets
much bigger than the MTU, which saves CPU cycles normally used to break
the packets down to MTU size and to calculate checksums.

It also saves CPU cycles used to parse multiple packets/headers during
the packet processing inside virtual switch.

If the destination of the packet is another guest in the same host, then
the same big packet can be sent through a vhost-user interface skipping
the segmentation completely. However, if the destination is not local,
the NIC hardware is instructed to do the TCP segmentation and checksum
calculation.

The first 2 patches are not really part of TSO support, but they are
required to make sure everything works.

There are good improvements sending to or receiving from veth pairs or
tap devices as well. See the iperf3 results below:

[*] veth with ethtool tx off.

VM sending to:  Default   Enabled Enabled/Default
   Local BR 3 Gbits/sec 23 Gbits/sec  7x
   Net NS (veth)3 Gbits/sec[*]  22 Gbits/sec  7x
   VM (same host) 2.5 Gbits/sec 24 Gbits/sec  9x
   Ext Host10 Gbits/sec 35 Gbits/sec  3x
   Ext Host (vxlan)   8.8 Gbits/sec (not supported) 

  Using VLAN:
   Local BR 3 Gbits/sec 23 Gbits/sec  7x
   VM (same host) 2.5 Gbits/sec 21 Gbits/sec  8x
   Ext Host   6.4 Gbits/sec 34 Gbits/sec  5x

  Using IPv6:
   Net NS (veth)  2.7 Gbits/sec[*]  22 Gbits/sec  8x
   VM (same host) 2.6 Gbits/sec 21 Gbits/sec  8x
   Ext Host   8.7 Gbits/sec 34 Gbits/sec  4x

  Conntrack:
   No packet changes: 1.41 Gbits/sec33 Gbits/sec  23x

VM receiving from:
   Local BR   2.5 Gbits/sec 2.4 Gbits/sec 1x
   Net NS (veth)  2.5 Gbits/sec[*]  9.3 Gbits/sec 3x
   VM (same host) 4.9 Gbits/sec  25 Gbits/sec 5x
   Ext Host   9.7 Gbits/sec 9.4 Gbits/sec 1x
   Ext Host (vxlan)   5.5 Gbits/sec (not supported)

  Using VLAN:
   Local BR   2.4 Gbits/sec 2.4 Gbits/sec 1x
   VM (same host) 3.8 Gbits/sec  24 Gbits/sec 8x
   Ext Host   9.5 Gbits/sec 9.5 Gbits/sec 1x

  Using IPv6:
   Net NS (veth)  2.2 Gbits/sec[*]   9 Gbits/sec  4x
   VM (same host) 4.5 Gbits/sec 24 Gbits/sec  5x
   Ext Host   8.9 Gbits/sec8.9 Gbits/sec  1x

Used iperf3 -u to test UDP traffic limited at default 1Mbits/sec
and noticed no change with the exception for tunneled packets (not
supported).

Travis, AppVeyor, and Cirrus-ci passed.

Flavio Leitner (3):
  dp-packet: preserve headroom when cloning a pkt batch
  vhost: Disable multi-segmented buffers
  netdev-dpdk: Add TCP Segmentation Offload support

 Documentation/automake.mk  |   1 +
 Documentation/topics/index.rst |   1 +
 Documentation/topics/userspace-tso.rst |  98 +++
 NEWS   |   1 +
 lib/automake.mk|   2 +
 lib/conntrack.c|  29 +-
 lib/dp-packet.h| 192 +++-
 lib/ipf.c  |  32 +-
 lib/netdev-dpdk.c  | 355 ---
 lib/netdev-linux-private.h |   5 +
 lib/netdev-linux.c | 386 ++---
 lib/netdev-provider.h  |   9 +
 lib/netdev.c   |  78 -
 lib/userspace-tso.c|  48 +++
 lib/userspace-tso.h|  23 ++
 vswitchd/bridge.c  |   2 +
 vswitchd/vswitch.xml   |  17 ++
 17 files changed, 1154 insertions(+), 125 deletions(-)
 create mode 100644 Documentation/topics/userspace-tso.rst
 create mode 100644 lib/userspace-tso.c
 create mode 100644 lib/userspace-tso.h

-- 
2.24.1

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

[ovs-dev] [PATCH AUTOSEL 5.4 096/205] net: openvswitch: don't unlock mutex when changing the user_features fails

[Sasha Levin]

From: Tonghao Zhang 

[ Upstream commit 4c76bf696a608ea5cc555fe97ec59a9033236604 ]

Unlocking of a not locked mutex is not allowed.
Other kernel thread may be in critical section while
we unlock it because of setting user_feature fail.

Fixes: 95a7233c4 ("net: openvswitch: Set OvS recirc_id from tc chain index")
Cc: Paul Blakey 
Signed-off-by: Tonghao Zhang 
Tested-by: Greg Rose 
Acked-by: William Tu 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
---
 net/openvswitch/datapath.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 23f67b8fdeaa..3eed90bfa2bf 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1667,6 +1667,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct 
genl_info *info)
ovs_dp_reset_user_features(skb, info);
}
 
+   ovs_unlock();
goto err_destroy_meters;
}
 
@@ -1683,7 +1684,6 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct 
genl_info *info)
return 0;
 
 err_destroy_meters:
-   ovs_unlock();
ovs_meters_exit(dp);
 err_destroy_ports_array:
kfree(dp->ports);
-- 
2.20.1

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

[ovs-dev] [PATCH] ofproto: Fix for frequent invalidation of flows due to mismatch in mask bits

[Vishal Deep Ajmera via dev]

The wildcard bits in the installed megaflow entry could be different from
the bits originally generated by the ofproto layer. Datapath implementation
wildcards those match fields which are not present in the incoming packet
before installing the flow.

When the revalidator thread validates a megaflow, it will first query the
ofproto layer to get the wildcard bits and then compare it against the
wildcard bits in the megaflow. If the bits are different the entry will be
removed. A subsequent packet will again result in the same megaflow entry
being installed only for it to be removed by the revalidator thread. This
cycle will continue and will significantly degrade performance.

An earlier patch fixing the issue for MPLS and VLAN was sent.
However similar problem now appears for IPv6 datapath flows.

This patch addresses the issue in a generic way.

Signed-off-by: Vishal Deep Ajmera 
---
 ofproto/ofproto-dpif-upcall.c | 14 +-
 ofproto/ofproto-dpif-xlate.c  | 20 
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 409286a..1371486 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2169,7 +2169,7 @@ revalidate_ukey__(struct udpif *udpif, const struct 
udpif_key *ukey,
 {
 struct xlate_out *xoutp;
 struct netflow *netflow;
-struct flow_wildcards dp_mask, wc;
+struct flow_wildcards dp_mask, wc, wc_from_flow;
 enum reval_result result;
 struct reval_context ctx = {
 .odp_actions = odp_actions,
@@ -2215,6 +2215,18 @@ revalidate_ukey__(struct udpif *udpif, const struct 
udpif_key *ukey,
 goto exit;
 }
 
+/* Clear flow wildcard bits for fields which are not present
+ * in the original packet header. These wildcards may get set
+ * due to push/set_field actions. This results into frequent
+ * invalidation of datapath flows by revalidator thread. */
+
+/* Create wc mask based on incoming packet. */
+flow_wildcards_init_for_packet(_from_flow,
+   );
+
+/* Clear mask fields in ctx which are not relevant for packet. */
+flow_wildcards_and(ctx.wc, _from_flow, ctx.wc);
+
 /* Do not modify if any bit is wildcarded by the installed datapath flow,
  * but not the newly revalidated wildcard mask (wc), i.e., if revalidation
  * tells that the datapath flow is now too generic and must be narrowed
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 4407f9c..42fdb9f 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -7363,26 +7363,6 @@ xlate_wc_finish(struct xlate_ctx *ctx)
 ctx->wc->masks.tp_src = 0;
 ctx->wc->masks.tp_dst = 0;
 }
-
-/* Clear flow wildcard bits for fields which are not present
- * in the original packet header. These wildcards may get set
- * due to push/set_field actions. This results into frequent
- * invalidation of datapath flows by revalidator thread. */
-
-/* Clear mpls label wc bits if original packet is non-mpls. */
-if (!eth_type_mpls(ctx->xin->upcall_flow->dl_type)) {
-for (i = 0; i < FLOW_MAX_MPLS_LABELS; i++) {
-ctx->wc->masks.mpls_lse[i] = 0;
-}
-}
-/* Clear vlan header wc bits if original packet does not have
- * vlan header. */
-for (i = 0; i < FLOW_MAX_VLAN_HEADERS; i++) {
-if (!eth_type_vlan(ctx->xin->upcall_flow->vlans[i].tpid)) {
-ctx->wc->masks.vlans[i].tpid = 0;
-ctx->wc->masks.vlans[i].tci = 0;
-}
-}
 }
 
 /* Translates the flow, actions, or rule in 'xin' into datapath actions in
-- 
1.9.1

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

Re: [ovs-dev] [PATCH v2 ovn 2/3] Forwarding group to load balance l2 traffic with liveness detection

[Numan Siddique]

On Sat, Jan 11, 2020 at 6:11 AM Manoj Sharma 
wrote:

> Add forwarding group support for a logical switch. It will add a new OVN
> action "fwd_group" with a virtual IP and virtual MAC. Any number of logical
> switch ports of this switch can be added to this forwarding group.
> If traffic has to be load balanced across these logical switch ports then
> traffic should be sent to forwarding group's virtual IP.
>
> If the logical switch ports correspond to tunnel interfaces and BFD
> can be enabled on these tunnel interfaces, then liveness can be enabled
> for the forwarding group so that if any of the tunnel is down then
> the corresponding logical switch port will not be selected during
> load balancing.
>
> Signed-off-by: Manoj Sharma 
>

Hi Manoj,

As I mentioned in the earlier email, please add the detailed description in
patch 0 to this patch.

How about defining the action - fwd_group as - fwd_group(liveliness=true,
childports=p1,p2,...) ?

Normally in the OVN action, we use ";" for inner OpenFlow actions, but
these are more like arguments
to the action - fwd_group.

In your setup, you would manually set the child port's chassis yourself ?
i.e ovn-sbctl lsp-bindright ?

You need to add the relevant documentation in ovn-northd.8.xml for the
logical flows added

You need to add documentation for the action - fwd_group in ovn-sb.xml.

Also please update the NEWS file about this feature.

Please see below for few comments.

---
>  controller/lflow.c|  20 +
>  controller/physical.c |  13 ++
>  controller/physical.h |   4 ++
>  include/ovn/actions.h |  19 +++-
>  lib/actions.c | 122
> ++
>  northd/ovn-northd.c   |  63 ++
>  utilities/ovn-trace.c |   3 ++
>  7 files changed, 243 insertions(+), 1 deletion(-)
>
> diff --git a/controller/lflow.c b/controller/lflow.c
> index a689320..49dfa06 100644
> --- a/controller/lflow.c
> +++ b/controller/lflow.c
> @@ -103,6 +103,25 @@ lookup_port_cb(const void *aux_, const char
> *port_name, unsigned int *portp)
>  return false;
>  }
>
> +/* Given the OVN port name, get its openflow port */
> +static bool
> +tunnel_ofport_cb(const void *aux_, const char *port_name, ofp_port_t
> *ofport)
> +{
> +const struct lookup_port_aux *aux = aux_;
> +
> +const struct sbrec_port_binding *pb
> += lport_lookup_by_name(aux->sbrec_port_binding_by_name,
> port_name);
> +if (!pb || (pb->datapath != aux->dp) || !pb->chassis) {
> +return false;
> +}
> +
> +if (!get_tunnel_ofport(pb->chassis->name, NULL, ofport)) {
> +return false;
> +}
> +
> +return true;
> +}
> +
>  static bool
>  is_chassis_resident_cb(const void *c_aux_, const char *port_name)
>  {
> @@ -681,6 +700,7 @@ consider_logical_flow(
>  struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
>  struct ovnact_encode_params ep = {
>  .lookup_port = lookup_port_cb,
> +.tunnel_ofport = tunnel_ofport_cb,
>  .aux = ,
>  .is_switch = is_switch(ldp),
>  .group_table = group_table,
> diff --git a/controller/physical.c b/controller/physical.c
> index 500d419..af1d10f 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -1794,3 +1794,16 @@ physical_run(struct ovsdb_idl_index
> *sbrec_port_binding_by_name,
>
>  simap_destroy(_tunnel_to_ofport);
>  }
> +
> +bool
> +get_tunnel_ofport(const char *chassis_name, char *encap_ip, ofp_port_t
> *ofport)
> +{
> +struct chassis_tunnel *tun = NULL;
> +tun = chassis_tunnel_find(chassis_name, encap_ip);
> +if (!tun) {
> +return false;
> +}
> +
> +*ofport = tun->ofport;
> +return true;
> +}
> diff --git a/controller/physical.h b/controller/physical.h
> index c93f6b1..c0e17cd 100644
> --- a/controller/physical.h
> +++ b/controller/physical.h
> @@ -72,4 +72,8 @@ void physical_handle_mc_group_changes(
>  const struct simap *ct_zones,
>  const struct hmap *local_datapaths,
>  struct ovn_desired_flow_table *);
> +bool get_tunnel_ofport(
> +const char *chassis_name,
> +char *encap_ip,
> +ofp_port_t *ofport);
>  #endif /* controller/physical.h */
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index 047a8d7..2d39239 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -89,7 +89,8 @@ struct ovn_extend_table;
>  OVNACT(CHECK_PKT_LARGER,  ovnact_check_pkt_larger) \
>  OVNACT(TRIGGER_EVENT, ovnact_controller_event) \
>  OVNACT(BIND_VPORT,ovnact_bind_vport)   \
> -OVNACT(HANDLE_SVC_CHECK,  ovnact_handle_svc_check)
> +OVNACT(HANDLE_SVC_CHECK,  ovnact_handle_svc_check) \
> +OVNACT(FWD_GROUP, ovnact_fwd_group)
>
>  /* enum ovnact_type, with a member OVNACT_ for each action. */
>  enum OVS_PACKED_ENUM ovnact_type {
> @@ -359,6 +360,15 @@ struct ovnact_handle_svc_check {
>  struct expr_field port; /* Logical port name. */
>  

[ovs-dev] [PATCH ovn 2/2] ovn-northd: Support hairpinning for logical switch load balancing.

[Dumitru Ceara]

In case a VIF is trying to connect to a load balancer VIP that includes in
its backends the VIF itself, traffic would get DNAT-ed, ct_lb(VIP), but
when it reaches the VIF, the VIF will try to reply locally as the source IP
is known to be local. For this kind of hairpinning to work properly, reply
traffic must be sent back through OVN and the way to enforce that is to
perform SNAT (VIF source IP -> VIP) on hairpinned packets.

For load balancers configured on gateway logical routers we already have the
possibility of using 'lb_force_snat_ip' but for load balancers configured
on logical switches there's no such configuration.

For this second case we take an automatic approach which determines if
load balanced traffic needs to be hairpinned and execute the SNAT. To achieve
this, two new stages are added to the logical switch ingress pipeline:
- Ingress Table 11: Pre-Hairpin: which matches on load balanced traffic
  coming from VIFs that needs to be hairpinned and sets REGBIT_HAIRPIN
  (reg0[6]) to 1. If the traffic is in the direction that initiated the
  connection then 'ct_snat(VIP)' is performed, otherwise 'ct_snat' is
  used to unSNAT replies.
- Ingress Table 12: Hairpin: which hairpins packets at L2 (swaps Ethernet
  addresses and loops traffic back on the ingress port) if REGBIT_HAIRPIN
  is 1.

Also, update all references to logical switch ingress pipeline tables to use
the correct indices.

Reported-at: https://github.com/ovn-org/ovn-kubernetes/issues/817
Signed-off-by: Dumitru Ceara 
---
 northd/ovn-northd.8.xml   |   57 --
 northd/ovn-northd.c   |  260 ++---
 tests/ovn.at  |  209 
 utilities/ovn-trace.8.xml |4 -
 4 files changed, 406 insertions(+), 124 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 4b227ca..3e7315c 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -527,7 +527,40 @@
   
 
 
-Ingress Table 11: ARP/ND responder
+Ingress Table 11: Pre-Hairpin
+
+  
+For all configured load balancer backends a priority-2 flow that
+matches on traffic that needs to be hairpinned, i.e., after load
+balancing the destination IP matches the source IP, which sets
+reg0[6] = 1  and executes ct_snat(VIP)
+to force replies to these packets to come back through OVN.
+  
+  
+For all configured load balancer backends a priority-1 flow that
+matches on replies to hairpinned traffic, i.e., destination IP is VIP,
+source IP is the backend IP and source L4 port is backend port, which
+sets reg0[6] = 1  and executes ct_snat;.
+  
+  
+A priority-0 flow that simply moves traffic to the next table.
+  
+
+
+Ingress Table 12: Hairpin
+
+  
+A priority-1 flow that hairpins traffic matched by non-default
+flows in the Pre-Hairpin table. Hairpinning is done at L2, Ethernet
+addresses are swapped and the packets are looped back on the input
+port.
+  
+  
+A priority-0 flow that simply moves traffic to the next table.
+  
+
+
+Ingress Table 13: ARP/ND responder
 
 
   This table implements ARP/ND responder in a logical switch for known
@@ -772,7 +805,7 @@ output;
   
 
 
-Ingress Table 12: DHCP option processing
+Ingress Table 14: DHCP option processing
 
 
   This table adds the DHCPv4 options to a DHCPv4 packet from the
@@ -829,11 +862,11 @@ next;
   
 
   
-A priority-0 flow that matches all packets to advances to table 11.
+A priority-0 flow that matches all packets to advances to table 15.
   
 
 
-Ingress Table 13: DHCP responses
+Ingress Table 15: DHCP responses
 
 
   This table implements DHCP responder for the DHCP replies generated by
@@ -911,11 +944,11 @@ output;
   
 
   
-A priority-0 flow that matches all packets to advances to table 12.
+A priority-0 flow that matches all packets to advances to table 16.
   
 
 
-Ingress Table 14 DNS Lookup
+Ingress Table 16 DNS Lookup
 
 
   This table looks up and resolves the DNS names to the corresponding
@@ -944,7 +977,7 @@ reg0[4] = dns_lookup(); next;
   
 
 
-Ingress Table 15 DNS Responses
+Ingress Table 17 DNS Responses
 
 
   This table implements DNS responder for the DNS replies generated by
@@ -979,7 +1012,7 @@ output;
   
 
 
-Ingress table 16 External ports
+Ingress table 18 External ports
 
 
   Traffic from the external logical ports enter the ingress
@@ -1007,11 +1040,11 @@ output;
   
 
   
-A priority-0 flow that matches all packets to advances to table 17.
+A priority-0 flow that matches all packets to advances to table 19.
   
 
 
-Ingress Table 17 Destination Lookup
+Ingress Table 19 

[ovs-dev] [PATCH ovn 1/2] ovn-northd: Fix Pre-LB logical flows with IPv4 and IPv6.

[Dumitru Ceara]

When both IPv4 and IPv6 load balancers were configured, the logical flows
that send packets that need to be load balanced to conntrack (for
defragmentation) were incorrectly trying to match on IPv6 fields.

Fix this issue by using two address sets, one for IPv4 and the other for
IPv6.

Signed-off-by: Dumitru Ceara 
---
 northd/ovn-northd.c |   29 +
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index b6dc809..4ac9668 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -4654,13 +4654,14 @@ build_pre_lb(struct ovn_datapath *od, struct hmap 
*lflows,
 ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 0, "1", "next;");
 ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 0, "1", "next;");
 
-struct sset all_ips = SSET_INITIALIZER(_ips);
+struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
+struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
 bool vip_configured = false;
-int addr_family = AF_INET;
 for (int i = 0; i < od->nbs->n_load_balancer; i++) {
 struct nbrec_load_balancer *lb = od->nbs->load_balancer[i];
 struct smap *vips = >vips;
 struct smap_node *node;
+int addr_family = AF_INET;
 
 SMAP_FOR_EACH (node, vips) {
 vip_configured = true;
@@ -4674,8 +4675,10 @@ build_pre_lb(struct ovn_datapath *od, struct hmap 
*lflows,
 continue;
 }
 
-if (!sset_contains(_ips, ip_address)) {
-sset_add(_ips, ip_address);
+if (addr_family == AF_INET) {
+sset_add(_ips_v4, ip_address);
+} else {
+sset_add(_ips_v6, ip_address);
 }
 
 build_empty_lb_event_flow(od, lflows, node, ip_address, lb,
@@ -4694,20 +4697,22 @@ build_pre_lb(struct ovn_datapath *od, struct hmap 
*lflows,
 /* 'REGBIT_CONNTRACK_DEFRAG' is set to let the pre-stateful table send
  * packet to conntrack for defragmentation. */
 const char *ip_address;
-SSET_FOR_EACH(ip_address, _ips) {
-char *match;
+SSET_FOR_EACH (ip_address, _ips_v4) {
+char *match = xasprintf("ip && ip4.dst == %s", ip_address);
+ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB,
+  100, match, REGBIT_CONNTRACK_DEFRAG" = 1; next;");
+free(match);
+}
 
-if (addr_family == AF_INET) {
-match = xasprintf("ip && ip4.dst == %s", ip_address);
-} else {
-match = xasprintf("ip && ip6.dst == %s", ip_address);
-}
+SSET_FOR_EACH (ip_address, _ips_v6) {
+char *match = xasprintf("ip && ip6.dst == %s", ip_address);
 ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB,
   100, match, REGBIT_CONNTRACK_DEFRAG" = 1; next;");
 free(match);
 }
 
-sset_destroy(_ips);
+sset_destroy(_ips_v4);
+sset_destroy(_ips_v6);
 
 if (vip_configured) {
 ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB,

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

Re: [ovs-dev] [PATCH v2 ovn 1/3] Forwarding group to load balance l2 traffic with liveness detection

[Numan Siddique]

On Sat, Jan 11, 2020 at 6:11 AM Manoj Sharma 
wrote:

> Add a forwarding group table and a reference to the logical switch it is
> configured on. The forwarding group is configured with a virtual IP,
> virtual
> MAC and a number of logical switch ports from a logical switch.
>
> Signed-off-by: Manoj Sharma 
>

This patch has below checkpatch issues. Can you please fix them.


== Checking 198cdb9bab38 ("Forwarding group to load balance l2 traffic with
liveness detection") ==
WARNING: Line is 149 characters long (recommended limit is 79)
#125 FILE: utilities/ovn-nbctl.8.xml:489:
  [--liveness]fwd-group-add
group switch vip vmac
ports

WARNING: Line is 85 characters long (recommended limit is 79)
#146 FILE: utilities/ovn-nbctl.8.xml:510:
  [--if-exists] fwd-group-del
group

WARNING: Line lacks whitespace around operator
#172 FILE: utilities/ovn-nbctl.c:653:
  fwd-group-add GROUP SWITCH VIP VMAC PORTS...\n\

WARNING: Line lacks whitespace around operator
#174 FILE: utilities/ovn-nbctl.c:655:
  fwd-group-del GROUP   delete a forwarding group\n\

WARNING: Line lacks whitespace around operator
#175 FILE: utilities/ovn-nbctl.c:656:
  fwd-group-list [SWITCH]   print forwarding groups\n\

ERROR: Improper whitespace around control block
#196 FILE: utilities/ovn-nbctl.c:4742:
NBREC_FORWARDING_GROUP_FOR_EACH(fwd_group, ctx->idl) {

ERROR: Improper whitespace around control block
#392 FILE: utilities/ovn-nbctl.c:4938:
NBREC_FORWARDING_GROUP_FOR_EACH(fwd_group, ctx->idl) {
**

I think you can ignore the warnings in ovn-nbctl.c.

You can run these checks your self -  "utilities/checkpatch.py -1"

You need to bump  up the version in ovn-nb.ovsschema to 5.19.0

Also please move the relevant test cases from patch 3 to this one.

Thanks
Numan




> ---
>  ovn-nb.ovsschema  |  18 +++-
>  ovn-nb.xml|  35 +++
>  utilities/ovn-nbctl.8.xml |  37 +++
>  utilities/ovn-nbctl.c | 253
> ++
>  4 files changed, 341 insertions(+), 2 deletions(-)
>
> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> index 12999a4..99b6285 100644
> --- a/ovn-nb.ovsschema
> +++ b/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>  "name": "OVN_Northbound",
>  "version": "5.18.0",
> -"cksum": "2806349485 24196",
> +"cksum": "63300136 24879",
>  "tables": {
>  "NB_Global": {
>  "columns": {
> @@ -59,7 +59,12 @@
>   "min": 0, "max": "unlimited"}},
>  "external_ids": {
>  "type": {"key": "string", "value": "string",
> - "min": 0, "max": "unlimited"}}},
> + "min": 0, "max": "unlimited"}},
> +   "forwarding_groups": {
> +"type": {"key": {"type": "uuid",
> + "refTable": "Forwarding_Group",
> + "refType": "strong"},
> + "min": 0, "max": "unlimited"}}},
>  "isRoot": true},
>  "Logical_Switch_Port": {
>  "columns": {
> @@ -113,6 +118,15 @@
>   "min": 0, "max": "unlimited"}}},
>  "indexes": [["name"]],
>  "isRoot": false},
> +"Forwarding_Group": {
> +"columns": {
> +"name": {"type": "string"},
> +"vip": {"type": "string"},
> +"vmac": {"type": "string"},
> +"liveness": {"type": "boolean"},
> +"child_port": {"type": {"key": "string",
> +"min": 1, "max": "unlimited"}}},
> +"isRoot": false},
>  "Address_Set": {
>  "columns": {
>  "name": {"type": "string"},
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 5ae52bb..decb4ae 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -197,6 +197,11 @@
>Please see the  table.
>  
>
> +
> +  Groups a set of logical port endpoints for traffic going out of the
> +  logical switch.
> +
> +
>  
>
>  These columns provide names for the logical switch.  From OVN's
> @@ -1152,6 +1157,36 @@
>  
>
>
> +  
> +
> +  Each row represents one forwarding group.
> +
> +
> +
> +  A name for the forwarding group.  This name has no special meaning
> or
> +  purpose other than to provide convenience for human interaction with
> +  the ovn-nb database.
> +
> +
> +
> +  The virtual IP address assigned to the forwarding group. It will
> respond
> +  with vmac when an ARP request is sent for vip.
> +
> +
> +
> +  The virtual MAC address assigned to the forwarding group.
> +
> +
> +
> +  If set to true, liveness is enabled for child ports
> +  otherwise it is disabled.
> +
> +
> +
> +  List of child ports in the forwarding group.
> +
> +  
> +
>
>  
> 

[ovs-dev] [PATCH ovn 0/2] Support hairpinning for logical switch load balancing.

[Dumitru Ceara]

The first patch of the series fixes Pre-LB logical flows when both IPv4
and IPv6 load balancers are configured.

The second patch of the series adds support for correctly hairpinning
traffic that has been load balanced on a logical switch and the chosen
backend is actually the source of the traffic.

Reported-at: https://github.com/ovn-org/ovn-kubernetes/issues/817
Signed-off-by: Dumitru Ceara 

Dumitru Ceara (2):
  ovn-northd: Fix Pre-LB logical flows with IPv4 and IPv6.
  ovn-northd: Support hairpinning for logical switch load balancing.


 northd/ovn-northd.8.xml   |   57 +++--
 northd/ovn-northd.c   |  289 ++---
 tests/ovn.at  |  209 +
 utilities/ovn-trace.8.xml |4 -
 4 files changed, 423 insertions(+), 136 deletions(-)


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

Re: [ovs-dev] [PATCH v2 ovn 0/3] Forwarding group to load balance l2 traffic with liveness detection

[Numan Siddique]

On Sat, Jan 11, 2020 at 6:11 AM Manoj Sharma 
wrote:

> A forwarding group is an aggregation of logical switch ports of a
> logical switch to load balance traffic across the ports. It also detects
> the liveness if the logical switch ports are realized as OVN tunnel ports
> on the physical topology.
>
> In the below logical topology diagram, the logical switch has two ports
> connected to chassis / external routers R1 and R2. The logical router needs
> to send traffic to an external network that is connected through R1 and R2.
>
> ++
>  +--+ R1 |*
> /   ++  ** **
>   +--++--+ / lsp1  * *
>   | Logical  ||   Logical|/   * External  *
>   | Router   ++   switch X*  Network  *
>   |  ||  |\   *   *
>   +--++--+ \ lsp2  * *
>  ^  \   ++  ** **
>  |   +--+ R2 |*
>  |  ++
>fwd_group -> (lsp1, lsp2)
>
> In the absence of forwarding group, the logical router will have unicast
> route to point to either R1 or R2. In case of R1 or R2 going down, it will
> require control plane's intervention to update the route to point to proper
> nexthop.
>
> With forwarding group, a virtual IP (VIP) and virtual MAC (VMAC) address
> are configured on the forwarding group. The logical router points to the
> forwarding group's VIP as the nexthop for hosts behind R1 and R2.
>
> [root@fwd-group]# ovn-nbctl fwd-group-add fwd ls1 VIP_1 VMAC_1 lsp1 lsp2
>
> [root@fwd-group]# ovn-nbctl fwd-group-list
> UUIDFWD_GROUP  VIPVMAC   CHILD_PORTS
> UUID_1fwd VIP_1  VMAC_1   lsp1 lsp2
>
> [root@fwd-group]# ovn-nbctl lr-route-list lr1
> IPv4 Routes
> external_host_prefix/prefix_lenVIP_1 dst-ip
>
> The logical switch will install an ARP responder rule to reply with VMAC
> as the MAC address for ARP requests for VIP. It will also install a MAC
> lookup rule for VMAC with action to load balance across the logical switch
> ports of the forwarding group.
>
> Datapath: "ls1" Pipeline: ingress
> table=10(ls_in_arp_rsp  ), priority=50   , match=(arp.tpa == VIP_1 &&
> arp.op == 1), action=(eth.dst = eth.src; eth.src = VMAC_1; arp.op = 2;
> /* ARP reply */ arp.tha = arp.sha; arp.sha = VMAC_1; arp.tpa = arp.spa;
> arp.spa = VIP; outport = inport; flags.loopback = 1; output;)
>
> table=13(ls_in_l2_lkup  ), priority=50   , match=(eth.dst == VMAC_1),
> action=(fwd_group("lsp1","lsp2");)
>
> In the physical topology, OVN managed hypervisors are connected to R1 and
> R2 through overlay tunnels. The logical flow's "fwd_group" action mentioned
> above, gets translated to openflow group type "select" with one bucket for
> each logical switch port.
>
> cookie=0x0, duration=16.869s, table=29, n_packets=4, n_bytes=392,
> idle_age=0,
> priority=111,metadata=0x9,dl_dst=VMAC_1 actions=group:1
>
> group_id=1,type=select,selection_method=dp_hash,
> bucket=actions=load:0x2->NXM_NX_REG15[0..15], resubmit(,32),
> bucket=actions=load:0x3->NXM_NX_REG15[0..15],resubmit(,32)
>
> where 0x2 and 0x3 are port tunnel keys of lsp1 and lsp2.
>
> The openflow group type "select" with selection method "dp_hash" load
> balances traffic based on source and destination Ethernet address, VLAN ID,
> Ethernet type, IPv4/v6 source and destination address and protocol, and for
> TCP and SCTP only, the source and destination ports.
>
> To detect path failure between OVN managed hypervisors and (R1, R2), BFD is
> enabled on the tunnel interfaces. The openflow group is modified to include
> watch_port for liveness detection of a port.
>
> group_id=1,type=select,selection_method=dp_hash,
>   bucket=watch_port:31,actions=load:0x2->NXM_NX_REG15[0..15],resubmit(,32),
>   bucket=watch_port:32,actions=load:0x3->NXM_NX_REG15[0..15],resubmit(,32)
>
> Where 31 and 32 are ovs port numbers for the tunnel interfaces connecting
> to R1 and R2.
>
> If the BFD forwarding status is down for any of the tunnels, the
> corresponding bucket will not be selected for packet forwarding.
>
>
Hi Manoj,

Thanks for the explanation in the previous message.

Regarding the commit message, I think I was not clear.

Can you please include the above description (with the ascii diagram) in
the commit
message of patch 2 of this series ?

I think you can drop patch 3 and instead include the test cases added to
ovn-nbctl.at into patch 1
and the rest in patch 2.


I have some comments about the series which I will reply separately.

Thanks
Numan


> Manoj Sharma (3):
>   Schema changes for adding forwarding_group 

Re: [ovs-dev] [PATCH v2] tc: handle packet mark of zero

[Aaron Conole]

Simon Horman  writes:

> On Thu, Jan 16, 2020 at 04:59:54AM -0500, 0-day Robot wrote:
>> Bleep bloop.  Greetings Simon Horman, 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: Unexpected sign-offs from developers who are not authors or
>> co-authors or committers: Simon Horman 
>> Lines checked: 44, Warnings: 1, Errors: 0
>
> Is the correct tag Co-Authored-by rather than Co-Authored as used in
> this patch?

Yes.

Co-authored-by

Not sure if we should make the tags a bit more lenient, but that's a
separate discussion. :)

>> 
>> 
>> Please check this out.  If you feel there has been an error, please email 
>> acon...@redhat.com
>> 
>> Thanks,
>> 0-day Robot
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>> 

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

Re: [ovs-dev] [PATCH] Documentation: Fix building with Python 3.9

[Flavio Leitner]

On Thu, Jan 16, 2020 at 02:21:47PM +0100, Timothy Redaelli wrote:
> open(), io.open(), codecs.open() and fileinput.FileInput no longer accept 'U'
> ("universal newline") in the file mode.
> This flag was deprecated since Python 3.3.
> In Python 3, the "universal newline" is used by default when a file is open
> in text mode.
> 
> Reported-at: https://bugzilla.redhat.com/1791681
> Reported-by: Miro Hrončok 
> Signed-off-by: Timothy Redaelli 
> ---

LGTM
Acked-by: Flavio Leitner 

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

Re: [ovs-dev] [v4] dpif-netdev: Modified ovs-appctl dpctl/dump-flows command

[Ilya Maximets]

On 16.01.2020 15:09, Finn, Emma wrote:
> Comments inline.
> 
>> -Original Message-
>> From: Finn, Emma 
>> Sent: Thursday 16 January 2020 11:45
>> To: d...@openvswitch.org
>> Cc: i.maxim...@ovn.org; ian.sto...@intel.org; Finn, Emma
>> 
>> Subject: [v4] dpif-netdev: Modified ovs-appctl dpctl/dump-flows command
>>
>> Modified ovs-appctl dpctl/dump-flows command to output the miniflow bits
>> for a given flow when -m option is passed.
>>
>> $ ovs-appctl dpctl/dump-flows -m
>>
>> Signed-off-by: Emma Finn 
>>
>> ---
>>
>> RFC -> v1
>>
>> * Changed revision from RFC to v1
>> * Reformatted based on comments
>> * Fixed same classifier being dumped multiple times
>>   flagged by Ilya
>> * Fixed print of subtables flagged by William
>> * Updated print count of bits as well as bits themselves
>>
>> ---
>>
>> v1 -> v2
>>
>> * Reformatted based on comments
>> * Refactored code to make output part of ovs-appctl
>>   dpctl/dump-flows -m command.
>>
>> ---
>>
>> v2 -> v3
>>
>> * Added attribute dp_extra_info to dpif_flow_attrs struct
>>   to store miniflow bits as a string
>>
>> ---
>>
>> v3 -> v4
>>
>> * Fixed string leak
>> * Refactored to code to make it  independent from the flowmap size
>> ---
> 
> Any other feedback? 
> I will have to rebase a send v5.

I didn't test this.  But since you're rebasing, couple of style
coments inline.

> 
> Regards, 
> Emma
> 
>>  NEWS  |  2 ++
>>  lib/dpctl.c   |  5 +
>>  lib/dpif-netdev.c | 14 ++
>>  lib/dpif.h|  1 +
>>  4 files changed, 22 insertions(+)
>>
>> diff --git a/NEWS b/NEWS
>> index 965faca..1c9d2db 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -8,6 +8,8 @@ Post-v2.12.0
>>   * Add option to enable, disable and query TCP sequence checking in
>> conntrack.
>>   * Add support for conntrack zone limits.
>> + * Command "ovs-appctl dpctl/dump-flows" refactored to show subtable
>> +   miniflow bits for userspace datapath.
>> - AF_XDP:
>>   * New option 'use-need-wakeup' for netdev-afxdp to control enabling
>> of corresponding 'need_wakeup' flag in AF_XDP rings.  Enabled by
>> default diff --git a/lib/dpctl.c b/lib/dpctl.c index a1ea25b..1b0a2bf 100644
>> --- a/lib/dpctl.c
>> +++ b/lib/dpctl.c
>> @@ -825,6 +825,11 @@ format_dpif_flow(struct ds *ds, const struct
>> dpif_flow *f, struct hmap *ports,
>>  }
>>  ds_put_cstr(ds, ", actions:");
>>  format_odp_actions(ds, f->actions, f->actions_len, ports);
>> +if (dpctl_p->verbosity && f->attrs.dp_extra_info) {
>> +ds_put_format(ds, ", dp-extra-info:%s",
>> +  f->attrs.dp_extra_info);

This should fit in a single line. Doesn't it?

>> +}
>> +free(f->attrs.dp_extra_info);
>>  }
>>
>>  struct dump_types {
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 079bd1b..a640b49
>> 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -3101,6 +3101,20 @@ dp_netdev_flow_to_dpif_flow(const struct
>> dp_netdev_flow *netdev_flow,
>>
>>  flow->attrs.offloaded = false;
>>  flow->attrs.dp_layer = "ovs";
>> +
>> +struct ds extra_info = DS_EMPTY_INITIALIZER;
>> +size_t unit;
>> +
>> +ds_put_cstr(_info, "miniflow_bits("); FLOWMAP_FOR_EACH_UNIT
>> (unit) {

Something bad happened here so lines was concatenated.

>> +if (unit) {
>> +ds_put_char(_info, ',');
>> +}
>> +ds_put_format(_info, "%d",
>> +  count_1bits(netdev_flow->cr.mask->mf.map.bits[unit]));
>> +}
>> +ds_put_char(_info, ')');
>> +flow->attrs.dp_extra_info = ds_steal_cstr(_info);
>> +ds_destroy(_info);
>>  }
>>
>>  static int
>> diff --git a/lib/dpif.h b/lib/dpif.h
>> index c21e897..59d82dc 100644
>> --- a/lib/dpif.h
>> +++ b/lib/dpif.h
>> @@ -513,6 +513,7 @@ struct dpif_flow_detailed_stats {  struct
>> dpif_flow_attrs {
>>  bool offloaded; /* True if flow is offloaded to HW. */
>>  const char *dp_layer;   /* DP layer the flow is handled in. */
>> +char *dp_extra_info;/* Extra information provided by DP. */
>>  };
>>
>>  struct dpif_flow_dump_types {
>> --
>> 2.7.4
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v6 ovn 2/2] northd: add logical flows for dhcpv6 pfd parsing

[Numan Siddique]

On Thu, Jan 16, 2020 at 7:40 PM Numan Siddique  wrote:
>
> On Tue, Jan 14, 2020 at 4:38 PM Lorenzo Bianconi
>  wrote:
> >
> > Introduce logical flows in ovn router pipeline in order to parse dhcpv6
> > advertise/reply from IPv6 prefix delegation router.
> > Do not overwrite ipv6_ra_pd_list info in options column of SB port_binding
> > table written by ovn-controller
> > Introduce ipv6_prefix column in NB Logical_router_port table to report
> > IPv6 prefix received from delegation router to the CMS
> >
> > Signed-off-by: Lorenzo Bianconi 
>
> Hi Lorenzo,
>
> Please see below for few comments.
>
> > ---
> >  northd/ovn-northd.c |  99 +-
> >  ovn-nb.ovsschema|   5 +-
> >  ovn-nb.xml  |  22 
> >  tests/atlocal.in|   5 +-
> >  tests/system-ovn.at | 127 
> >  5 files changed, 255 insertions(+), 3 deletions(-)
> >
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index b6dc809d7..031792706 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -2645,6 +2645,41 @@ copy_gw_chassis_from_nbrp_to_sbpb(
> >  free(sb_ha_chassis);
> >  }
> >
> > +static void
> > +ovn_port_update_ipv6_prefix(struct northd_context *ctx,
> > +const struct ovn_port *op,
> > +struct smap *sb_option)
> > +{
> > +const char *ipv6_pd_list = smap_get(>sb->options, 
> > "ipv6_ra_pd_list");
> > +if (!ipv6_pd_list) {
> > +return;
> > +}
> > +
> > +smap_add(sb_option, "ipv6_ra_pd_list", ipv6_pd_list);
> > +
> > +const struct nbrec_logical_router_port *lrp = NULL, *iter;
> > +/* update logical_router_port table */
> > +NBREC_LOGICAL_ROUTER_PORT_FOR_EACH (iter, ctx->ovnnb_idl) {
> > +if (!strcmp(iter->name, op->sb->logical_port)) {
> > +lrp = iter;
> > +break;
> > +}
> > +}
>
> I don't think you need to run this for loop to get the lrp. I think
> "struct ovn_port" should have a
> pointer to "struct nbrec_logical_router_port".
>
>
> > +if (!lrp) {
> > +return;
> > +}
> > +
> > +struct sset ipv6_prefix_set = SSET_INITIALIZER(_prefix_set);
> > +sset_add_array(_prefix_set, lrp->ipv6_prefix, lrp->n_ipv6_prefix);
> > +if (!sset_contains(_prefix_set, ipv6_pd_list)) {
> > +sset_add(_prefix_set, ipv6_pd_list);
> > +nbrec_logical_router_port_set_ipv6_prefix(lrp,
> > +sset_array(_prefix_set),
> > +sset_count(_prefix_set));
> > +}
>
> I think it's better to just copy whatever is there in the
> port_binding.options:ipv6_ra_pd_list
> to logical_router_port.ipv6_prefix, instead of appending.
>
> This function ovn_port_update_ipv6_prefix() is called from multiple places in
> ovn_port_update_sbrec(). I think this can be called just once right ?
>
> Also ovn_port_update_sbrec() doesn't seem like the right place to
> update the nb db.
> May be a new function - sync_ipv6_pd or something similar ?
>
> Can you please update ovn-northd.8.xml about the new flow being added ?
>
> Thanks
> Numan
>
> > +sset_destroy(_prefix_set);
> > +}
> > +
> >  static void
> >  ovn_port_update_sbrec(struct northd_context *ctx,
> >struct ovsdb_idl_index *sbrec_chassis_by_name,
> > @@ -2653,6 +2688,7 @@ ovn_port_update_sbrec(struct northd_context *ctx,
> >struct sset *active_ha_chassis_grps)
> >  {
> >  sbrec_port_binding_set_datapath(op->sb, op->od->sb);
> > +
> >  if (op->nbrp) {
> >  /* If the router is for l3 gateway, it resides on a chassis
> >   * and its port type is "l3gateway". */
> > @@ -2775,6 +2811,9 @@ ovn_port_update_sbrec(struct northd_context *ctx,
> >  smap_add(, "l3gateway-chassis", chassis_name);
> >  }
> >  }
> > +
> > +ovn_port_update_ipv6_prefix(ctx, op, );
> > +
> >  sbrec_port_binding_set_options(op->sb, );
> >  smap_destroy();
> >
> > @@ -2824,6 +2863,9 @@ ovn_port_update_sbrec(struct northd_context *ctx,
> >  smap_add_format(,
> >  "qdisc_queue_id", "%d", queue_id);
> >  }
> > +
> > +ovn_port_update_ipv6_prefix(ctx, op, );
> > +
> >  sbrec_port_binding_set_options(op->sb, );
> >  smap_destroy();
> >  if (ovn_is_known_nb_lsp_type(op->nbsp->type)) {
> > @@ -2873,6 +2915,9 @@ ovn_port_update_sbrec(struct northd_context *ctx,
> >  if (chassis) {
> >  smap_add(, "l3gateway-chassis", chassis);
> >  }
> > +
> > +ovn_port_update_ipv6_prefix(ctx, op, );
> > +
> >  sbrec_port_binding_set_options(op->sb, );
> >  smap_destroy();
> >  } else {
> > @@ -7129,6 +7174,11 @@ copy_ra_to_sb(struct ovn_port *op, const char 
> > *address_mode)
> >  }
> >  ds_put_format(, "%s/%u 

Re: [ovs-dev] [PATCH ovn 0/2] Caching logical flow expr tree for each lflow

[Numan Siddique]

On Thu, Jan 16, 2020 at 4:02 AM Mark Michelson  wrote:
>
> For the series:
> Acked-by: Mark Michelson 
>
> I think the speedup is commendable. Great job!
>
> That being said, I'm surprised it wasn't better. Do you know, with this
> patch applied, what the hit rate is for the cache? And assuming the hit
> rate is high, do you know what the new bottleneck in add_logical_flows() is?

Thanks for the review.

The hit ratio will be 100% for subsequent flow calculations for a logical flow.
However if the address set/port group associated with a logical flow
gets updated,
the cache is destroyed and updated with the new one.

Actually even I was surprised too. I was expecting a better improvement.

Right now, we still clone the (normalized) expr as
expr_evaluate_condition() could
destroy the original expression.
I think major chunk is spent in the expr_to_matches() function.

Thanks
Numan


>
> On 1/9/20 12:36 PM, num...@ovn.org wrote:
> > From: Numan Siddique 
> >
> > This patch series tries to improve the time taken to proceess logical
> > flows by caching the expr tree. For large scale deployments with lots
> > of logical flows, the logical flow processing to OpenFlow rules
> > takes a lot of time as it is CPU intensive.
> >
> > This patch series tries to improve this by caching the expr tree
> > generated for each logical flow so that we don't have to generate the
> > expr tree for each lflow_run().
> >
> > Below are the details of the OVN resource in my setup
> >
> > No of logical switches - 49
> > No of logical ports- 1191
> > No of logical routers  - 7
> > No of logical ports- 51
> > No of ACLs - 1221
> > No of Logical flows- 664736
> >
> > On a chassis hosting 7 distributed router ports and around 1090
> > port bindings, the no of OVS rules was 921162.
> >
> > Without this patch, the function add_logical_flows() took ~15 seconds.
> > And with this patch it took ~10 seconds. There is a good 34% improvement
> > in logical flow processing.
> >
> >
> > Numan Siddique (2):
> >expr: Evaluate the condition expression in a separate step.
> >ovn-controller: Cache logical flow expr tree for each lflow.
> >
> >   controller/lflow.c  | 181 +++-
> >   controller/lflow.h  |   9 +-
> >   controller/ovn-controller.c |  17 +++-
> >   include/ovn/expr.h  |  10 +-
> >   lib/expr.c  |  55 ---
> >   tests/test-ovn.c|  10 +-
> >   utilities/ovn-trace.c   |   5 +-
> >   7 files changed, 212 insertions(+), 75 deletions(-)
> >
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [v4] dpif-netdev: Modified ovs-appctl dpctl/dump-flows command

[Finn, Emma]

Comments inline.

> -Original Message-
> From: Finn, Emma 
> Sent: Thursday 16 January 2020 11:45
> To: d...@openvswitch.org
> Cc: i.maxim...@ovn.org; ian.sto...@intel.org; Finn, Emma
> 
> Subject: [v4] dpif-netdev: Modified ovs-appctl dpctl/dump-flows command
> 
> Modified ovs-appctl dpctl/dump-flows command to output the miniflow bits
> for a given flow when -m option is passed.
> 
> $ ovs-appctl dpctl/dump-flows -m
> 
> Signed-off-by: Emma Finn 
> 
> ---
> 
> RFC -> v1
> 
> * Changed revision from RFC to v1
> * Reformatted based on comments
> * Fixed same classifier being dumped multiple times
>   flagged by Ilya
> * Fixed print of subtables flagged by William
> * Updated print count of bits as well as bits themselves
> 
> ---
> 
> v1 -> v2
> 
> * Reformatted based on comments
> * Refactored code to make output part of ovs-appctl
>   dpctl/dump-flows -m command.
> 
> ---
> 
> v2 -> v3
> 
> * Added attribute dp_extra_info to dpif_flow_attrs struct
>   to store miniflow bits as a string
> 
> ---
> 
> v3 -> v4
> 
> * Fixed string leak
> * Refactored to code to make it  independent from the flowmap size
> ---

Any other feedback? 
I will have to rebase a send v5.

Regards, 
Emma

>  NEWS  |  2 ++
>  lib/dpctl.c   |  5 +
>  lib/dpif-netdev.c | 14 ++
>  lib/dpif.h|  1 +
>  4 files changed, 22 insertions(+)
> 
> diff --git a/NEWS b/NEWS
> index 965faca..1c9d2db 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -8,6 +8,8 @@ Post-v2.12.0
>   * Add option to enable, disable and query TCP sequence checking in
> conntrack.
>   * Add support for conntrack zone limits.
> + * Command "ovs-appctl dpctl/dump-flows" refactored to show subtable
> +   miniflow bits for userspace datapath.
> - AF_XDP:
>   * New option 'use-need-wakeup' for netdev-afxdp to control enabling
> of corresponding 'need_wakeup' flag in AF_XDP rings.  Enabled by
> default diff --git a/lib/dpctl.c b/lib/dpctl.c index a1ea25b..1b0a2bf 100644
> --- a/lib/dpctl.c
> +++ b/lib/dpctl.c
> @@ -825,6 +825,11 @@ format_dpif_flow(struct ds *ds, const struct
> dpif_flow *f, struct hmap *ports,
>  }
>  ds_put_cstr(ds, ", actions:");
>  format_odp_actions(ds, f->actions, f->actions_len, ports);
> +if (dpctl_p->verbosity && f->attrs.dp_extra_info) {
> +ds_put_format(ds, ", dp-extra-info:%s",
> +  f->attrs.dp_extra_info);
> +}
> +free(f->attrs.dp_extra_info);
>  }
> 
>  struct dump_types {
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 079bd1b..a640b49
> 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -3101,6 +3101,20 @@ dp_netdev_flow_to_dpif_flow(const struct
> dp_netdev_flow *netdev_flow,
> 
>  flow->attrs.offloaded = false;
>  flow->attrs.dp_layer = "ovs";
> +
> +struct ds extra_info = DS_EMPTY_INITIALIZER;
> +size_t unit;
> +
> +ds_put_cstr(_info, "miniflow_bits("); FLOWMAP_FOR_EACH_UNIT
> (unit) {
> +if (unit) {
> +ds_put_char(_info, ',');
> +}
> +ds_put_format(_info, "%d",
> +  count_1bits(netdev_flow->cr.mask->mf.map.bits[unit]));
> +}
> +ds_put_char(_info, ')');
> +flow->attrs.dp_extra_info = ds_steal_cstr(_info);
> +ds_destroy(_info);
>  }
> 
>  static int
> diff --git a/lib/dpif.h b/lib/dpif.h
> index c21e897..59d82dc 100644
> --- a/lib/dpif.h
> +++ b/lib/dpif.h
> @@ -513,6 +513,7 @@ struct dpif_flow_detailed_stats {  struct
> dpif_flow_attrs {
>  bool offloaded; /* True if flow is offloaded to HW. */
>  const char *dp_layer;   /* DP layer the flow is handled in. */
> +char *dp_extra_info;/* Extra information provided by DP. */
>  };
> 
>  struct dpif_flow_dump_types {
> --
> 2.7.4

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

Re: [ovs-dev] [PATCH v6 ovn 2/2] northd: add logical flows for dhcpv6 pfd parsing

[Numan Siddique]

On Tue, Jan 14, 2020 at 4:38 PM Lorenzo Bianconi
 wrote:
>
> Introduce logical flows in ovn router pipeline in order to parse dhcpv6
> advertise/reply from IPv6 prefix delegation router.
> Do not overwrite ipv6_ra_pd_list info in options column of SB port_binding
> table written by ovn-controller
> Introduce ipv6_prefix column in NB Logical_router_port table to report
> IPv6 prefix received from delegation router to the CMS
>
> Signed-off-by: Lorenzo Bianconi 

Hi Lorenzo,

Please see below for few comments.

> ---
>  northd/ovn-northd.c |  99 +-
>  ovn-nb.ovsschema|   5 +-
>  ovn-nb.xml  |  22 
>  tests/atlocal.in|   5 +-
>  tests/system-ovn.at | 127 
>  5 files changed, 255 insertions(+), 3 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index b6dc809d7..031792706 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -2645,6 +2645,41 @@ copy_gw_chassis_from_nbrp_to_sbpb(
>  free(sb_ha_chassis);
>  }
>
> +static void
> +ovn_port_update_ipv6_prefix(struct northd_context *ctx,
> +const struct ovn_port *op,
> +struct smap *sb_option)
> +{
> +const char *ipv6_pd_list = smap_get(>sb->options, "ipv6_ra_pd_list");
> +if (!ipv6_pd_list) {
> +return;
> +}
> +
> +smap_add(sb_option, "ipv6_ra_pd_list", ipv6_pd_list);
> +
> +const struct nbrec_logical_router_port *lrp = NULL, *iter;
> +/* update logical_router_port table */
> +NBREC_LOGICAL_ROUTER_PORT_FOR_EACH (iter, ctx->ovnnb_idl) {
> +if (!strcmp(iter->name, op->sb->logical_port)) {
> +lrp = iter;
> +break;
> +}
> +}

I don't think you need to run this for loop to get the lrp. I think
"struct ovn_port" should have a
pointer to "struct nbrec_logical_router_port".


> +if (!lrp) {
> +return;
> +}
> +
> +struct sset ipv6_prefix_set = SSET_INITIALIZER(_prefix_set);
> +sset_add_array(_prefix_set, lrp->ipv6_prefix, lrp->n_ipv6_prefix);
> +if (!sset_contains(_prefix_set, ipv6_pd_list)) {
> +sset_add(_prefix_set, ipv6_pd_list);
> +nbrec_logical_router_port_set_ipv6_prefix(lrp,
> +sset_array(_prefix_set),
> +sset_count(_prefix_set));
> +}

I think it's better to just copy whatever is there in the
port_binding.options:ipv6_ra_pd_list
to logical_router_port.ipv6_prefix, instead of appending.

This function ovn_port_update_ipv6_prefix() is called from multiple places in
ovn_port_update_sbrec(). I think this can be called just once right ?

Also ovn_port_update_sbrec() doesn't seem like the right place to
update the nb db.
May be a new function - sync_ipv6_pd or something similar ?

Can you please update ovn-northd.8.xml about the new flow being added ?

Thanks
Numan

> +sset_destroy(_prefix_set);
> +}
> +
>  static void
>  ovn_port_update_sbrec(struct northd_context *ctx,
>struct ovsdb_idl_index *sbrec_chassis_by_name,
> @@ -2653,6 +2688,7 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>struct sset *active_ha_chassis_grps)
>  {
>  sbrec_port_binding_set_datapath(op->sb, op->od->sb);
> +
>  if (op->nbrp) {
>  /* If the router is for l3 gateway, it resides on a chassis
>   * and its port type is "l3gateway". */
> @@ -2775,6 +2811,9 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>  smap_add(, "l3gateway-chassis", chassis_name);
>  }
>  }
> +
> +ovn_port_update_ipv6_prefix(ctx, op, );
> +
>  sbrec_port_binding_set_options(op->sb, );
>  smap_destroy();
>
> @@ -2824,6 +2863,9 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>  smap_add_format(,
>  "qdisc_queue_id", "%d", queue_id);
>  }
> +
> +ovn_port_update_ipv6_prefix(ctx, op, );
> +
>  sbrec_port_binding_set_options(op->sb, );
>  smap_destroy();
>  if (ovn_is_known_nb_lsp_type(op->nbsp->type)) {
> @@ -2873,6 +2915,9 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>  if (chassis) {
>  smap_add(, "l3gateway-chassis", chassis);
>  }
> +
> +ovn_port_update_ipv6_prefix(ctx, op, );
> +
>  sbrec_port_binding_set_options(op->sb, );
>  smap_destroy();
>  } else {
> @@ -7129,6 +7174,11 @@ copy_ra_to_sb(struct ovn_port *op, const char 
> *address_mode)
>  }
>  ds_put_format(, "%s/%u ", addrs->network_s, addrs->plen);
>  }
> +
> +const char *ra_pd_list = smap_get(>sb->options, "ipv6_ra_pd_list");
> +if (ra_pd_list) {
> +ds_put_cstr(, ra_pd_list);
> +}
>  /* Remove trailing space */
>  ds_chomp(, ' ');
>  smap_add(, "ipv6_ra_prefixes", ds_cstr());
> @@ -7851,7 

Re: [ovs-dev] [PATCH V7 00/18] netdev datapath actions offload

[Ilya Maximets]

On 13.01.2020 19:17, Ilya Maximets wrote:
> On 09.01.2020 08:46, Eli Britstein wrote:
>> Currently, netdev datapath offload only accelerates the flow match
>> sequence by associating a mark per flow. This series introduces the full
>> offload of netdev datapath flows by having the HW also perform the flow
>> actions.
>>
>> This series adds HW offload for output, drop, set MAC, set IPv4 and set
>> TCP/UDP ports actions using the DPDK rte_flow API.
>>
>> v1 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/614511472
>> v2 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/619262148
>> v3 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/622253870
>> v4 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/625654160
>> v5 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/626692202
>> v6 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/627132194
>> v7 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/634603553
>>
>> v1-v2:
>> - fallback to mark/rss in case of any failure of action offload.
>> - dp_layer changed to "in_hw" in case the flow is offloaded.
>> - using netdev_ports_get instead of dp_netdev_lookup_port in stats reading.
>> - using flow_dump_next API instead of a new API.
>> - validity checks for last action of output and clone.
>> - count action should be done for drop as well.
>> - validity check for output action.
>> - added doc in Documentation/howto/dpdk.rst.
>> v2-v3:
>> - removed refactoring for netdev-offload-dpdk-flow.c file.
>> - dropped flush commits.
>> - dbg prints rate-limited, and outside lock.
>> - added validity check for output action - same flow_api handle for both 
>> netdevs.
>> - added a mutex to protect possible concurrent deletion/querying of a flow.
>> - statistics are collected using flow_get API.
>> - added more documentation in Documentation/howto/dpdk.rst.
>> v3-v4:
>> - debug prints moved to netdev-offload-dpdk.c.
>> - flow get returns full stats, not diff.
>> - removed additional fields from offload_info and dp_netdev_flow.
>> - read HW stats during dpif_netdev_flow_get as well as
>>   dpif_netdev_flow_dump_next.
>> - moved actions offload framework just before support commit.
>> - fixed memory leak in rewrite code.
>> - dropped clone commit - will be posted in next series.
>> v4-v5:
>> - statistics collecting changed to populate dp_layer and offloaded
>>   attributes. dp_layer changed to "dpdk" for fully offloaded by dpdk.
>> - display offloaded:partial for partially offloaded flows.
>> - support filter types "dpdk" and "partially-offloaded" in
>>   dpctl/dump-flows type=X.
>> - simplify code readability of rewrite commits.
>> v5-v6:
>> - debug prints improvement for partial offload installations. dbg prints
>>   for failed actions, warn for other failure.
>> - fixed memory leak in case retrieved dpdk port-id is invalid.
>> v6-v7:
>> - rebase to support explicit drop action.
>> - fix dpctl/dump-flows issues discovered by make check-offloads.
>>
>> Eli Britstein (17):
>>   netdev-offload-dpdk: Refactor flow patterns
>>   netdev-offload-dpdk: Refactor action items freeing scheme
>>   netdev-offload-dpdk: Dynamically allocate pattern items
>>   netdev-offload-dpdk: Improve HW offload flow debuggability
>>   netdev-dpdk: Introduce rte flow query function
>>   netdev-offload-dpdk: Return UFID-rte_flow entry in find method
>>   netdev-offload-dpdk: Framework for actions offload
>>   netdev-offload-dpdk: Implement flow get method
>>   dpctl: Support dump-flows filters "dpdk" and "partially-offloaded"
>>   dpif-netdev: Populate dpif class field in offload struct
>>   netdev-dpdk: Getter function for dpdk port id API
>>   netdev-offload: Introduce a function to validate same flow api handle
>>   netdev-offload-dpdk: Support offload of output action
>>   netdev-offload-dpdk: Support offload of drop action
>>   netdev-offload-dpdk: Support offload of set MAC actions
>>   netdev-offload-dpdk: Support offload of set IPv4 actions
>>   netdev-offload-dpdk: Support offload of set TCP/UDP ports actions
>>
>> Ophir Munk (1):
>>   dpif-netdev: Update offloaded flows statistics
>>
>>  Documentation/howto/dpdk.rst  |  21 +-
>>  NEWS  |   5 +
>>  lib/dpctl.c   |  28 +-
>>  lib/dpctl.man |   2 +
>>  lib/dpif-netdev.c |  81 +++-
>>  lib/netdev-dpdk.c |  48 +++
>>  lib/netdev-dpdk.h |   8 +
>>  lib/netdev-offload-dpdk.c | 967 
>> --
>>  lib/netdev-offload-provider.h |   1 +
>>  lib/netdev-offload.c  |  12 +
>>  10 files changed, 927 insertions(+), 246 deletions(-)
>>
> 
> Thanks for v7!  This version looks OK to me except few minor style issues
> that I could fix myself before applying.  Tomorrow I'm going to run few
> tests with this version and will apply the series on Wednesday in case
> no serious issues will be observed.

So, I fixed all the issues I found in my review and 

Re: [ovs-dev] [PATCH net-next v3] openvswitch: add TTL decrement action

[Nicolas Dichtel]

Le 16/01/2020 à 08:21, Pravin Shelar a écrit :
> On Wed, Jan 15, 2020 at 8:40 AM Matteo Croce  wrote:
[snip]
>> @@ -1050,4 +1051,5 @@ struct ovs_zone_limit {
>> __u32 count;
>>  };
>>
>> +#define OVS_DEC_TTL_ATTR_EXEC  0
> 
> I am not sure if we need this, But if you want the nested attribute
> then lets define enum with this single attribute and have actions as
> part of its data. This would be optional argument, so userspace can
> skip it, and in that case datapath can drop the packet.
And note that 0 is a reserved value and should not be used. Look at other
attributes.

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

Re: [ovs-dev] [PATCH v6 ovn 1/2] controller: add ipv6 prefix delegation state machine

[Numan Siddique]

"

On Tue, Jan 14, 2020 at 4:37 PM Lorenzo Bianconi
 wrote:
>
> Introduce IPv6 Prefix delegation state machine according to RFC 3633
> https://tools.ietf.org/html/rfc3633.
> Add handle_dhcpv6_reply controller action to parse advertise/reply from
> IPv6 delegation server. Advertise/reply are parsed running respectively:
> - pinctrl_parse_dhcv6_advt
> - pinctrl_parse_dhcv6_reply
> The IPv6 requesting router starts sending dhcpv6 solicit through the logical
> router port marked with ipv6_prefix_delegation set to true.
> An IPv6 prefix will be requested for each logical router port marked
> with "prefix" set to true in option column of logical router port table.
> Save IPv6 prefix received by IPv6 delegation router in the options columns of
> SB port binding table in order to be reused by Router Advertisement framework
> run by ovn logical router pipeline.
> IPv6 Prefix delegation state machine is enabled on Gateway Router or on
> a Gateway Router Port
>
> Signed-off-by: Lorenzo Bianconi 


Hi Lorenzo,
Thanks for the v6.

I tested this series.  Below are some comments

 - When I configured the prefix options on the router ports,
ovn-controller hosting the gateway
   router port sends the PD solicit message and gets the prefix, but
it never sends the renew/Solicit
   requests periodically. In the code I see that the next announce
time is chosen. But looks like that is not
  working. I think it's better to use the "prefix lifetime option"
value sent by the delegating server rather than
  current_time + random(3000) which this patch does.

- Suppose if a router port had received a prefix 'P1' and if I restart
ovn-controller, ovn-controller
  sends the PD messages and gets another prefix 'P2'. Ideally
ovn-controller should try to renew
  the existing prefix. Can you please check if it is possible to
include the prefix option in the Request message ?
  Probably ovn-controller can send a Renew request if a router port
has already a prefix ? If the delegating server,
  can't allocate the same prefix, ovn-controller can start the fresh
process of sending Solicit message.

- In the above case, I notice that when delegating server sends 'P2',
ovn-controller doesn't update this new prefix
  in the port_binding.options:ipv6_ra_pd_list. And the patch 2 of this
series, doesn't update the "ipv6_prefix column
  of logical router port.  I think it's better for ovn-northd to just
update/reset the "ipv6_prefix" column from the
  port_binding.options:ipv6_ra_pd_list, rather than appending the value.

 - The action handle_dhcp6_reply takes inner actions. But this is not
documented.
   I think it's better not to include the inner actions. When the
delegating server sends the "reply" message, ovn-controller
   doesn't need to send any reply. But since this action is used for
handling the "Advertise" and "Reply" messages from
   the delegating router, it doesn't seem appropriate to include the
inner actions.

Please see some more comments below


> ---
>  controller/ovn-controller.c |   1 +
>  controller/pinctrl.c| 612 +++-
>  controller/pinctrl.h|   2 +
>  include/ovn/actions.h   |   8 +-
>  lib/actions.c   |  22 ++
>  lib/ovn-l7.h|  19 ++
>  ovn-sb.xml  |   8 +
>  tests/ovn.at|   6 +
>  utilities/ovn-trace.c   |   3 +
>  9 files changed, 673 insertions(+), 8 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 17744d416..d559e845e 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -2145,6 +2145,7 @@ main(int argc, char *argv[])
>  runtime_data = engine_get_data(_runtime_data);
>  if (runtime_data) {
>  pinctrl_run(ovnsb_idl_txn,
> +ovnsb_idl_loop.idl,
>  sbrec_datapath_binding_by_key,
>  sbrec_port_binding_by_datapath,
>  sbrec_port_binding_by_key,
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 452ca8a1c..2a37fc525 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -270,6 +270,20 @@ static void pinctrl_ip_mcast_handle_igmp(
>  const struct match *md,
>  struct ofpbuf *userdata);
>
> +static void init_ipv6_prefixd(void);
> +static void destroy_ipv6_prefixd(void);
> +static void ipv6_prefixd_wait(long long int timeout);
> +static void
> +prepare_ipv6_prefixd(struct ovsdb_idl_txn *ovnsb_idl_txn,
> + struct ovsdb_idl *ovnsb_idl,
> + struct ovsdb_idl_index *sbrec_port_binding_by_name,
> + const struct hmap *local_datapaths,
> + const struct sbrec_chassis *chassis,
> + const struct sset *active_tunnels)
> +OVS_REQUIRES(pinctrl_mutex);
> +static void
> +send_ipv6_prefixd(struct rconn *swconn, long 

Re: [ovs-dev] [PATCH] dpif-netdev: Make datapath port mutex recursive.

[Ilya Maximets]

On 16.01.2020 13:13, Stokes, Ian wrote:
> 
> 
> On 1/15/2020 5:29 PM, Ilya Maximets wrote:
>> Upcoming HW offloading will request flow statistics from the dpdk
>> offloading module.  This operation requires holding datapath port
>> mutex.  However, there is a possible scenarion in which flow deletion
>> happens during datapath reconfiguration process and the mutex already
>> acquired:
>>
>>    0  raise () from /lib64/libc.so.6
>>    1  abort () from /lib64/libc.so.6
>>    2  ovs_abort_valist ()
>>    3  ovs_abort ()
>>    4  ovs_mutex_lock_at ()
>>    5  dpif_netdev_get_flow_offload_status ()
>>    6  get_dpif_flow_status ()
>>    7  flow_del_on_pmd ()
>>    8  dpif_netdev_flow_del ()
>>    9  dpif_netdev_operate ()
>>    10 dpif_operate ()
>>    11 push_dp_ops ()
>>    12 push_ukey_ops ()
>>    13 dp_purge_cb ()
>>    14 dp_netdev_del_pmd ()
>>    15 reconfigure_pmd_threads ()
>>    16 reconfigure_datapath ()
>>    17 do_del_port ()
>>    18 dpif_netdev_port_del ()
>>    19 dpif_port_del ()
>>    20 port_del ()
>>    21 ofproto_port_del ()
>>    22 bridge_delete_or_reconfigure_ports ()
>>    23 bridge_reconfigure ()
>>    24 bridge_run ()
>>    25 main ()
>>
>> This happens while removing the last port of a particular PMD thread.
>> Reconfiguration process decides that we need to remove current PMD
>> thread and calls datapath purge callback in order to clean up resources
>> assigned to it.  This turns into flow removal and flow_del() tries to
>> request statistics.
>>
>> Turning the dp->mutex into recursive version as a quick fix for this
>> issue.  Better solutions might be to avoid statistics request somehow,
>> or fully disassociate offloaded flows from the datapath flows.
>>
>> Signed-off-by: Ilya Maximets 
> 
> Thanks for the patch Ilya, makes snse to me as a workaround for the moment. 
> Tested in my own deployment and didnt see any issues.
> 
> Acked.


Thanks.  I don't really like this solution.  Recursive mutexes usually
indicates architectural issues (which we definitely have) and should be
avoided as possible.  Hope, we'll find a way to revert this change in
the future.  For now, applied to master.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] Documentation: Fix building with Python 3.9

[Timothy Redaelli]

open(), io.open(), codecs.open() and fileinput.FileInput no longer accept 'U'
("universal newline") in the file mode.
This flag was deprecated since Python 3.3.
In Python 3, the "universal newline" is used by default when a file is open
in text mode.

Reported-at: https://bugzilla.redhat.com/1791681
Reported-by: Miro Hrončok 
Signed-off-by: Timothy Redaelli 
---
 Documentation/conf.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/conf.py b/Documentation/conf.py
index 6bbfc02bd..37d92c36f 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -58,7 +58,7 @@ author = u'The Open vSwitch Development Community'
 # The full version, including alpha/beta/rc tags.
 release = None
 filename = "../configure.ac"
-with open(filename, 'rU') as f:
+with open(filename, 'r') as f:
 for line in f:
 if 'AC_INIT' in line:
 # Parse "AC_INIT(openvswitch, 2.7.90, b...@openvswitch.org)":
-- 
2.24.1

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

Re: [ovs-dev] [PATCH] dpif-netdev: Make datapath port mutex recursive.

[Stokes, Ian]

On 1/15/2020 5:29 PM, Ilya Maximets wrote:

Upcoming HW offloading will request flow statistics from the dpdk
offloading module.  This operation requires holding datapath port
mutex.  However, there is a possible scenarion in which flow deletion
happens during datapath reconfiguration process and the mutex already
acquired:

   0  raise () from /lib64/libc.so.6
   1  abort () from /lib64/libc.so.6
   2  ovs_abort_valist ()
   3  ovs_abort ()
   4  ovs_mutex_lock_at ()
   5  dpif_netdev_get_flow_offload_status ()
   6  get_dpif_flow_status ()
   7  flow_del_on_pmd ()
   8  dpif_netdev_flow_del ()
   9  dpif_netdev_operate ()
   10 dpif_operate ()
   11 push_dp_ops ()
   12 push_ukey_ops ()
   13 dp_purge_cb ()
   14 dp_netdev_del_pmd ()
   15 reconfigure_pmd_threads ()
   16 reconfigure_datapath ()
   17 do_del_port ()
   18 dpif_netdev_port_del ()
   19 dpif_port_del ()
   20 port_del ()
   21 ofproto_port_del ()
   22 bridge_delete_or_reconfigure_ports ()
   23 bridge_reconfigure ()
   24 bridge_run ()
   25 main ()

This happens while removing the last port of a particular PMD thread.
Reconfiguration process decides that we need to remove current PMD
thread and calls datapath purge callback in order to clean up resources
assigned to it.  This turns into flow removal and flow_del() tries to
request statistics.

Turning the dp->mutex into recursive version as a quick fix for this
issue.  Better solutions might be to avoid statistics request somehow,
or fully disassociate offloaded flows from the datapath flows.

Signed-off-by: Ilya Maximets 


Thanks for the patch Ilya, makes snse to me as a workaround for the 
moment. Tested in my own deployment and didnt see any issues.


Acked.

Ian

---

Ideas for better solution are welcome.

  lib/dpif-netdev.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 079bd1bdf..d4b1ebbff 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1547,7 +1547,7 @@ create_dp_netdev(const char *name, const struct 
dpif_class *class,
  ovs_refcount_init(>ref_cnt);
  atomic_flag_clear(>destroyed);
  
-ovs_mutex_init(>port_mutex);

+ovs_mutex_init_recursive(>port_mutex);
  hmap_init(>ports);
  dp->port_seq = seq_create();
  fat_rwlock_init(>upcall_rwlock);


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

[ovs-dev] [v4] dpif-netdev: Modified ovs-appctl dpctl/dump-flows command

[Emma Finn]

Modified ovs-appctl dpctl/dump-flows command to output
the miniflow bits for a given flow when -m option is passed.

$ ovs-appctl dpctl/dump-flows -m

Signed-off-by: Emma Finn 

---

RFC -> v1

* Changed revision from RFC to v1
* Reformatted based on comments
* Fixed same classifier being dumped multiple times
  flagged by Ilya
* Fixed print of subtables flagged by William
* Updated print count of bits as well as bits themselves

---

v1 -> v2

* Reformatted based on comments
* Refactored code to make output part of ovs-appctl
  dpctl/dump-flows -m command.

---

v2 -> v3

* Added attribute dp_extra_info to dpif_flow_attrs struct
  to store miniflow bits as a string

---

v3 -> v4

* Fixed string leak
* Refactored to code to make it  independent from the flowmap size
---
 NEWS  |  2 ++
 lib/dpctl.c   |  5 +
 lib/dpif-netdev.c | 14 ++
 lib/dpif.h|  1 +
 4 files changed, 22 insertions(+)

diff --git a/NEWS b/NEWS
index 965faca..1c9d2db 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,8 @@ Post-v2.12.0
  * Add option to enable, disable and query TCP sequence checking in
conntrack.
  * Add support for conntrack zone limits.
+ * Command "ovs-appctl dpctl/dump-flows" refactored to show subtable
+   miniflow bits for userspace datapath.
- AF_XDP:
  * New option 'use-need-wakeup' for netdev-afxdp to control enabling
of corresponding 'need_wakeup' flag in AF_XDP rings.  Enabled by default
diff --git a/lib/dpctl.c b/lib/dpctl.c
index a1ea25b..1b0a2bf 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -825,6 +825,11 @@ format_dpif_flow(struct ds *ds, const struct dpif_flow *f, 
struct hmap *ports,
 }
 ds_put_cstr(ds, ", actions:");
 format_odp_actions(ds, f->actions, f->actions_len, ports);
+if (dpctl_p->verbosity && f->attrs.dp_extra_info) {
+ds_put_format(ds, ", dp-extra-info:%s",
+  f->attrs.dp_extra_info);
+}
+free(f->attrs.dp_extra_info);
 }
 
 struct dump_types {
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 079bd1b..a640b49 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3101,6 +3101,20 @@ dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow 
*netdev_flow,
 
 flow->attrs.offloaded = false;
 flow->attrs.dp_layer = "ovs";
+
+struct ds extra_info = DS_EMPTY_INITIALIZER;
+size_t unit;
+
+ds_put_cstr(_info, "miniflow_bits("); FLOWMAP_FOR_EACH_UNIT (unit) {
+if (unit) {
+ds_put_char(_info, ',');
+}
+ds_put_format(_info, "%d",
+  count_1bits(netdev_flow->cr.mask->mf.map.bits[unit]));
+}
+ds_put_char(_info, ')');
+flow->attrs.dp_extra_info = ds_steal_cstr(_info);
+ds_destroy(_info);
 }
 
 static int
diff --git a/lib/dpif.h b/lib/dpif.h
index c21e897..59d82dc 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -513,6 +513,7 @@ struct dpif_flow_detailed_stats {
 struct dpif_flow_attrs {
 bool offloaded; /* True if flow is offloaded to HW. */
 const char *dp_layer;   /* DP layer the flow is handled in. */
+char *dp_extra_info;/* Extra information provided by DP. */
 };
 
 struct dpif_flow_dump_types {
-- 
2.7.4

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

Re: [ovs-dev] [v3] dpif-netdev: Modified ovs-appctl dpctl/dump-flows command

[Ilya Maximets]

On 16.01.2020 11:18, Finn, Emma wrote:
> 
> 
>> -Original Message-
>> From: Ilya Maximets 
>> Sent: Wednesday 15 January 2020 17:27
>> To: Finn, Emma ; d...@openvswitch.org
>> Cc: i.maxim...@ovn.org; ian.sto...@intel.org
>> Subject: Re: [v3] dpif-netdev: Modified ovs-appctl dpctl/dump-flows
>> command
>>
>> On 15.01.2020 17:19, Emma Finn wrote:
>>> Modified ovs-appctl dpctl/dump-flows command to output the miniflow
>>> bits for a given flow when -m option is passed.
>>>
>>> $ ovs-appctl dpctl/dump-flows -m
>>>
>>> Signed-off-by: Emma Finn 
>>>
>>> ---
>>>
>>> RFC -> v1
>>>
>>> * Changed revision from RFC to v1
>>> * Reformatted based on comments
>>> * Fixed same classifier being dumped multiple times
>>>   flagged by Ilya
>>> * Fixed print of subtables flagged by William
>>> * Updated print count of bits as well as bits themselves
>>>
>>> ---
>>>
>>> v1 -> v2
>>>
>>> * Reformatted based on comments
>>> * Refactored code to make output part of ovs-appctl
>>>   dpctl/dump-flows -m command.
>>>
>>> ---
>>>
>>> v2 -> v3
>>>
>>> * Added attribute dp_extra_info to dpif_flow_attrs struct
>>>   to store miniflow bits as a string
>>> ---
>>>  NEWS  | 2 ++
>>>  lib/dpctl.c   | 4 
>>>  lib/dpif-netdev.c | 8 
>>>  lib/dpif.h| 1 +
>>>  4 files changed, 15 insertions(+)
>>>
>>> diff --git a/NEWS b/NEWS
>>> index 965faca..1c9d2db 100644
>>> --- a/NEWS
>>> +++ b/NEWS
>>> @@ -8,6 +8,8 @@ Post-v2.12.0
>>>   * Add option to enable, disable and query TCP sequence checking in
>>> conntrack.
>>>   * Add support for conntrack zone limits.
>>> + * Command "ovs-appctl dpctl/dump-flows" refactored to show
>> subtable
>>> +   miniflow bits for userspace datapath.
>>> - AF_XDP:
>>>   * New option 'use-need-wakeup' for netdev-afxdp to control enabling
>>> of corresponding 'need_wakeup' flag in AF_XDP rings.  Enabled
>>> by default diff --git a/lib/dpctl.c b/lib/dpctl.c index
>>> a1ea25b..bd33ac4 100644
>>> --- a/lib/dpctl.c
>>> +++ b/lib/dpctl.c
>>> @@ -825,6 +825,10 @@ format_dpif_flow(struct ds *ds, const struct
>> dpif_flow *f, struct hmap *ports,
>>>  }
>>>  ds_put_cstr(ds, ", actions:");
>>>  format_odp_actions(ds, f->actions, f->actions_len, ports);
>>> +if (dpctl_p->verbosity && f->attrs.dp_extra_info) {
>>> +ds_put_format(ds, ", dp-extra-info:miniflow_bits%s",
>>> +  f->attrs.dp_extra_info);
>>
>> Please, make 'miniflow_bits' part of f->attrs.dp_extra_info.
>> This doesn't make sense for other datapaths.
>>
>> And the string leaked here.
> 
> Hi Ilya, Can you explain further what you are referring to here?

I mean that here should be:
ds_put_format(ds, ", dp-extra-info:%s", f->attrs.dp_extra_info);

And in dpif-netdev.c:
ds_put_format(_info, "miniflow_bits(0x%X,0x%X)", ...


And someone needs to call free() on f->attrs.dp_extra_info in the end.
(Note that this should be done regardless of verbosity level).

>>
>>> +}
>>>  }
>>>
>>>  struct dump_types {
>>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index
>>> 079bd1b..51def10 100644
>>> --- a/lib/dpif-netdev.c
>>> +++ b/lib/dpif-netdev.c
>>> @@ -3101,6 +3101,14 @@ dp_netdev_flow_to_dpif_flow(const struct
>>> dp_netdev_flow *netdev_flow,
>>>
>>>  flow->attrs.offloaded = false;
>>>  flow->attrs.dp_layer = "ovs";
>>> +
>>> +struct ds extra_info = DS_EMPTY_INITIALIZER;
>>> +
>>> +ds_put_format(_info, "(0x%X,0x%X)",
>>> +  count_1bits(netdev_flow->cr.mask->mf.map.bits[0]),
>>> +  count_1bits(netdev_flow->cr.mask->mf.map.bits[1]));

Again, what is the purpose of printing bits count in hex?
Bits count should be printed as decimal, while actual bitmap may be
printed as hex (I don't think we actually need to print it).

One more thing is that we could actually make above code independent from
the flowmap size like this:

size_t unit;

ds_put_cstr(_info, "miniflow_bits(");
FLOWMAP_FOR_EACH_UNIT (unit) {
if (unit) {
ds_put_char(_info, ',');
}
ds_put_format(_info, "%d",
  count_1bits(netdev_flow->cr.mask->mf.map.bits[unit]));
}
ds_put_char(_info, ')');

>>> +flow->attrs.dp_extra_info = ds_steal_cstr(_info);
>>> +ds_destroy(_info);
>>>  }
>>>
>>>  static int
>>> diff --git a/lib/dpif.h b/lib/dpif.h
>>> index c21e897..064f884 100644
>>> --- a/lib/dpif.h
>>> +++ b/lib/dpif.h
>>> @@ -513,6 +513,7 @@ struct dpif_flow_detailed_stats {  struct
>>> dpif_flow_attrs {
>>>  bool offloaded; /* True if flow is offloaded to HW. */
>>>  const char *dp_layer;   /* DP layer the flow is handled in. */
>>> +char *dp_extra_info;
>>
>> Comment required.  Keep it generic, please, i.e. no mentioning of
>> 'miniflow_bits'.
>>
> /* Stores extra information on DP*/  - Is this comment Ok?

/* Extra information provided by DP. */ ?

>>>  };
>>>
>>>  struct dpif_flow_dump_types {
>>>
___
dev mailing list

Re: [ovs-dev] [PATCH v2] tc: handle packet mark of zero

[Simon Horman]

On Thu, Jan 16, 2020 at 04:59:54AM -0500, 0-day Robot wrote:
> Bleep bloop.  Greetings Simon Horman, 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: Unexpected sign-offs from developers who are not authors or 
> co-authors or committers: Simon Horman 
> Lines checked: 44, Warnings: 1, Errors: 0

Is the correct tag Co-Authored-by rather than Co-Authored as used in
this patch?

> 
> 
> Please check this out.  If you feel there has been an error, please email 
> acon...@redhat.com
> 
> Thanks,
> 0-day Robot
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [v3] dpif-netdev: Modified ovs-appctl dpctl/dump-flows command

[Finn, Emma]

> -Original Message-
> From: Ilya Maximets 
> Sent: Wednesday 15 January 2020 17:27
> To: Finn, Emma ; d...@openvswitch.org
> Cc: i.maxim...@ovn.org; ian.sto...@intel.org
> Subject: Re: [v3] dpif-netdev: Modified ovs-appctl dpctl/dump-flows
> command
> 
> On 15.01.2020 17:19, Emma Finn wrote:
> > Modified ovs-appctl dpctl/dump-flows command to output the miniflow
> > bits for a given flow when -m option is passed.
> >
> > $ ovs-appctl dpctl/dump-flows -m
> >
> > Signed-off-by: Emma Finn 
> >
> > ---
> >
> > RFC -> v1
> >
> > * Changed revision from RFC to v1
> > * Reformatted based on comments
> > * Fixed same classifier being dumped multiple times
> >   flagged by Ilya
> > * Fixed print of subtables flagged by William
> > * Updated print count of bits as well as bits themselves
> >
> > ---
> >
> > v1 -> v2
> >
> > * Reformatted based on comments
> > * Refactored code to make output part of ovs-appctl
> >   dpctl/dump-flows -m command.
> >
> > ---
> >
> > v2 -> v3
> >
> > * Added attribute dp_extra_info to dpif_flow_attrs struct
> >   to store miniflow bits as a string
> > ---
> >  NEWS  | 2 ++
> >  lib/dpctl.c   | 4 
> >  lib/dpif-netdev.c | 8 
> >  lib/dpif.h| 1 +
> >  4 files changed, 15 insertions(+)
> >
> > diff --git a/NEWS b/NEWS
> > index 965faca..1c9d2db 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -8,6 +8,8 @@ Post-v2.12.0
> >   * Add option to enable, disable and query TCP sequence checking in
> > conntrack.
> >   * Add support for conntrack zone limits.
> > + * Command "ovs-appctl dpctl/dump-flows" refactored to show
> subtable
> > +   miniflow bits for userspace datapath.
> > - AF_XDP:
> >   * New option 'use-need-wakeup' for netdev-afxdp to control enabling
> > of corresponding 'need_wakeup' flag in AF_XDP rings.  Enabled
> > by default diff --git a/lib/dpctl.c b/lib/dpctl.c index
> > a1ea25b..bd33ac4 100644
> > --- a/lib/dpctl.c
> > +++ b/lib/dpctl.c
> > @@ -825,6 +825,10 @@ format_dpif_flow(struct ds *ds, const struct
> dpif_flow *f, struct hmap *ports,
> >  }
> >  ds_put_cstr(ds, ", actions:");
> >  format_odp_actions(ds, f->actions, f->actions_len, ports);
> > +if (dpctl_p->verbosity && f->attrs.dp_extra_info) {
> > +ds_put_format(ds, ", dp-extra-info:miniflow_bits%s",
> > +  f->attrs.dp_extra_info);
> 
> Please, make 'miniflow_bits' part of f->attrs.dp_extra_info.
> This doesn't make sense for other datapaths.
> 
> And the string leaked here.

Hi Ilya, Can you explain further what you are referring to here?
> 
> > +}
> >  }
> >
> >  struct dump_types {
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index
> > 079bd1b..51def10 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -3101,6 +3101,14 @@ dp_netdev_flow_to_dpif_flow(const struct
> > dp_netdev_flow *netdev_flow,
> >
> >  flow->attrs.offloaded = false;
> >  flow->attrs.dp_layer = "ovs";
> > +
> > +struct ds extra_info = DS_EMPTY_INITIALIZER;
> > +
> > +ds_put_format(_info, "(0x%X,0x%X)",
> > +  count_1bits(netdev_flow->cr.mask->mf.map.bits[0]),
> > +  count_1bits(netdev_flow->cr.mask->mf.map.bits[1]));
> > +flow->attrs.dp_extra_info = ds_steal_cstr(_info);
> > +ds_destroy(_info);
> >  }
> >
> >  static int
> > diff --git a/lib/dpif.h b/lib/dpif.h
> > index c21e897..064f884 100644
> > --- a/lib/dpif.h
> > +++ b/lib/dpif.h
> > @@ -513,6 +513,7 @@ struct dpif_flow_detailed_stats {  struct
> > dpif_flow_attrs {
> >  bool offloaded; /* True if flow is offloaded to HW. */
> >  const char *dp_layer;   /* DP layer the flow is handled in. */
> > +char *dp_extra_info;
> 
> Comment required.  Keep it generic, please, i.e. no mentioning of
> 'miniflow_bits'.
> 
/* Stores extra information on DP*/  - Is this comment Ok?
> >  };
> >
> >  struct dpif_flow_dump_types {
> >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] tc: handle packet mark of zero

[0-day Robot]

Bleep bloop.  Greetings Simon Horman, 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: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Simon Horman 
Lines checked: 44, Warnings: 1, Errors: 0


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

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

[ovs-dev] [PATCH v2] tc: handle packet mark of zero

[Simon Horman]

From: John Hurley 

Openstack may set an skb mark of 0 in tunnel rules. This is considered to
be an unused/unset value. However, it prevents the rule from being
offloaded.

Check if the key value of the skb mark is 0 when it is in use (mask is
set to all ones). If it is then ignore the field and continue with TC offload.

Only the exact-match case is covered by this patch as it addresses the
Openstack use-case and seems most robust against feature evolution: f.e. in
future there may exist hardware offload scenarios where an operation, such
as a BPF offload, sets the SKB mark before proceeding tho the in-HW OVS.
datapath.

Signed-off-by: John Hurley 
Co-Authored: Simon Horman 
Signed-off-by: Simon Horman 

---
v0 [John Hurley]

v1 [Simon Horman]
* Check for exact rather than masked match on skb 0

v2 [Simon Horman]
* Add co-authored tag
  Add explanation of check against exact-match to changelog
---
 lib/netdev-offload-tc.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 4988dadc3f50..5781d163e276 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -1619,6 +1619,11 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 mask->ct_label = OVS_U128_ZERO;
 }
 
+/* ignore exact match on skb_mark of 0. */
+if (mask->pkt_mark == UINT32_MAX && !key->pkt_mark) {
+mask->pkt_mark = 0;
+}
+
 err = test_key_and_mask(match);
 if (err) {
 return err;
-- 
2.20.1

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

[ovs-dev] Powerful Electr0nic M0squito Lamp ⚔️⚔️⚔️⚔️⚔️⚔️⚔️.........................................

[Zachary]

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