Re: [ovs-dev] [PATCH] FAQ: Add entry about different datapaths features.

[Gray, Mark D]

Good initiative Daniele. Comment below.

> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Daniele Di
> Proietto
> Sent: Monday, December 7, 2015 9:01 PM
> To: dev@openvswitch.org
> Subject: [ovs-dev] [PATCH] FAQ: Add entry about different datapaths
> features.
> 
> This is an easy way to keep track of the features supported by the different
> datapaths.
> 
> Nithin helped filling the list for the Hyper-V port.
> 
> CC: Nithin Raju 
> Signed-off-by: Daniele Di Proietto 
> ---
>  FAQ.md | 64
> ++
> ++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/FAQ.md b/FAQ.md
> index 8397e0f..aaf2026 100644
> --- a/FAQ.md
> +++ b/FAQ.md
> @@ -188,6 +188,70 @@ A: Features are gradually introduced into the
> upstream kernel so unless
>  |STT| 2.4+ | 3.5+
>  | Conntrack | 2.5+ | 3.10+
> 
> +### Q: Are all features available with all datapaths?
> +
> +A: Open vSwitch supports different datapaths on different platforms.  Each
> +   datapath has a different feature set: the following tables try to 
> summarize
> +   the status.
> +
> +   Supported datapaths:
> +
> +   * *Linux upstream*: The datapath implemented by the kernel module
> shipped
> +   with the upstream Linux kernel.  The numbers here 
> refer
> +   to the minimum Linux version that has an openvswitch
> +   module supporting a feature.
> +
> +   * *Linux OVS tree*: The datapath implemented by the Linux kernel
> module
> +   distributed with the OVS source tree. The numbers here
> +   refer to the minimum Linux kernel version against 
> which
> +   the module can be compiled to support a feature.
> +
> +   * *Userspace*: Also known as DPDK, dpif-netdev or dummy datapath.
> +
> +   * *Hyper-V*: Also known as the windows datapath.
> +
> +   The following table lists the datapath supported features from
> +   an Open vSwitch user's perspective.
> +
> +Feature   | Linux upstream | Linux OVS tree | Userspace | 
> Hyper-V |
> +--|:--:|:--:|:-:|:-
> +--|--:|
> +Connection tracking   |  4.3   |   3.10 |NO |   NO   
>  |
> +Tunnel - LISP |  NO|   YES  |NO |   NO   
>  |
> +Tunnel - STT  |  NO|   3.5  |NO |   YES  
>  |
> +Tunnel - GRE  |  3.11  |   YES  |YES|   NO   
>  |
> +Tunnel - VXLAN|  3.12  |   YES  |YES|   YES  
>  |
> +Tunnel - Geneve   |  3.18  |   YES  |YES|   NO   
>  |
> +QoS   |  YES   |   YES  |NO |   NO   
>  |

 [Gray, Mark D] 
QoS is such a loaded term. It may be good to expand on this a bit.

> +sFlow |  YES   |   YES  |YES|   NO   
>  |
> +Set action|  YES   |   YES  |YES| 
> PARTIAL |
> +NIC Bonding   |  YES   |   YES  |YES|   NO   
>  |
> +Multiple VTEPs|  YES   |   YES  |YES|   NO   
>  |
> +
> +   **Notes:**
> +   * Only a limited set of flow fields is modifiable via the set action by 
> the
> + Hyper-V datapath.
> +   * The Hyper-V datapath only supports one physical NIC per datapath. This
> is
> + why bonding is not supported.
> +   * The Hyper-V datapath can have at most one IP address configured as a
> + tunnel endpoint.
> +
> +   The following table lists features that do not *directly* impact an
> +   Open vSwitch user, e.g. because their absence can be hidden by the
> ofproto
> +   layer (usually this comes with a performance penalty).
> +
> +Feature   | Linux upstream | Linux OVS tree | Userspace | 
> Hyper-V |
> +--|:--:|:--:|:-:|:-
> +--|--:|
> +SCTP flows|  3.12  |   YES  |YES|   YES  
>  |
> +MPLS  |  3.19  |   YES  |YES|   NO   
>  |
> +UFID  |  4.0   |   YES  |YES|   NO   
>  |
> +Megaflows |  3.12  |   YES  |YES|   NO   
>  |
> +Masked set action |  4.0   |   YES  |YES|   NO   
>  |
> +Recirculation |  3.19  |   YES  |YES|   NO   
>  |
> +TCP flags matching|  3.13  |   YES  |YES|   NO   
>  |
> +Validate flow actions |  YES   |   YES  |N/A|   NO   
>  |
> +Multiple datapaths|  YES   |   YES  |YES|   NO   
>  |
> +Tunnel TSO - STT  |  N/A   |   YES  |NO |   YES  
>  |
> +
>  ### Q: I get an error like this when I configur

Re: [ovs-dev] [PATCH] datapath-windows: Support for OVS_KEY_ATTR_MPLS attribute

[Sorin Vinturis]

Hi Nithin,

Thanks for your review. The mpls push and pop actions were tested by manually 
changing the action type when one was received. I don't know how to trigger the 
MPLS push/pop actions for properly testing this part. 

The part of the patch regarding MPLS key extraction or NL buffer parsing/setup 
were tested using a Hyper-V machine and a Devstack one. On the Devstack machine 
I have installed OVS and added mpls push flows to a tun bridge. In this way the 
network packets leaving the Devstack had the MPLS labels.

Your comments are valid and will be addressed in a subsequent patch.

Thanks,
-Sorin

-Original Message-
From: Nithin Raju [mailto:nit...@vmware.com] 
Sent: Tuesday, 8 December, 2015 03:03
To: Sorin Vinturis
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] datapath-windows: Support for OVS_KEY_ATTR_MPLS 
attribute

Hi Sorin,
I found some bugs in OvsActionMplsPush(). Can you pls. clarify if my comments 
are valid? If they are genuine bugs, did your testing not catch them? 
Apologies, if I missed anything.

I¹ll review the rest of the patch after the claritification.

Thanks,
-- Nithin

-Original Message-
From: dev  on behalf of Sorin Vinturis 

Date: Wednesday, November 11, 2015 at 12:11 PM
To: "dev@openvswitch.org" 
Subject: [ovs-dev] [PATCH] datapath-windows: Support for
OVS_KEY_ATTR_MPLS   attribute

>This patch adds OVS_KEY_ATTR_MPLS to the OVS flow mechanism.
>
>Signed-off-by: Sorin Vinturis 
>---
> datapath-windows/ovsext/Actions.c  | 176
>+
> datapath-windows/ovsext/DpInternal.h   |   7 ++
> datapath-windows/ovsext/Ethernet.h |   2 +
> datapath-windows/ovsext/Flow.c |  89 -
> datapath-windows/ovsext/NetProto.h |  33 +++
> datapath-windows/ovsext/PacketParser.c |  12 +--
> datapath-windows/ovsext/PacketParser.h |   7 ++
> 7 files changed, 318 insertions(+), 8 deletions(-)
>
>diff --git a/datapath-windows/ovsext/Actions.c
>b/datapath-windows/ovsext/Actions.c
>index ce592b3..9ee1763 100644
>--- a/datapath-windows/ovsext/Actions.c
>+++ b/datapath-windows/ovsext/Actions.c
>@@ -1074,6 +1074,142 @@ OvsPopVlanInPktBuf(OvsForwardingContext
>*ovsFwdCtx)
> return NDIS_STATUS_SUCCESS;
> }
> 
>+static __inline NDIS_STATUS
>+OvsActionMplsPush(OvsForwardingContext *ovsFwdCtx,
>+  const struct ovs_action_push_mpls *mpls) {
>+NDIS_STATUS status;
>+PNET_BUFFER curNb = NULL;
>+PMDL curMdl = NULL;
>+PUINT8 bufferStart = NULL;
>+OVS_PACKET_HDR_INFO *layers = &ovsFwdCtx->layers;
>+EthHdr *ethHdr = NULL;
>+MPLSHdr *mplsHdr = NULL;
>+UINT32 mdlLen = 0, curMdlOffset = 0;
>+UINT32 packetLen = 0;
>+PNET_BUFFER_LIST newNbl;
>+
>+curNb = NET_BUFFER_LIST_FIRST_NB(ovsFwdCtx->curNbl);
>+ASSERT(curNb->Next == NULL);
>+packetLen = NET_BUFFER_DATA_LENGTH(curNb);
>+curMdl = NET_BUFFER_CURRENT_MDL(curNb);
>+NdisQueryMdl(curMdl, &bufferStart, &mdlLen, LowPagePriority);
>+if (!bufferStart) {
>+ovsActionStats.noResource++;
>+return NDIS_STATUS_RESOURCES;
>+}
>+curMdlOffset = NET_BUFFER_CURRENT_MDL_OFFSET(curNb);
>+mdlLen -= curMdlOffset;
>+mdlLen -= NET_BUFFER_CURRENT_MDL_OFFSET(curNb);
>+ASSERT(mdlLen > 0);
>+/* Bail out if the L2 header is not in a contiguous buffer. */
>+if (MIN(packetLen, mdlLen) < sizeof *ethHdr) {
>+ASSERT(FALSE);
>+return NDIS_STATUS_FAILURE;
>+}
>+ASSERT((INT)mdlLen >= 0);

You don¹t need any of these checks if the NBL is going to be copied anyway. 
These checks are needed if you are going to modify the packet inline. Look at 
OvsUpdateIPv4Header() for correct usage.

>+
>+newNbl = OvsPartialCopyNBL(ovsFwdCtx->switchContext,
>ovsFwdCtx->curNbl,
>+   MPLS_HLEN, 0, TRUE /*copy NBL info*/);

What is the intention of this partial copy? Are you trying to allocate more 
headroom or creating a copy so as to update the L2 header?

You probably want to do:
newNbl = OvsPartialCopyNBL(ovsFwdCtx->switchContext, ovsFwdCtx->curNbl,
   L2 Header size + MPLS_HLEN, MPLS_HLEN, TRUE /*copy 
NBL info*/);


>+if (!newNbl) {
>+ovsActionStats.noCopiedNbl++;
>+return NDIS_STATUS_RESOURCES;
>+}
>+OvsCompleteNBLForwardingCtx(ovsFwdCtx,
>+L"Complete after partial copy.");
>+
>+status = OvsInitForwardingCtx(ovsFwdCtx, ovsFwdCtx->switchContext,
>+  newNbl, ovsFwdCtx->srcVportNo, 0,
>+ 
>NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(newNbl),
>+  NULL, &ovsFwdCtx->layers, FALSE);
>+if (status != NDIS_STATUS_SUCCESS) {
>+OvsCompleteNBLForwardingCtx(ovsFwdCtx,
>+L"OVS-Dropped due to resources");
>+return NDIS_STATUS_RESOURCES;
>+}
>+
>+curNb = NET_BUFFER_LIST_FIRST_NB(ovsFwdCtx->curNbl);
>+ASSERT(curNb->Next == NULL);
>+c

Re: [ovs-dev] [ovs-discuss] [announce] driverctl: utility for persistent alternative driver binding

[Panu Matilainen]

On 12/04/2015 05:44 PM, Gray, Mark D wrote:

I welcome this initiative, one question below:


-Original Message-
From: discuss [mailto:discuss-boun...@openvswitch.org] On Behalf Of Panu
Matilainen
Sent: Friday, December 4, 2015 10:54 AM
To: d...@dpdk.org; us...@dpdk.org; dev@openvswitch.org;
disc...@openvswitch.org
Subject: [ovs-discuss] [announce] driverctl: utility for persistent alternative
driver binding

Hi all,

While this is not directly related to DPDK or OVS, it is potentially
useful for users of both, so excuse me for cross-posting.

Quoting from the project README (for the full text see
http://laiskiainen.org/git/?p=driverctl.git;a=blob_plain;f=README)

  > driverctl is a tool for manipulating and inspecting the system
  > device driver choices.
  >
  > Devices are normally assigned to their sole designated kernel driver
  > by default. However in some situations it may be desireable to
  > override that default, for example to try an older driver to
  > work around a regression in a driver or to try an experimental
  > alternative driver. Another common use-case is pass-through
  > drivers and driver stubs to allow userspace to drive the device,
  > such as in case of virtualization.
  >
  > driverctl integrates with udev to support overriding
  > driver selection for both cold- and hotplugged devices from the
  > moment of discovery, but can also change already assigned drivers,
  > assuming they are not in use by the system. The driver overrides
  > created by driverctl are persistent across system reboots
  > by default.
  >
  > Usage
  > -
  >
  > Find devices currently driven by ixgbe driver:
  >
  > # driverctl -v list-devices | grep ixgbe
  > :01:00.0 ixgbe (Ethernet 10G 4P X520/I350 rNDC)
  > :01:00.1 ixgbe (Ethernet 10G 4P X520/I350 rNDC)
  >
  > Change them to use the vfio-pci driver:
  > # driverctl set-override :01:00.0 vfio-pci
  > # driverctl set-override :01:00.1 vfio-pci
  >
  > Find devices with driver overrides:
  > # driverctl -v list-devices|grep \\*
  > :01:00.0 vfio-pci [*] (Ethernet 10G 4P X520/I350 rNDC)
  > :01:00.1 vfio-pci [*] (Ethernet 10G 4P X520/I350 rNDC)
  >
  > Remove the override from slot :01:00.1:
  > # driverctl unset-override :01:00.1

DPDK of course has its own dpdk_nic_bind(.py) tool for this purpose, the
main differences to driverctl are:
- driverctl bindings are persistent across system boots


  [Gray, Mark D] This is great!

Will this integrate with, for example in Red Hat-based systems, 
/etc/sysconfig/network-scripts/ifcfg-X? In DPDK, could we then
potentially reference devices by that (arbitrary) name?


driverctl is not specific to NICs so network-scripts integration is out 
of scope.


That aside, maybe I'm missing something but I'm not sure what there is 
to integrate with since DPDK ports are ultimately application specific. 
For OVS I've sent a patch to support managing OVS DPDK ports via 
network-scripts: 
http://openvswitch.org/pipermail/dev/2015-December/062850.html


. Panu -


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] 802.1ad code contribution to OVS

[ravulakollu.kumar]

Hi Team ,

We all know that ovs supports 4 types of port configuration .

1.   Access

2.   Trunk

3.   Native-tagged

4.   Native-untagged
Through access port configuration we can achieve 802.1q by adding normal 
flow(Not Openflow) ,we all know that when ovs is configured with normal 
flow(default), ovs
bridge acts as a legacy bridge. We are seeing no support for 802.1ad to 
implement provider-edge bridging.

In an effort to implement the same in ovs and ovs-dpdk, we have written our own 
code to realize 802.1ad (Q-in-Q) . For this
We have introduced a new vlan_mode as "trunk-qinq" .

When you configure a phy port added to ovs as "trunk-qinq" , upon receiving 
cvlan tagged/untagged  traffic it adds outer vlan header with specific outer 
vlan ID ; on egress(other side of
Provider) it removes the outer vlan header and send it out .

Our code changes include control plane and datapath changes.

We want to contribute the same to the openvswitch . Please , let me know whom 
to approach regarding reviews. All comments are appreciated.

Regards,
Uday

The information contained in this electronic message and any attachments to 
this message are intended for the exclusive use of the addressee(s) and may 
contain proprietary, confidential or privileged information. If you are not the 
intended recipient, you should not disseminate, distribute or copy this e-mail. 
Please notify the sender immediately and destroy all copies of this message and 
any attachments. WARNING: Computer viruses can be transmitted via email. The 
recipient should check this email and any attachments for the presence of 
viruses. The company accepts no liability for any damage caused by any virus 
transmitted by this email. www.wipro.com
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] ovn-nbctl:What are router commands like?

[Russell Bryant]

On 12/07/2015 06:46 PM, Ben Pfaff wrote:
> On Mon, Dec 07, 2015 at 01:13:27PM -0500, Russell Bryant wrote:
>> I'm looking at this again and I think it's worth keeping because the
>> output is a bit friendlier.
>>
>> How about supporting both "show LSWITCH" and "show LROUTER".  The
>> implementation would just have to search both tables for a match on name
>> or UUID.
> 
> I guess that lswitch-show and lrouter-show might be OK too.
> 
> For ovs-vsctl, I don't think that "show" supports any arguments at all.
> 

OK, that sounds good to me.

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [ovs] ovn-nbctl:add db commands help (#93)

[Russell Bryant]

On 12/08/2015 01:06 AM, Wei Li wrote:
> 
> 
> 在 2015/12/7 23:30, Russell Bryant 写道:
>>
>> Can you update the man page, as well?
>>
> I would like to do this,but have a question
> 
> There is a reference to /db-ctl-base.man/ in /ovn-sbctl.8.in/  implement
> by "/.so lib/db-ctl-base.man/"
> 
> but ovn-nbctl`s manpage is xml format
> 
> How to add a reference to another manual page in xml file?

Good question.  I don't know of an example you can copy so we may have
to create it.  ovn-sbctl needs the same content, but is written in nroff
instead of the XML format.

Perhaps we need an XML equivalent to db-ctl-base.man and then include it
using xi:include.

...
  
...

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.

[Gray, Mark D]

> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Kevin
> Traynor
> Sent: Monday, December 7, 2015 5:58 PM
> To: dev@openvswitch.org
> Subject: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.
> 
> Add some information about the DPDK -c and -n parameters.
> 
> Signed-off-by: Kevin Traynor 
> Reported-by: Zoltan Kiss 
> ---
>  INSTALL.DPDK.md |   14 --
>  1 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md index 96b686c..ee016da
> 100644
> --- a/INSTALL.DPDK.md
> +++ b/INSTALL.DPDK.md
> @@ -145,8 +145,18 @@ Using the DPDK with ovs-vswitchd:
> 
> DPDK configuration arguments can be passed to vswitchd via `--dpdk`
> argument. This needs to be first argument passed to vswitchd process.
> -   dpdk arg -c is ignored by ovs-dpdk, but it is a required parameter
> -   for dpdk initialization.
> +   The DPDK configuration arguments are passed to DPDK during DPDK
> +   initialization.
> +
> +   The DPDK -c coremask is a required argument. To avoid wasted resources
> +   only one core should be set. The standard OVS threads (e.g. main
> +   process, handler, revalidator) will run on the core that is specified.

Might be worth mentioning that then there is a corresponding potential decrease
in performance of revalidation and flow handling. 

> +
> +   Note: Ideally the -c coremask should not overlap with any pmd-cpu-mask
> +   setting where the DPDK polling threads will run to avoid cpu contention.
> +
> +   The DPDK -n argument is a required argument that is a DPDK optimization
> +   for the number of memory channels. It is typically set to 4.
> 
> ```
> export DB_SOCK=/usr/local/var/run/openvswitch/db.sock
> --
> 1.7.4.1
> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Invoice #CS-49411815

[Liza Witt]

Dear Customer

Your invoice appears below. Please remit payment at your earliest convenience.

Thank you for your business - we appreciate it very much.

Sincerely,
Liza Witt Courier Service
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] FAQ: Add entry about different datapaths features.

[Nithin Raju]

Thanks for putting this together. Like we discussed offline, a lot of the
hyper-v values are subject to change in the near future, but the matrix
represents that state of the Hyper-V datapath as of today.


Acked-by: Nithin Raju 

-Original Message-
From: Daniele Di Proietto 
Date: Monday, December 7, 2015 at 1:00 PM
To: "dev@openvswitch.org" 
Cc: Daniele Di Proietto , Nithin Raju

Subject: [PATCH] FAQ: Add entry about different datapaths features.

>This is an easy way to keep track of the features supported by the
>different datapaths.
>
>Nithin helped filling the list for the Hyper-V port.
>
>CC: Nithin Raju 
>Signed-off-by: Daniele Di Proietto 
>---
> FAQ.md | 64 
>
> 1 file changed, 64 insertions(+)
>
>diff --git a/FAQ.md b/FAQ.md
>index 8397e0f..aaf2026 100644
>--- a/FAQ.md
>+++ b/FAQ.md
>@@ -188,6 +188,70 @@ A: Features are gradually introduced into the
>upstream kernel so unless
> |STT| 2.4+ | 3.5+
> | Conntrack | 2.5+ | 3.10+
> 
>+### Q: Are all features available with all datapaths?
>+
>+A: Open vSwitch supports different datapaths on different platforms.
>Each
>+   datapath has a different feature set: the following tables try to
>summarize
>+   the status.
>+
>+   Supported datapaths:
>+
>+   * *Linux upstream*: The datapath implemented by the kernel module
>shipped
>+   with the upstream Linux kernel.  The numbers here
>refer
>+   to the minimum Linux version that has an
>openvswitch
>+   module supporting a feature.
>+
>+   * *Linux OVS tree*: The datapath implemented by the Linux kernel
>module
>+   distributed with the OVS source tree. The numbers
>here
>+   refer to the minimum Linux kernel version against
>which
>+   the module can be compiled to support a feature.
>+
>+   * *Userspace*: Also known as DPDK, dpif-netdev or dummy datapath.
>+
>+   * *Hyper-V*: Also known as the windows datapath.
>+
>+   The following table lists the datapath supported features from
>+   an Open vSwitch user's perspective.
>+
>+Feature   | Linux upstream | Linux OVS tree | Userspace |
>Hyper-V |
>+--|:--:|:--:|:-:|:---
>:|
>+Connection tracking   |  4.3   |   3.10 |NO |
>NO|
>+Tunnel - LISP |  NO|   YES  |NO |
>NO|
>+Tunnel - STT  |  NO|   3.5  |NO |
>YES   |
>+Tunnel - GRE  |  3.11  |   YES  |YES|
>NO|
>+Tunnel - VXLAN|  3.12  |   YES  |YES|
>YES   |
>+Tunnel - Geneve   |  3.18  |   YES  |YES|
>NO|
>+QoS   |  YES   |   YES  |NO |
>NO|
>+sFlow |  YES   |   YES  |YES|
>NO|
>+Set action|  YES   |   YES  |YES|
>PARTIAL |
>+NIC Bonding   |  YES   |   YES  |YES|
>NO|
>+Multiple VTEPs|  YES   |   YES  |YES|
>NO|
>+
>+   **Notes:** 
>+   * Only a limited set of flow fields is modifiable via the set action
>by the
>+ Hyper-V datapath.
>+   * The Hyper-V datapath only supports one physical NIC per datapath.
>This is
>+ why bonding is not supported.
>+   * The Hyper-V datapath can have at most one IP address configured as a
>+ tunnel endpoint.
>+   
>+   The following table lists features that do not *directly* impact an
>+   Open vSwitch user, e.g. because their absence can be hidden by the
>ofproto
>+   layer (usually this comes with a performance penalty).
>+
>+Feature   | Linux upstream | Linux OVS tree | Userspace |
>Hyper-V |
>+--|:--:|:--:|:-:|:---
>:|
>+SCTP flows|  3.12  |   YES  |YES|
>YES   |
>+MPLS  |  3.19  |   YES  |YES|
>NO|
>+UFID  |  4.0   |   YES  |YES|
>NO|
>+Megaflows |  3.12  |   YES  |YES|
>NO|
>+Masked set action |  4.0   |   YES  |YES|
>NO|
>+Recirculation |  3.19  |   YES  |YES|
>NO|
>+TCP flags matching|  3.13  |   YES  |YES|
>NO|
>+Validate flow actions |  YES   |   YES  |N/A|
>NO|
>+Multiple datapaths|  YES   |   YES  |YES|
>NO|
>+Tunnel TSO - STT  |  N/A   |   YES  |NO |
>YES   |
>+
> ### Q: I get an error like this when I configure Open vSwitch:
> 
>configure: error: Linux kernel in  is version , but
>-- 
>2.1.4
>

___
dev mailing list

Re: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.

[Ben Pfaff]

On Tue, Dec 08, 2015 at 04:41:37PM +, Gray, Mark D wrote:
> > -Original Message-
> > From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Kevin
> > Traynor
> > Sent: Monday, December 7, 2015 5:58 PM
> > To: dev@openvswitch.org
> > Subject: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.
> > 
> > Add some information about the DPDK -c and -n parameters.
> > 
> > Signed-off-by: Kevin Traynor 
> > Reported-by: Zoltan Kiss 
> > ---
> >  INSTALL.DPDK.md |   14 --
> >  1 files changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md index 96b686c..ee016da
> > 100644
> > --- a/INSTALL.DPDK.md
> > +++ b/INSTALL.DPDK.md
> > @@ -145,8 +145,18 @@ Using the DPDK with ovs-vswitchd:
> > 
> > DPDK configuration arguments can be passed to vswitchd via `--dpdk`
> > argument. This needs to be first argument passed to vswitchd process.
> > -   dpdk arg -c is ignored by ovs-dpdk, but it is a required parameter
> > -   for dpdk initialization.
> > +   The DPDK configuration arguments are passed to DPDK during DPDK
> > +   initialization.
> > +
> > +   The DPDK -c coremask is a required argument. To avoid wasted resources
> > +   only one core should be set. The standard OVS threads (e.g. main
> > +   process, handler, revalidator) will run on the core that is specified.
> 
> Might be worth mentioning that then there is a corresponding potential 
> decrease
> in performance of revalidation and flow handling. 

With the kernel datapath, OVS sets up flows and revalidates them on
multiple cores.  You're saying that with DPDK it only uses one core?
Why?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] datapath: Avoid warning for unused static data on Linux <=3.9.0.

[Ben Pfaff]

On Mon, Dec 07, 2015 at 04:27:32PM -0800, Pravin Shelar wrote:
> On Mon, Dec 7, 2015 at 12:34 PM, Ben Pfaff  wrote:
> > Signed-off-by: Ben Pfaff 
> 
> Acked-by: Pravin B Shelar 

Thanks, applied to master and branch-2.5.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] FAQ: Add entry about different datapaths features.

[Justin Pettit]

> On Dec 8, 2015, at 1:01 AM, Gray, Mark D  wrote:
> 
>> +Feature   | Linux upstream | Linux OVS tree | Userspace | 
>> Hyper-V |
>> +--|:--:|:--:|:-:|:-
>> +--|--:|
>> +Connection tracking   |  4.3   |   3.10 |NO |   NO  
>>   |
>> +Tunnel - LISP |  NO|   YES  |NO |   NO  
>>   |
>> +Tunnel - STT  |  NO|   3.5  |NO |   YES 
>>   |
>> +Tunnel - GRE  |  3.11  |   YES  |YES|   NO  
>>   |
>> +Tunnel - VXLAN|  3.12  |   YES  |YES|   YES 
>>   |
>> +Tunnel - Geneve   |  3.18  |   YES  |YES|   NO  
>>   |
>> +QoS   |  YES   |   YES  |NO |   NO  
>>   |
> 
> [Gray, Mark D] 
> QoS is such a loaded term. It may be good to expand on this a bit.

What about breaking it into "Traffic Policing" and "Traffic Shaping"?  Those 
are the two dimensions we usually focus on.

--Justin



___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] FAQ: Add entry about different datapaths features.

[Justin Pettit]

> On Dec 8, 2015, at 8:51 AM, Nithin Raju  wrote:
> 
> Thanks for putting this together. Like we discussed offline, a lot of the
> hyper-v values are subject to change in the near future, but the matrix
> represents that state of the Hyper-V datapath as of today.

That's great to hear.  When a feature is added, we should include an update of 
the matrix as part of the commit.

--Justin


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] 802.1ad code contribution to OVS

[Joe Stringer]

Hi Uday,

On 8 December 2015 at 06:43,  wrote:


We want to contribute the same to the openvswitch . Please , let me know
> whom to approach regarding reviews. All comments are appreciated.


Reviews are conducted on this mailinglist, you can send patches as per the
instructions in CONTRIBUTING.md.


> The information contained in this electronic message and any attachments
> to this message are intended for the exclusive use of the addressee(s) and
> may contain proprietary, confidential or privileged information. If you are
> not the intended recipient, you should not disseminate, distribute or copy
> this e-mail. Please notify the sender immediately and destroy all copies of
> this message and any attachments. WARNING: Computer viruses can be
> transmitted via email. The recipient should check this email and any
> attachments for the presence of viruses. The company accepts no liability
> for any damage caused by any virus transmitted by this email.
> www.wipro.com


This signature doesn't make sense on a public mailinglist.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.

[Gray, Mark D]

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Tuesday, December 8, 2015 4:50 PM
> To: Gray, Mark D
> Cc: Traynor, Kevin; dev@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.
> 
> On Tue, Dec 08, 2015 at 04:41:37PM +, Gray, Mark D wrote:
> > > -Original Message-
> > > From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Kevin
> > > Traynor
> > > Sent: Monday, December 7, 2015 5:58 PM
> > > To: dev@openvswitch.org
> > > Subject: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.
> > >
> > > Add some information about the DPDK -c and -n parameters.
> > >
> > > Signed-off-by: Kevin Traynor 
> > > Reported-by: Zoltan Kiss 
> > > ---
> > >  INSTALL.DPDK.md |   14 --
> > >  1 files changed, 12 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md index
> > > 96b686c..ee016da
> > > 100644
> > > --- a/INSTALL.DPDK.md
> > > +++ b/INSTALL.DPDK.md
> > > @@ -145,8 +145,18 @@ Using the DPDK with ovs-vswitchd:
> > >
> > > DPDK configuration arguments can be passed to vswitchd via `--dpdk`
> > > argument. This needs to be first argument passed to vswitchd process.
> > > -   dpdk arg -c is ignored by ovs-dpdk, but it is a required parameter
> > > -   for dpdk initialization.
> > > +   The DPDK configuration arguments are passed to DPDK during DPDK
> > > +   initialization.
> > > +
> > > +   The DPDK -c coremask is a required argument. To avoid wasted
> resources
> > > +   only one core should be set. The standard OVS threads (e.g. main
> > > +   process, handler, revalidator) will run on the core that is specified.
> >
> > Might be worth mentioning that then there is a corresponding potential
> > decrease in performance of revalidation and flow handling.
> 
> With the kernel datapath, OVS sets up flows and revalidates them on
> multiple cores.  You're saying that with DPDK it only uses one core?
> Why?

In my opinion, the core mask should define the affinities of the other
threads (main, process, handler, revalidator) and the pmd-cpu-mask
should define the affinities of the packet processing threads. However,
I don't know if this was the intended behavior because the name is a little
too generic ("core mask").  If this was the intended behavior, Kevin and I
just did some tests, and it is not behaving like this. 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 0/4] datapath: Backported upstream fixes.

[Pravin Shelar]

On Mon, Dec 7, 2015 at 7:49 PM, Jesse Gross  wrote:
> On Mon, Dec 7, 2015 at 6:23 PM, Pravin B Shelar  wrote:
>> Few important bugfixes for OVS out of tree module.
>>
>> Pravin B Shelar (4):
>>   datapath: Backport: openvswitch: fix hangup on vxlan/gre/geneve
>> device deletion
>>   datapath: Backport: openvswitch: properly refcount vport-vxlan module
>>   datapath: Backport: vxlan: fix incorrect RCO bit in VXLAN header
>>   datapath: Backport: vxlan: interpret IP headers for ECN correctly
>
> All of these look good to me.
>
> Acked-by: Jesse Gross 

I pushed the series to master and branch-2.5.

Thanks.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] FAQ: Add entry about different datapaths features.

[Joe Stringer]

On 7 December 2015 at 13:00, Daniele Di Proietto 
wrote:

> This is an easy way to keep track of the features supported by the
> different datapaths.
>
> Nithin helped filling the list for the Hyper-V port.
>
> CC: Nithin Raju 
> Signed-off-by: Daniele Di Proietto 
>

Thanks for doing this!

This seems to supersede the question immediately before it in the FAQ.
Perhaps we should drop the previous question, or consider rolling some of
the previous wording into the first question here if it's useful.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] datapath-windows: Don't assert for unknown actions

[Ben Pfaff]

Thanks Sairam and Nithin, I applied this to master.

On Tue, Dec 01, 2015 at 01:06:15AM +, Sairam Venugopal wrote:
> Thanks for the patch.
> 
> Acked-by: Sairam Venugopal 
> 
> 
> 
> On 11/25/15, 12:32 PM, "Nithin Raju"  wrote:
> 
> >On Hyper-V, we currently don't validate a flow to see if datapath can
> >indeed execute all the actions specified or not. While support for it
> >gets implemented, an ASSERT seems too strong. I'm working on the support
> >for actions validation. Here's a workaround in the meantime to help
> >debugging.
> >
> >Signed-off-by: Nithin Raju 
> >---
> > datapath-windows/ovsext/Actions.c | 18 +++---
> > 1 file changed, 3 insertions(+), 15 deletions(-)
> >
> >diff --git a/datapath-windows/ovsext/Actions.c
> >b/datapath-windows/ovsext/Actions.c
> >index e902983..7d34458 100644
> >--- a/datapath-windows/ovsext/Actions.c
> >+++ b/datapath-windows/ovsext/Actions.c
> >@@ -1360,22 +1360,10 @@ OvsExecuteSetAction(OvsForwardingContext
> >*ovsFwdCtx,
> > RtlCopyMemory(&ovsFwdCtx->tunKey, &tunKey, sizeof
> >ovsFwdCtx->tunKey);
> > break;
> > }
> >-case OVS_KEY_ATTR_SKB_MARK:
> >-/* XXX: Not relevant to Hyper-V. Return OK */
> >-break;
> >-case OVS_KEY_ATTR_UNSPEC:
> >-case OVS_KEY_ATTR_ENCAP:
> >-case OVS_KEY_ATTR_ETHERTYPE:
> >-case OVS_KEY_ATTR_IN_PORT:
> >-case OVS_KEY_ATTR_VLAN:
> >-case OVS_KEY_ATTR_ICMP:
> >-case OVS_KEY_ATTR_ICMPV6:
> >-case OVS_KEY_ATTR_ARP:
> >-case OVS_KEY_ATTR_ND:
> >-case __OVS_KEY_ATTR_MAX:
> >+
> > default:
> >-OVS_LOG_INFO("Unhandled attribute %#x", type);
> >-ASSERT(FALSE);
> >+OVS_LOG_INFO("Unhandled attribute %#x", type);
> >+break
> > }
> > return status;
> > }
> >-- 
> >1.8.5.6
> >
> >___
> >dev mailing list
> >dev@openvswitch.org
> >https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
> >n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Dc
> >ruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=bJw80vN4kTApMoeiII-yuUy3B3UfaQ
> >kofjX2CR1CwS4&s=bGDKEvKIo16mIe4PTdxwl9ZfQ4L5diag9yZhWMVNRlQ&e= 
> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 1/2] tests: Add vlog tests for C implementation to match Python tests.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 
---
 tests/automake.mk|   2 +
 tests/test-unixctl.c | 186 +++
 tests/vlog.at| 131 
 3 files changed, 319 insertions(+)
 create mode 100644 tests/test-unixctl.c

diff --git a/tests/automake.mk b/tests/automake.mk
index bd06a51..dd981ca 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -170,6 +170,7 @@ valgrind_wrappers = \
tests/valgrind/test-stp \
tests/valgrind/test-type-props \
tests/valgrind/test-unix-socket \
+   tests/valgrind/test-unixctl \
tests/valgrind/test-uuid \
tests/valgrind/test-vconn
 
@@ -313,6 +314,7 @@ tests_ovstest_SOURCES = \
tests/test-sflow.c \
tests/test-sha1.c \
tests/test-stp.c \
+   tests/test-unixctl.c \
tests/test-util.c \
tests/test-uuid.c \
tests/test-bitmap.c \
diff --git a/tests/test-unixctl.c b/tests/test-unixctl.c
new file mode 100644
index 000..bf35b19
--- /dev/null
+++ b/tests/test-unixctl.c
@@ -0,0 +1,186 @@
+/* Copyright (c) 2015 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+
+#include 
+
+#include "command-line.h"
+#include "daemon.h"
+#include "fatal-signal.h"
+#include "openvswitch/vlog.h"
+#include "ovstest.h"
+#include "poll-loop.h"
+#include "unixctl.h"
+#include "util.h"
+
+VLOG_DEFINE_THIS_MODULE(test_unixctl);
+
+static void parse_options(int *argc, char **argvp[], char **unixctl_pathp);
+OVS_NO_RETURN static void usage(void);
+
+static void
+test_unixctl_exit(struct unixctl_conn *conn, int argc OVS_UNUSED,
+  const char *argv[] OVS_UNUSED, void *exiting_)
+{
+bool *exiting = exiting_;
+*exiting = true;
+unixctl_command_reply(conn, NULL);
+}
+
+static void
+test_unixctl_echo(struct unixctl_conn *conn, int argc OVS_UNUSED,
+  const char *argv[], void *aux OVS_UNUSED)
+{
+unixctl_command_reply(conn, argv[1]);
+}
+
+static void
+test_unixctl_echo_error(struct unixctl_conn *conn, int argc OVS_UNUSED,
+const char *argv[], void *aux OVS_UNUSED)
+{
+unixctl_command_reply_error(conn, argv[1]);
+}
+
+static void
+test_unixctl_log(struct unixctl_conn *conn, int argc OVS_UNUSED,
+  const char *argv[], void *aux OVS_UNUSED)
+{
+VLOG_INFO("%s", argv[1]);
+unixctl_command_reply(conn, NULL);
+}
+
+static void
+test_unixctl_block(struct unixctl_conn *conn OVS_UNUSED, int argc OVS_UNUSED,
+   const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
+{
+VLOG_INFO("%s", argv[1]);
+unixctl_command_reply(conn, NULL);
+}
+
+static int
+test_unixctl_main(int argc, char *argv[])
+{
+char *unixctl_path = NULL;
+struct unixctl_server *unixctl;
+bool exiting = false;
+
+ovs_cmdl_proctitle_init(argc, argv);
+set_program_name(argv[0]);
+service_start(&argc, &argv);
+fatal_ignore_sigpipe();
+parse_options(&argc, &argv, &unixctl_path);
+
+daemonize_start(false);
+int retval = unixctl_server_create(unixctl_path, &unixctl);
+if (retval) {
+exit(EXIT_FAILURE);
+}
+unixctl_command_register("exit", "", 0, 0, test_unixctl_exit, &exiting);
+unixctl_command_register("echo", "ARG", 1, 1, test_unixctl_echo, NULL);
+unixctl_command_register("echo_error", "ARG", 1, 1,
+ test_unixctl_echo_error, NULL);
+unixctl_command_register("log", "ARG", 1, 1, test_unixctl_log, NULL);
+unixctl_command_register("block", "", 0, 0, test_unixctl_block, NULL);
+daemonize_complete();
+
+VLOG_INFO("Entering run loop.");
+while (!exiting) {
+unixctl_server_run(unixctl);
+unixctl_server_wait(unixctl);
+if (exiting) {
+poll_immediate_wake();
+}
+poll_block();
+}
+unixctl_server_destroy(unixctl);
+
+service_stop();
+return 0;
+}
+
+static void
+parse_options(int *argcp, char **argvp[], char **unixctl_pathp)
+{
+enum {
+OPT_REMOTE = UCHAR_MAX + 1,
+OPT_UNIXCTL,
+VLOG_OPTION_ENUMS,
+DAEMON_OPTION_ENUMS
+};
+static const struct option long_options[] = {
+{"unixctl", required_argument, NULL, OPT_UNIXCTL},
+{"help",no_argument, NULL, 'h'},
+{"version", no_argument, NULL, 'V'},
+DAEMON_LONG_OPTIONS,
+VLOG_LONG_OPTIONS,
+{NULL, 0, NULL, 0},
+};
+char

[ovs-dev] [PATCH 2/2] vlog: Add vlog/close command.

[Ben Pfaff]

Requested-by: P R Dinesh
Requested-at: https://github.com/openvswitch/ovs/pull/94
Signed-off-by: Ben Pfaff 
---
 lib/vlog.c | 24 ++
 python/ovs/vlog.py | 11 +++-
 tests/vlog.at  | 74 ++
 3 files changed, 108 insertions(+), 1 deletion(-)

diff --git a/lib/vlog.c b/lib/vlog.c
index 28cea5d..2b8ba2b 100644
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -677,6 +677,28 @@ vlog_unixctl_reopen(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
 }
 
 static void
+vlog_unixctl_close(struct unixctl_conn *conn, int argc OVS_UNUSED,
+   const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
+{
+ovs_mutex_lock(&log_file_mutex);
+if (log_fd >= 0) {
+close(log_fd);
+log_fd = -1;
+
+async_append_destroy(log_writer);
+log_writer = NULL;
+
+struct vlog_module *mp;
+LIST_FOR_EACH (mp, list, &vlog_modules) {
+update_min_level(mp);
+}
+}
+ovs_mutex_unlock(&log_file_mutex);
+
+unixctl_command_reply(conn, NULL);
+}
+
+static void
 set_all_rate_limits(bool enable)
 {
 struct vlog_module *mp;
@@ -770,6 +792,8 @@ vlog_init(void)
  0, INT_MAX, vlog_disable_rate_limit, NULL);
 unixctl_command_register("vlog/reopen", "", 0, 0,
  vlog_unixctl_reopen, NULL);
+unixctl_command_register("vlog/close", "", 0, 0,
+ vlog_unixctl_close, NULL);
 
 ovs_rwlock_rdlock(&pattern_rwlock);
 print_syslog_target_deprecation = syslog_fd >= 0;
diff --git a/python/ovs/vlog.py b/python/ovs/vlog.py
index d5741a6..7102503 100644
--- a/python/ovs/vlog.py
+++ b/python/ovs/vlog.py
@@ -1,5 +1,5 @@
 
-# Copyright (c) 2011, 2012, 2013 Nicira, Inc.
+# Copyright (c) 2011, 2012, 2013, 2015 Nicira, Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -238,6 +238,8 @@ class Vlog:
 
 ovs.unixctl.command_register("vlog/reopen", "", 0, 0,
  Vlog._unixctl_vlog_reopen, None)
+ovs.unixctl.command_register("vlog/close", "", 0, 0,
+ Vlog._unixctl_vlog_close, None)
 ovs.unixctl.command_register("vlog/set", "spec", 1, sys.maxint,
  Vlog._unixctl_vlog_set, None)
 ovs.unixctl.command_register("vlog/list", "", 0, 0,
@@ -387,6 +389,13 @@ class Vlog:
 conn.reply("Logging to file not configured")
 
 @staticmethod
+def _unixctl_vlog_close(conn, unused_argv, unused_aux):
+if Vlog.__log_file:
+logger = logging.getLogger("file")
+logger.removeHandler(Vlog.__file_handler)
+conn.reply(None)
+
+@staticmethod
 def _unixctl_vlog_set(conn, argv, unused_aux):
 for arg in argv:
 msg = Vlog.set_levels_from_string(arg)
diff --git a/tests/vlog.at b/tests/vlog.at
index a614647..269bfc5 100644
--- a/tests/vlog.at
+++ b/tests/vlog.at
@@ -255,6 +255,80 @@ AT_CHECK([sed 's/.*|//' log], [0], [dnl
 ])
 AT_CLEANUP
 
+AT_SETUP([vlog - vlog/close - C])
+on_exit 'kill `cat test-unixctl.pid`'
+
+AT_CAPTURE_FILE([log])
+AT_CAPTURE_FILE([log.old])
+AT_CHECK([ovstest test-unixctl --log-file=`pwd`/log --pidfile --detach],
+  [0], [], [stderr])
+AT_CHECK([vlog_filt stderr], [0], [opened log file
+])
+
+AT_CHECK([APPCTL -t test-unixctl log message])
+AT_CHECK([APPCTL -t test-unixctl log message2])
+
+# After closing the log file, message3 won't appear anywhere.
+AT_CHECK([APPCTL -t test-unixctl vlog/close])
+mv log log.old
+AT_CHECK([APPCTL -t test-unixctl log message3])
+
+# Closing the log file again is harmless.
+AT_CHECK([APPCTL -t test-unixctl vlog/close])
+AT_CHECK([APPCTL -t test-unixctl log message4])
+
+# After reopening the log file, further messages start appearing again.
+AT_CHECK([APPCTL -t test-unixctl vlog/reopen])
+AT_CHECK([APPCTL -t test-unixctl log message5])
+AT_CHECK([APPCTL -t test-unixctl exit])
+
+AT_CHECK([vlog_filt log.old], [0], [dnl
+opened log file
+Entering run loop.
+message
+message2
+])
+AT_CHECK([vlog_filt log], [0], [dnl
+opened log file
+message5
+])
+AT_CLEANUP
+
+AT_SETUP([vlog - vlog/close - Python])
+AT_SKIP_IF([test $HAVE_PYTHON = no])
+on_exit 'kill `cat test-unixctl.py.pid`'
+
+AT_CAPTURE_FILE([log])
+AT_CAPTURE_FILE([log.old])
+AT_CHECK([$PYTHON $srcdir/test-unixctl.py --log-file=`pwd`/log --pidfile 
--detach])
+
+AT_CHECK([APPCTL -t test-unixctl.py log message])
+AT_CHECK([APPCTL -t test-unixctl.py log message2])
+
+# After closing the log file, message3 won't appear anywhere.
+AT_CHECK([APPCTL -t test-unixctl.py vlog/close])
+mv log log.old
+AT_CHECK([APPCTL -t test-unixctl.py log message3])
+
+# Closing the log file again is harmless.
+AT_CHECK([APPCTL -t test-unixctl.py vlog/close])
+AT_CHECK([APPCTL -t test-unixctl.py log message4])
+
+# After reopening th

Re: [ovs-dev] [PATCH] datapath-windows: reduce padding size in _OVS_PACKET_HDR_INFO

[Sairam Venugopal]

Acked-by: Sairam Venugopal 


On 12/7/15, 3:13 PM, "Nithin Raju"  wrote:

>Fixing a little buglet from commit: efee3309.
>
>Signed-by: Nithin Raju 
>---
> datapath-windows/ovsext/NetProto.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/datapath-windows/ovsext/NetProto.h
>b/datapath-windows/ovsext/NetProto.h
>index a364869..c60081a 100644
>--- a/datapath-windows/ovsext/NetProto.h
>+++ b/datapath-windows/ovsext/NetProto.h
>@@ -205,7 +205,7 @@ typedef union _OVS_PACKET_HDR_INFO {
> UINT16 tcpCsumNeeded:1;
> UINT16 udpCsumNeeded:1;
> UINT16 udpCsumZero:1;
>-UINT16 pad:9;
>+UINT16 pad:8;
> } ;
> UINT64 value;
> } OVS_PACKET_HDR_INFO, *POVS_PACKET_HDR_INFO;
>-- 
>1.8.5.6
>
>___
>dev mailing list
>dev@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Dc
>ruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=Ae1JC0tageL-QrQatbFKCuAC9qm5z-
>w-Z8-1yYLpkY0&s=nByFJSJRKBYjU49mz2AkyXr8YGHGM7coTHMiH26BBDo&e= 

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] seq: Add a coverage counter for seq_change.

[Jarno Rajahalme]

> On Dec 7, 2015, at 9:34 AM, Ben Pfaff  wrote:
> 
> On Fri, Dec 04, 2015 at 03:57:09PM -0800, Jarno Rajahalme wrote:
>> Having a coverage counter tracking the value of the internal seq_next
>> should help in debugging.
>> 
>> Suggested-by: Justin Pettit 
>> Signed-off-by: Jarno Rajahalme 
> 
> Acked-by: Ben Pfaff 

Thanks for the review. Applied to master. How far back I should back port this?

  Jarno

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] seq: Add a coverage counter for seq_change.

[Justin Pettit]

> On Dec 8, 2015, at 11:39 AM, Jarno Rajahalme  wrote:
> 
> 
>> On Dec 7, 2015, at 9:34 AM, Ben Pfaff  wrote:
>> 
>> On Fri, Dec 04, 2015 at 03:57:09PM -0800, Jarno Rajahalme wrote:
>>> Having a coverage counter tracking the value of the internal seq_next
>>> should help in debugging.
>>> 
>>> Suggested-by: Justin Pettit 
>>> Signed-off-by: Jarno Rajahalme 
>> 
>> Acked-by: Ben Pfaff 
> 
> Thanks for the review. Applied to master. How far back I should back port 
> this?

I think back to 2.3 is enough.

--Justin


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] seq: Add a coverage counter for seq_change.

[Ben Pfaff]

On Tue, Dec 08, 2015 at 11:39:23AM -0800, Jarno Rajahalme wrote:
> 
> > On Dec 7, 2015, at 9:34 AM, Ben Pfaff  wrote:
> > 
> > On Fri, Dec 04, 2015 at 03:57:09PM -0800, Jarno Rajahalme wrote:
> >> Having a coverage counter tracking the value of the internal seq_next
> >> should help in debugging.
> >> 
> >> Suggested-by: Justin Pettit 
> >> Signed-off-by: Jarno Rajahalme 
> > 
> > Acked-by: Ben Pfaff 
> 
> Thanks for the review. Applied to master. How far back I should back
> port this?

2.3 maybe?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovs-ofctl: replace-flows and diff-flows support for multiple tables

[Shashank Shanbhag]

Hi Jarno,

It looks great. We tested it internally to make sure.
I'm not in favor of exclusion lists, as it gets unwieldy if the number of
tables of interest is pretty small (which is the case in our application).
Thanks for this.

Acked-by: Shashank Shanbhag 





-
Shashank Shanbhag,
-

On Thu, Nov 19, 2015 at 1:43 PM, Jarno Rajahalme  wrote:

>
> > On Oct 19, 2015, at 8:50 AM, Ben Pfaff  wrote:
> >
> > On Sun, Oct 18, 2015 at 04:22:22PM -0700, Shashank Shanbhag wrote:
> >> Fix replace-flows and diff-flows to modify/diff flows in multiple
> tables.
> >> Add a --tables(-T) option that allows the user to specify a
> comma-separated
> >> list of table indexes to replace/diff.
> >>
> >> Signed-off-by: Shashank Shanbhag 
> >> Acked-by: Romain Lenglet 
> >
> > Hi Jarno, I know that you were talking about a related bug fix a few
> > days ago.  Can you see if this patch does this same thing?  I reviewed
> > earlier versions of it.  I have not looked at this version yet.
>
>
> Shashank,
>
> I just rebased and sent a version 2 of the replace-flows fix to the list,
> as I felt it was a bit less invasive change. It does not have the tables
> command line parameter, but instead operates on all possible tables. I’m
> not sure if this helps you in your use case, or if you need to explicitly
> leave some tables un-touched by replace-flows. If this is the case, I
> suggest you review this patch (http://patchwork.ozlabs.org/patch/546704/),
> and rebase your patch on top of it, but maybe have a command line option to
> exclude tables instead of listing what tables to process?
>
>   Jarno
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovs-ofctl: replace-flows and diff-flows support for multiple tables

[Jarno Rajahalme]

> On Dec 8, 2015, at 11:43 AM, Shashank Shanbhag  
> wrote:
> 
> Hi Jarno,
> 
> It looks great. We tested it internally to make sure.
> I'm not in favor of exclusion lists, as it gets unwieldy if the number of 
> tables of interest is pretty small (which is the case in our application). 
> Thanks for this.
> 
> Acked-by: Shashank Shanbhag  >
> 

Thanks for the review and testing. However, I was left wondering if this is all 
you need, or you still need to restrict the set of tables the commands apply 
on? Currently, if the file has no flows for a specific table, replace-flows 
will empty that table.

This change is in OVS master and branch-2.5.

  Jarno

> 
> 
> 
> -
> Shashank Shanbhag,
> -
> 
> On Thu, Nov 19, 2015 at 1:43 PM, Jarno Rajahalme  > wrote:
> 
> > On Oct 19, 2015, at 8:50 AM, Ben Pfaff  > > wrote:
> >
> > On Sun, Oct 18, 2015 at 04:22:22PM -0700, Shashank Shanbhag wrote:
> >> Fix replace-flows and diff-flows to modify/diff flows in multiple tables.
> >> Add a --tables(-T) option that allows the user to specify a comma-separated
> >> list of table indexes to replace/diff.
> >>
> >> Signed-off-by: Shashank Shanbhag  >> >
> >> Acked-by: Romain Lenglet  >> >
> >
> > Hi Jarno, I know that you were talking about a related bug fix a few
> > days ago.  Can you see if this patch does this same thing?  I reviewed
> > earlier versions of it.  I have not looked at this version yet.
> 
> 
> Shashank,
> 
> I just rebased and sent a version 2 of the replace-flows fix to the list, as 
> I felt it was a bit less invasive change. It does not have the tables command 
> line parameter, but instead operates on all possible tables. I’m not sure if 
> this helps you in your use case, or if you need to explicitly leave some 
> tables un-touched by replace-flows. If this is the case, I suggest you review 
> this patch (http://patchwork.ozlabs.org/patch/546704/ 
> ), and rebase your patch on top of 
> it, but maybe have a command line option to exclude tables instead of listing 
> what tables to process?
> 
>   Jarno
> 
> 

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] seq: Add a coverage counter for seq_change.

[Jarno Rajahalme]

> On Dec 8, 2015, at 11:41 AM, Ben Pfaff  wrote:
> 
> On Tue, Dec 08, 2015 at 11:39:23AM -0800, Jarno Rajahalme wrote:
>> 
>>> On Dec 7, 2015, at 9:34 AM, Ben Pfaff  wrote:
>>> 
>>> On Fri, Dec 04, 2015 at 03:57:09PM -0800, Jarno Rajahalme wrote:
 Having a coverage counter tracking the value of the internal seq_next
 should help in debugging.
 
 Suggested-by: Justin Pettit 
 Signed-off-by: Jarno Rajahalme 
>>> 
>>> Acked-by: Ben Pfaff 
>> 
>> Thanks for the review. Applied to master. How far back I should back
>> port this?
> 
> 2.3 maybe?

Backported to 2.3, 2.4 and 2.5,

 Jarno

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovs-ofctl: replace-flows and diff-flows support for multiple tables

[Romain Lenglet]

On December 8, 2015 at 12:18:29 PM, Jarno Rajahalme (ja...@ovn.org) wrote:

On Dec 8, 2015, at 11:43 AM, Shashank Shanbhag  
wrote:

Hi Jarno,

It looks great. We tested it internally to make sure.
I'm not in favor of exclusion lists, as it gets unwieldy if the number of 
tables of interest is pretty small (which is the case in our application). 
Thanks for this.

Acked-by: Shashank Shanbhag 



Thanks for the review and testing. However, I was left wondering if this is all 
you need, or you still need to restrict the set of tables the commands apply 
on? Currently, if the file has no flows for a specific table, replace-flows 
will empty that table.
We can work with this behavior for now. Passing the subset of tables to modify 
is an optimization which we can add on top later.

Thanks Jarno!

--Romain Lenglet



This change is in OVS master and branch-2.5.

  Jarno




-
Shashank Shanbhag,
-

On Thu, Nov 19, 2015 at 1:43 PM, Jarno Rajahalme  wrote:

> On Oct 19, 2015, at 8:50 AM, Ben Pfaff  wrote:
>
> On Sun, Oct 18, 2015 at 04:22:22PM -0700, Shashank Shanbhag wrote:
>> Fix replace-flows and diff-flows to modify/diff flows in multiple tables.
>> Add a --tables(-T) option that allows the user to specify a comma-separated
>> list of table indexes to replace/diff.
>>
>> Signed-off-by: Shashank Shanbhag 
>> Acked-by: Romain Lenglet 
>
> Hi Jarno, I know that you were talking about a related bug fix a few
> days ago.  Can you see if this patch does this same thing?  I reviewed
> earlier versions of it.  I have not looked at this version yet.


Shashank,

I just rebased and sent a version 2 of the replace-flows fix to the list, as I 
felt it was a bit less invasive change. It does not have the tables command 
line parameter, but instead operates on all possible tables. I’m not sure if 
this helps you in your use case, or if you need to explicitly leave some tables 
un-touched by replace-flows. If this is the case, I suggest you review this 
patch (http://patchwork.ozlabs.org/patch/546704/), and rebase your patch on top 
of it, but maybe have a command line option to exclude tables instead of 
listing what tables to process?

  Jarno




___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Invoice #55560319

[Sharlene Conrad]

Dear Customer

Your invoice appears below. Please remit payment at your earliest convenience.

Thank you for your business - we appreciate it very much.

Sincerely,
Sharlene ConradCourier Service
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] ovn: Fix ct_state bit mappings in OVN symtab.

[Russell Bryant]

The OVN symbol table contained outdated mappings between connection
states and the corresponding bit in the ct_state field.  This patch
updates the symbol table with the proper values as defined in
lib/packets.h.

Signed-off-by: Russell Bryant 
---
 ovn/controller/lflow.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 38c72c1..764a147 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -58,14 +58,14 @@ symtab_init(void)
 MFF_LOG_REGS;
 #undef MFF_LOG_REG
 
-/* Connection tracking state. */
+/* Connection tracking state. See CS_* in lib/packets.h. */
 expr_symtab_add_field(&symtab, "ct_state", MFF_CT_STATE, NULL, false);
-expr_symtab_add_predicate(&symtab, "ct.trk", "ct_state[7]");
+expr_symtab_add_predicate(&symtab, "ct.trk", "ct_state[5]");
 expr_symtab_add_subfield(&symtab, "ct.new", "ct.trk", "ct_state[0]");
 expr_symtab_add_subfield(&symtab, "ct.est", "ct.trk", "ct_state[1]");
 expr_symtab_add_subfield(&symtab, "ct.rel", "ct.trk", "ct_state[2]");
-expr_symtab_add_subfield(&symtab, "ct.inv", "ct.trk", "ct_state[5]");
-expr_symtab_add_subfield(&symtab, "ct.rpl", "ct.trk", "ct_state[6]");
+expr_symtab_add_subfield(&symtab, "ct.rpl", "ct.trk", "ct_state[3]");
+expr_symtab_add_subfield(&symtab, "ct.inv", "ct.trk", "ct_state[4]");
 
 /* Data fields. */
 expr_symtab_add_field(&symtab, "eth.src", MFF_ETH_SRC, NULL, false);
-- 
2.5.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovn: Fix ct_state bit mappings in OVN symtab.

[Joe Stringer]

On 8 December 2015 at 14:34, Russell Bryant  wrote:
> The OVN symbol table contained outdated mappings between connection
> states and the corresponding bit in the ct_state field.  This patch
> updates the symbol table with the proper values as defined in
> lib/packets.h.
>
> Signed-off-by: Russell Bryant 

Thanks for figuring this out!

Is there a way for these to use the correct constants from OVS? Maybe
they're not exported in an appropriate header right now..


Fixes: 63bc9fb1c69f ("packets: Reorder CS_* flags to remove gap.")
Acked-by: Joe Stringer 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 1/2] datapath: Respect conntrack zone even if invalid.

[Joe Stringer]

If userspace executes ct(zone=1), and the connection tracker determines
that the packet is invalid, then the ct_zone flow key field is populated
with the default zone rather than the zone that was specified. Even
though connection tracking failed, this field should be updated with the
value that userspace specified. Fix the issue.

Fixes: a94ebc39996b ("datapath: Add conntrack action")
Signed-off-by: Joe Stringer 
---
The same bug affects upstream 4.3 and 'net' trees, but I'm finding those to
be unstable right now so I plan to submit that version later in the week.
---
 datapath/conntrack.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index d1bd45ffe007..13a80433ced8 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -149,6 +149,7 @@ static void __ovs_ct_update_key(struct sw_flow_key *key, u8 
state,
  * previously sent the packet to conntrack via the ct action.
  */
 static void ovs_ct_update_key(const struct sk_buff *skb,
+ const struct ovs_conntrack_info *info,
  struct sw_flow_key *key, bool post_ct)
 {
const struct nf_conntrack_zone *zone = &nf_ct_zone_dflt;
@@ -166,13 +167,15 @@ static void ovs_ct_update_key(const struct sk_buff *skb,
zone = nf_ct_zone(ct);
} else if (post_ct) {
state = OVS_CS_F_TRACKED | OVS_CS_F_INVALID;
+   if (info)
+   zone = &info->zone;
}
__ovs_ct_update_key(key, state, zone, ct);
 }
 
 void ovs_ct_fill_key(const struct sk_buff *skb, struct sw_flow_key *key)
 {
-   ovs_ct_update_key(skb, key, false);
+   ovs_ct_update_key(skb, NULL, key, false);
 }
 
 int ovs_ct_put_key(const struct sw_flow_key *key, struct sk_buff *skb)
@@ -431,7 +434,7 @@ static int __ovs_ct_lookup(struct net *net, struct 
sw_flow_key *key,
}
}
 
-   ovs_ct_update_key(skb, key, true);
+   ovs_ct_update_key(skb, info, key, true);
 
return 0;
 }
-- 
2.1.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 2/2] datapath: Define nf_connlabels_{put,get}.

[Joe Stringer]

Previously this was only done when connlabels were enabled in the kernel
config, even if the functions didn't exist. Fix the compile error.

Reported-by: Simon Horman 
Signed-off-by: Joe Stringer 
---
 .../compat/include/net/netfilter/nf_conntrack_labels.h | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/datapath/linux/compat/include/net/netfilter/nf_conntrack_labels.h 
b/datapath/linux/compat/include/net/netfilter/nf_conntrack_labels.h
index e49a289c5388..88413e763c4e 100644
--- a/datapath/linux/compat/include/net/netfilter/nf_conntrack_labels.h
+++ b/datapath/linux/compat/include/net/netfilter/nf_conntrack_labels.h
@@ -5,8 +5,8 @@
 #include 
 #include_next 
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(4,3,0) && \
-IS_ENABLED(CONFIG_NF_CONNTRACK_LABELS)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(4,3,0)
+#if IS_ENABLED(CONFIG_NF_CONNTRACK_LABELS)
 
 #ifndef NF_CT_LABELS_MAX_SIZE
 #define NF_CT_LABELS_MAX_SIZE ((XT_CONNLABEL_MAXBIT + 1) / BITS_PER_BYTE)
@@ -37,5 +37,13 @@ static inline void nf_connlabels_put(struct net *net)
net->ct.label_words = 0;
 }
 
-#endif
+#else /* CONFIG_NF_CONNTRACK_LABELS */
+static inline int nf_connlabels_get(struct net *net, unsigned int n_bits)
+{
+   return -ERANGE;
+}
+
+static inline void nf_connlabels_put(struct net *net) { }
+#endif /* CONFIG_NF_CONNTRACK_LABELS */
+#endif /* 4.3 */
 #endif /* _NF_CONNTRACK_LABELS_WRAPPER_H */
-- 
2.1.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH net-next v4 1/8] netfilter: Remove IP_CT_NEW_REPLY definition.

[Jarno Rajahalme]

Remove the definition of IP_CT_NEW_REPLY from the kernel as it does
not make sense.  This allows the definition of IP_CT_NUMBER to be
simplified as well.

Signed-off-by: Jarno Rajahalme 
---
 include/uapi/linux/netfilter/nf_conntrack_common.h | 12 +---
 net/openvswitch/conntrack.c|  2 --
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/netfilter/nf_conntrack_common.h 
b/include/uapi/linux/netfilter/nf_conntrack_common.h
index 319f471..6d074d1 100644
--- a/include/uapi/linux/netfilter/nf_conntrack_common.h
+++ b/include/uapi/linux/netfilter/nf_conntrack_common.h
@@ -20,9 +20,15 @@ enum ip_conntrack_info {
 
IP_CT_ESTABLISHED_REPLY = IP_CT_ESTABLISHED + IP_CT_IS_REPLY,
IP_CT_RELATED_REPLY = IP_CT_RELATED + IP_CT_IS_REPLY,
-   IP_CT_NEW_REPLY = IP_CT_NEW + IP_CT_IS_REPLY,   
-   /* Number of distinct IP_CT types (no NEW in reply dirn). */
-   IP_CT_NUMBER = IP_CT_IS_REPLY * 2 - 1
+   /* No NEW in reply direction. */
+
+   /* Number of distinct IP_CT types. */
+   IP_CT_NUMBER,
+
+   /* only for userspace compatibility */
+#ifndef __KERNEL__
+   IP_CT_NEW_REPLY = IP_CT_NUMBER,
+#endif
 };
 
 #define NF_CT_STATE_INVALID_BIT(1 << 0)
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index c2cc111..a28a819 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -73,7 +73,6 @@ static u8 ovs_ct_get_state(enum ip_conntrack_info ctinfo)
switch (ctinfo) {
case IP_CT_ESTABLISHED_REPLY:
case IP_CT_RELATED_REPLY:
-   case IP_CT_NEW_REPLY:
ct_state |= OVS_CS_F_REPLY_DIR;
break;
default:
@@ -90,7 +89,6 @@ static u8 ovs_ct_get_state(enum ip_conntrack_info ctinfo)
ct_state |= OVS_CS_F_RELATED;
break;
case IP_CT_NEW:
-   case IP_CT_NEW_REPLY:
ct_state |= OVS_CS_F_NEW;
break;
default:
-- 
2.1.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH net-next v4 0/8] openvswitch: NAT support.

[Jarno Rajahalme]

This series adds NAT support to openvswitch kernel module.  A few
changes are needed to the netfilter code to facilitate this (patches
1-3/8).  Patches 4-7 make the openvswitch kernel module ready for the
patch 8 that adds the NAT support by calling into netfilter NAT code
from the openvswitch conntrack action.

The OVS master now has the corresponding OVS userspace support to use
and test the NAT features.  Below if a walk through of a simple use
case.

In this case ports 1 and 2 are in different namespaces.  The OpenFlow
table below only allows IPv4 connections initiated from port 1, and
applies source NAT to those connections:

   in_port=1,ip,action=ct(commit,zone=1,nat(src=10.1.1.240-10.1.1.255)),2
   in_port=2,ct_state=-trk,ip,action=ct(table=0,zone=1,nat)
   in_port=2,ct_state=+est,ct_zone=1,ip,action=1

This flow table matches all IPv4 traffic from port 1, runs them
through conntrack in zone 1 and NATs them.  The NAT is initialized to
do source IP mapping to the given range for the first packet of each
connection, after which the new connection is committed (confirmed).
For further packets of already tracked connections NAT is done
according to the connection state and the commit is a no-op.  Each
packet that is not flagged as a drop by the CT action is forwarded to
port 2.  The CT action does an implicit fragmentation reassembly, so
that only complete packets are run through conntrack.  Reassembled
packets are re-fragmented on output.

The IPv4 traffic coming from port 2 is first matched for the
non-tracked state (-trk), which means that the packet has not been
through a CT action yet.  Such traffic is run trough the conntrack in
zone 1 and all packets associated with a NATted connection are NATted
also in the return direction.  After the packet has been through
conntrack it is recirculated back to OpenFlow table 0 (which is the
default table, so all the rules above are in table 0).  The CT action
changes the 'trk' flag to being set, so the packets after
recirculation no longer match the second rule.  The third rule then
matches the recirculated packets that were marked as established by
conntrack (+est), and the packet is output on port 1.  Matching on
ct_zone is not strictly needed, but in this test case it verifies that
the ct_zone key attribute is properly set by the conntrack action.

A full test case requires rules for ARP handling not shown here.

The flow table above is an OpenFlow table, and the rules therein
are translated to kernel flow entries on-demand by ovs-vswitchd.

Jarno Rajahalme (8):
  netfilter: Remove IP_CT_NEW_REPLY definition.
  netfilter: Factor out nf_ct_get_info().
  netfilter: Allow calling into nat helper without skb_dst.
  openvswitch: Update the CT state key only after nf_conntrack_in().
  openvswitch: Find existing conntrack entry after upcall.
  openvswitch: Handle NF_REPEAT in conntrack action.
  openvswitch: Delay conntrack helper call for new connections.
  openvswitch: Interface with NAT.

 include/net/netfilter/nf_conntrack.h   |  15 +
 include/uapi/linux/netfilter/nf_conntrack_common.h |  12 +-
 include/uapi/linux/openvswitch.h   |  47 ++
 net/ipv4/netfilter/nf_nat_l3proto_ipv4.c   |  29 +-
 net/ipv6/netfilter/nf_nat_l3proto_ipv6.c   |  29 +-
 net/netfilter/nf_conntrack_core.c  |  22 +-
 net/openvswitch/conntrack.c| 632 +++--
 net/openvswitch/conntrack.h|   3 +-
 8 files changed, 686 insertions(+), 103 deletions(-)

-- 
2.1.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH net-next v4 2/8] netfilter: Factor out nf_ct_get_info().

[Jarno Rajahalme]

Define a new inline function to map conntrack status to enum
ip_conntrack_info.  This removes the need to otherwise duplicate this
code in a later patch ("openvswitch: Find existing conntrack entry
after upcall.").

Signed-off-by: Jarno Rajahalme 
---
 include/net/netfilter/nf_conntrack.h | 15 +++
 net/netfilter/nf_conntrack_core.c| 22 +++---
 2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack.h 
b/include/net/netfilter/nf_conntrack.h
index fde4068..b3de10e 100644
--- a/include/net/netfilter/nf_conntrack.h
+++ b/include/net/netfilter/nf_conntrack.h
@@ -125,6 +125,21 @@ nf_ct_tuplehash_to_ctrack(const struct 
nf_conntrack_tuple_hash *hash)
tuplehash[hash->tuple.dst.dir]);
 }
 
+static inline enum ip_conntrack_info
+nf_ct_get_info(const struct nf_conntrack_tuple_hash *h)
+{
+   const struct nf_conn *ct = nf_ct_tuplehash_to_ctrack(h);
+
+   if (NF_CT_DIRECTION(h) == IP_CT_DIR_REPLY)
+   return IP_CT_ESTABLISHED_REPLY;
+   /* Once we've had two way comms, always ESTABLISHED. */
+   if (test_bit(IPS_SEEN_REPLY_BIT, &ct->status))
+   return IP_CT_ESTABLISHED;
+   if (test_bit(IPS_EXPECTED_BIT, &ct->status))
+   return IP_CT_RELATED;
+   return IP_CT_NEW;
+}
+
 static inline u_int16_t nf_ct_l3num(const struct nf_conn *ct)
 {
return ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.l3num;
diff --git a/net/netfilter/nf_conntrack_core.c 
b/net/netfilter/nf_conntrack_core.c
index 3cb3cb8..70ddbd8 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1056,25 +1056,9 @@ resolve_normal_ct(struct net *net, struct nf_conn *tmpl,
ct = nf_ct_tuplehash_to_ctrack(h);
 
/* It exists; we have (non-exclusive) reference. */
-   if (NF_CT_DIRECTION(h) == IP_CT_DIR_REPLY) {
-   *ctinfo = IP_CT_ESTABLISHED_REPLY;
-   /* Please set reply bit if this packet OK */
-   *set_reply = 1;
-   } else {
-   /* Once we've had two way comms, always ESTABLISHED. */
-   if (test_bit(IPS_SEEN_REPLY_BIT, &ct->status)) {
-   pr_debug("nf_conntrack_in: normal packet for %p\n", ct);
-   *ctinfo = IP_CT_ESTABLISHED;
-   } else if (test_bit(IPS_EXPECTED_BIT, &ct->status)) {
-   pr_debug("nf_conntrack_in: related packet for %p\n",
-ct);
-   *ctinfo = IP_CT_RELATED;
-   } else {
-   pr_debug("nf_conntrack_in: new packet for %p\n", ct);
-   *ctinfo = IP_CT_NEW;
-   }
-   *set_reply = 0;
-   }
+   *ctinfo = nf_ct_get_info(h);
+   *set_reply = NF_CT_DIRECTION(h) == IP_CT_DIR_REPLY;
+
skb->nfct = &ct->ct_general;
skb->nfctinfo = *ctinfo;
return ct;
-- 
2.1.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH net-next v4 3/8] netfilter: Allow calling into nat helper without skb_dst.

[Jarno Rajahalme]

NAT checksum recalculation code assumes existence of skb_dst, which
becomes a problem for a later patch in the series ("openvswitch:
Interface with NAT.").  Simplify this by removing the check on
skb_dst, as the checksum will be dealt with later in the stack.

Suggested-by: Pravin Shelar 
Signed-off-by: Jarno Rajahalme 
---
 net/ipv4/netfilter/nf_nat_l3proto_ipv4.c | 29 -
 net/ipv6/netfilter/nf_nat_l3proto_ipv6.c | 29 -
 2 files changed, 16 insertions(+), 42 deletions(-)

diff --git a/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c 
b/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c
index 5075b7e..f8aad03 100644
--- a/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c
@@ -127,28 +127,15 @@ static void nf_nat_ipv4_csum_recalc(struct sk_buff *skb,
u8 proto, void *data, __sum16 *check,
int datalen, int oldlen)
 {
-   const struct iphdr *iph = ip_hdr(skb);
-   struct rtable *rt = skb_rtable(skb);
-
if (skb->ip_summed != CHECKSUM_PARTIAL) {
-   if (!(rt->rt_flags & RTCF_LOCAL) &&
-   (!skb->dev || skb->dev->features & NETIF_F_V4_CSUM)) {
-   skb->ip_summed = CHECKSUM_PARTIAL;
-   skb->csum_start = skb_headroom(skb) +
- skb_network_offset(skb) +
- ip_hdrlen(skb);
-   skb->csum_offset = (void *)check - data;
-   *check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
-   datalen, proto, 0);
-   } else {
-   *check = 0;
-   *check = csum_tcpudp_magic(iph->saddr, iph->daddr,
-  datalen, proto,
-  csum_partial(data, datalen,
-   0));
-   if (proto == IPPROTO_UDP && !*check)
-   *check = CSUM_MANGLED_0;
-   }
+   const struct iphdr *iph = ip_hdr(skb);
+
+   skb->ip_summed = CHECKSUM_PARTIAL;
+   skb->csum_start = skb_headroom(skb) + skb_network_offset(skb) +
+   ip_hdrlen(skb);
+   skb->csum_offset = (void *)check - data;
+   *check = ~csum_tcpudp_magic(iph->saddr, iph->daddr, datalen,
+   proto, 0);
} else
inet_proto_csum_replace2(check, skb,
 htons(oldlen), htons(datalen), true);
diff --git a/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c 
b/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
index 238e70c..e0be97e 100644
--- a/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
+++ b/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
@@ -131,28 +131,15 @@ static void nf_nat_ipv6_csum_recalc(struct sk_buff *skb,
u8 proto, void *data, __sum16 *check,
int datalen, int oldlen)
 {
-   const struct ipv6hdr *ipv6h = ipv6_hdr(skb);
-   struct rt6_info *rt = (struct rt6_info *)skb_dst(skb);
-
if (skb->ip_summed != CHECKSUM_PARTIAL) {
-   if (!(rt->rt6i_flags & RTF_LOCAL) &&
-   (!skb->dev || skb->dev->features & NETIF_F_V6_CSUM)) {
-   skb->ip_summed = CHECKSUM_PARTIAL;
-   skb->csum_start = skb_headroom(skb) +
- skb_network_offset(skb) +
- (data - (void *)skb->data);
-   skb->csum_offset = (void *)check - data;
-   *check = ~csum_ipv6_magic(&ipv6h->saddr, &ipv6h->daddr,
- datalen, proto, 0);
-   } else {
-   *check = 0;
-   *check = csum_ipv6_magic(&ipv6h->saddr, &ipv6h->daddr,
-datalen, proto,
-csum_partial(data, datalen,
- 0));
-   if (proto == IPPROTO_UDP && !*check)
-   *check = CSUM_MANGLED_0;
-   }
+   const struct ipv6hdr *ipv6h = ipv6_hdr(skb);
+
+   skb->ip_summed = CHECKSUM_PARTIAL;
+   skb->csum_start = skb_headroom(skb) + skb_network_offset(skb) +
+   (data - (void *)skb->data);
+   skb->csum_offset = (void *)check - data;
+   *check = ~csum_ipv6_magic(&ipv6h->saddr, &ipv6h->daddr,
+ datalen, proto, 0);
} else
inet_proto_csum_replace2(check, skb,
 htons

[ovs-dev] [PATCH net-next v4 4/8] openvswitch: Update the CT state key only after nf_conntrack_in().

[Jarno Rajahalme]

Only a successful nf_conntrack_in() call can effect a connection state
change, so if suffices to update the key only after the
nf_conntrack_in() returns.

This change is needed for the later NAT patches.

Signed-off-by: Jarno Rajahalme 
---
 net/openvswitch/conntrack.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index a28a819..10f4a6e 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -194,7 +194,6 @@ static int ovs_ct_set_mark(struct sk_buff *skb, struct 
sw_flow_key *key,
struct nf_conn *ct;
u32 new_mark;
 
-
/* The connection could be invalid, in which case set_mark is no-op. */
ct = nf_ct_get(skb, &ctinfo);
if (!ct)
@@ -385,6 +384,10 @@ static bool skb_nfct_cached(const struct net *net, const 
struct sk_buff *skb,
return true;
 }
 
+/* Pass 'skb' through conntrack in 'net', using zone configured in 'info', if
+ * not done already.  Update key with new CT state after passing the packet
+ * through conntrack.
+ */
 static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key,
   const struct ovs_conntrack_info *info,
   struct sk_buff *skb)
@@ -410,14 +413,14 @@ static int __ovs_ct_lookup(struct net *net, struct 
sw_flow_key *key,
skb) != NF_ACCEPT)
return -ENOENT;
 
+   ovs_ct_update_key(skb, key, true);
+
if (ovs_ct_helper(skb, info->family) != NF_ACCEPT) {
WARN_ONCE(1, "helper rejected packet");
return -EINVAL;
}
}
 
-   ovs_ct_update_key(skb, key, true);
-
return 0;
 }
 
-- 
2.1.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH net-next v4 5/8] openvswitch: Find existing conntrack entry after upcall.

[Jarno Rajahalme]

Add a new function ovs_ct_find_existing() to find an existing
conntrack entry for which this packet was already applied to.  This is
only to be called when there is evidence that the packet was already
tracked and committed, but we lost the ct reference due to an
userspace upcall.

ovs_ct_find_existing() is called from skb_nfct_cached(), which can now
hide the fact that the ct reference may have been lost due to an
upcall.  This allows ovs_ct_commit() to be simplified.

This patch is needed by later "openvswitch: Interface with NAT" patch,
as we need to be able to pass the packet through NAT using the
original ct reference also after the reference is lost after an
upcall.

Signed-off-by: Jarno Rajahalme 
---
 net/openvswitch/conntrack.c | 95 ++---
 1 file changed, 82 insertions(+), 13 deletions(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 10f4a6e..0c371d0 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -359,16 +359,87 @@ ovs_ct_expect_find(struct net *net, const struct 
nf_conntrack_zone *zone,
return __nf_ct_expect_find(net, zone, &tuple);
 }
 
+/* Find an existing conntrack entry for which this packet was already applied
+ * to.  This is only called when there is evidence that the packet was already
+ * tracked and commited, but we lost the ct reference due to an userspace
+ * upcall. This means that on entry skb->nfct is NULL.
+ * On success, returns conntrack ptr, sets skb->nfct and ctinfo.
+ * Must be called rcu_read_lock()ed. */
+static struct nf_conn *
+ovs_ct_find_existing(struct net *net, const struct nf_conntrack_zone *zone,
+u_int8_t l3num, struct sk_buff *skb,
+enum ip_conntrack_info *ctinfo)
+{
+   struct nf_conntrack_l3proto *l3proto;
+   struct nf_conntrack_l4proto *l4proto;
+   struct nf_conntrack_tuple tuple;
+   struct nf_conntrack_tuple_hash *h;
+   struct nf_conn *ct;
+   unsigned int dataoff;
+   u_int8_t protonum;
+
+   BUG_ON(skb->nfct != NULL);
+
+   l3proto = __nf_ct_l3proto_find(l3num);
+   if (!l3proto) {
+   pr_debug("ovs_ct_find_existing: Can't get l3proto\n");
+   return NULL;
+   }
+   if (l3proto->get_l4proto(skb, skb_network_offset(skb), &dataoff,
+&protonum) <= 0) {
+   pr_debug("ovs_ct_find_existing: Can't get protonum\n");
+   return NULL;
+   }
+   l4proto = __nf_ct_l4proto_find(l3num, protonum);
+   if (!l4proto) {
+   pr_debug("ovs_ct_find_existing: Can't get l4proto\n");
+   return NULL;
+   }
+   if (!nf_ct_get_tuple(skb, skb_network_offset(skb), dataoff, l3num,
+protonum, net, &tuple, l3proto, l4proto)) {
+   pr_debug("ovs_ct_find_existing: Can't get tuple\n");
+   return NULL;
+   }
+
+   /* look for tuple match */
+   h = nf_conntrack_find_get(net, zone, &tuple);
+   if (!h)
+   return NULL;   /* Not found. */
+
+   ct = nf_ct_tuplehash_to_ctrack(h);
+
+   *ctinfo = nf_ct_get_info(h);
+   if (*ctinfo == IP_CT_NEW) {
+   /* This should not happen. */
+   WARN_ONCE(1, "ovs_ct_find_existing: new packet for %p\n", ct);
+   }
+   skb->nfct = &ct->ct_general;
+   skb->nfctinfo = *ctinfo;
+   return ct;
+}
+
 /* Determine whether skb->nfct is equal to the result of conntrack lookup. */
-static bool skb_nfct_cached(const struct net *net, const struct sk_buff *skb,
-   const struct ovs_conntrack_info *info)
+static bool skb_nfct_cached(struct net *net,
+   const struct sw_flow_key *key,
+   const struct ovs_conntrack_info *info,
+   struct sk_buff *skb)
 {
enum ip_conntrack_info ctinfo;
struct nf_conn *ct;
 
ct = nf_ct_get(skb, &ctinfo);
+   /* If no ct, check if we have evidence that an existing conntrack entry
+* might be found for this skb.  This happens when we lose a skb->nfct
+* due to an upcall.  If the connection was not confirmed, it is not
+* cached and needs to be run through conntrack again. */
+   if (!ct && key->ct.state & OVS_CS_F_TRACKED
+   && !(key->ct.state & OVS_CS_F_INVALID)
+   && key->ct.zone == info->zone.id)
+   ct = ovs_ct_find_existing(net, &info->zone, info->family, skb,
+ &ctinfo);
if (!ct)
return false;
+
if (!net_eq(net, read_pnet(&ct->ct_net)))
return false;
if (!nf_ct_zone_equal_any(info->ct, nf_ct_zone(ct)))
@@ -397,7 +468,7 @@ static int __ovs_ct_lookup(struct net *net, struct 
sw_flow_key *key,
 * actually run the packet through conntrack twice unless it's for a
 * different zone.
 */
-   

[ovs-dev] [PATCH 03/14] packets: Add new functions for IPv4 and IPv6 address parsing.

[Ben Pfaff]

These will be used in an upcoming patch to reduce duplicated code.

Signed-off-by: Ben Pfaff 
---
 lib/packets.c | 132 ++
 lib/packets.h |   7 
 2 files changed, 102 insertions(+), 37 deletions(-)

diff --git a/lib/packets.c b/lib/packets.c
index 0ad5073..309ec0c 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -416,6 +416,14 @@ ip_format_masked(ovs_be32 ip, ovs_be32 mask, struct ds *s)
 }
 }
 
+/* Parses string 's', which must be an IP address.  Stores the IP address into
+ * '*ip'.  Returns true if successful, otherwise false. */
+bool
+ip_parse(const char *s, ovs_be32 *ip)
+{
+return inet_pton(AF_INET, s, ip) == 1;
+}
+
 /* Parses string 's', which must be an IP address with an optional netmask or
  * CIDR prefix length.  Stores the IP address into '*ip' and the netmask into
  * '*mask'.  (If 's' does not contain a netmask, 255.255.255.255 is
@@ -447,6 +455,93 @@ ip_parse_masked(const char *s, ovs_be32 *ip, ovs_be32 
*mask)
 return NULL;
 }
 
+/* Similar to ip_parse_masked(), but the mask, if present, must be a CIDR mask
+ * and is returned as a prefix length in '*plen'. */
+char * OVS_WARN_UNUSED_RESULT
+ip_parse_cidr(const char *s, ovs_be32 *ip, unsigned int *plen)
+{
+ovs_be32 mask;
+char *error;
+
+error = ip_parse_masked(s, ip, &mask);
+if (error) {
+return error;
+}
+
+if (!ip_is_cidr(mask)) {
+return xasprintf("%s: CIDR network required", s);
+}
+*plen = ip_count_cidr_bits(mask);
+return NULL;
+}
+
+/* Parses string 's', which must be an IPv6 address.  Stores the IPv6 address
+ * into '*ip'.  Returns true if successful, otherwise false. */
+bool
+ipv6_parse(const char *s, struct in6_addr *ip)
+{
+return inet_pton(AF_INET6, s, ip) == 1;
+}
+
+/* Parses string 's', which must be an IPv6 address with an optional netmask or
+ * CIDR prefix length.  Stores the IPv6 address into '*ip' and the netmask into
+ * '*mask'.  (If 's' does not contain a netmask, all-one-bits is assumed.)
+ *
+ * Returns NULL if successful, otherwise an error message that the caller must
+ * free(). */
+char * OVS_WARN_UNUSED_RESULT
+ipv6_parse_masked(const char *s, struct in6_addr *ip, struct in6_addr *mask)
+{
+char ipv6_s[IPV6_SCAN_LEN + 1];
+int prefix;
+int n;
+
+if (ovs_scan(s, IPV6_SCAN_FMT"%n", ipv6_s, &n) && ipv6_parse(ipv6_s, ip)) {
+s += n;
+if (!*s) {
+*mask = in6addr_exact;
+} else if (ovs_scan(s, "/%d%n", &prefix, &n) && !s[n]) {
+if (prefix <= 0 || prefix > 128) {
+return xasprintf("%s: IPv6 network prefix bits not between 0 "
+ "and 128", s);
+}
+*mask = ipv6_create_mask(prefix);
+} else if (ovs_scan(s, "/"IPV6_SCAN_FMT"%n", ipv6_s, &n)
+   && !s[n]
+   && ipv6_parse(ipv6_s, mask)) {
+/* OK. */
+} else {
+return xasprintf("%s: syntax error expecting IPv6 prefix length "
+ "or mask", s);
+}
+return NULL;
+}
+return xasprintf("%s: invalid IPv6 address", s);
+}
+
+/* Similar to ipv6_parse_masked(), but the mask, if present, must be a CIDR
+ * mask and is returned as a prefix length in '*plen'. */
+char * OVS_WARN_UNUSED_RESULT
+ipv6_parse_cidr(const char *s, struct in6_addr *ip, unsigned int *plen)
+{
+struct in6_addr mask;
+char *error;
+
+error = ipv6_parse_masked(s, ip, &mask);
+if (error) {
+return error;
+}
+
+if (!ipv6_is_cidr(&mask)) {
+return xasprintf("%s: IPv6 CIDR network required", s);
+}
+*plen = ipv6_count_cidr_bits(&mask);
+return NULL;
+}
+
+/* Stores the string representation of the IPv6 address 'addr' into the
+ * character array 'addr_str', which must be at least INET6_ADDRSTRLEN
+ * bytes long. */
 void
 ipv6_format_addr(const struct in6_addr *addr, struct ds *s)
 {
@@ -612,43 +707,6 @@ ipv6_is_cidr(const struct in6_addr *netmask)
 return true;
 }
 
-/* Parses string 's', which must be an IPv6 address with an optional
- * CIDR prefix length.  Stores the IP address into '*ipv6' and the CIDR
- * prefix in '*prefix'.  (If 's' does not contain a CIDR length, all-ones
- * is assumed.)
- *
- * Returns NULL if successful, otherwise an error message that the caller must
- * free(). */
-char * OVS_WARN_UNUSED_RESULT
-ipv6_parse_masked(const char *s, struct in6_addr *ipv6, struct in6_addr *mask)
-{
-char ipv6_s[IPV6_SCAN_LEN + 1];
-char mask_s[IPV6_SCAN_LEN + 1];
-int prefix;
-int n;
-
-if (ovs_scan(s, IPV6_SCAN_FMT"/"IPV6_SCAN_FMT"%n", ipv6_s, mask_s, &n)
-&& inet_pton(AF_INET6, ipv6_s, ipv6) == 1
-&& inet_pton(AF_INET6, mask_s, mask) == 1
-&& !s[n]) {
-/* OK. */
-} else if (ovs_scan(s, IPV6_SCAN_FMT"/%d%n", ipv6_s, &prefix, &n)
-&& inet_pton(AF_INET6, ipv6_s, ipv6) == 1
-&& !s[n]) {
-if (

[ovs-dev] [PATCH 00/14] ovn: Implement basic ARP support for L3 logical routers

[Ben Pfaff]

Ben Pfaff (14):
  sparse: Define INET_ADDRSTRLEN.
  packets: New macro ETH_ADDR_STRLEN.
  packets: Add new functions for IPv4 and IPv6 address parsing.
  Use ip_parse() and ipv6_parse() and variants in more places.
  Add OpenFlow support for new "arp" action.
  actions: Factor out new helper function add_prerequisite().
  actions: Factor parsing a single action into a new function
parse_action().
  actions: Implement OVN "arp" action.
  actions: Bundle action parsing parameters into a structure.
  ovn: Use callback function instead of simap for logical port number
map.
  ovn-controller: Add data structure for indexing lports, multicast
groups.
  expr: Generalize wording of error message in expand_symbol().
  ovn: Reorganize action parsing test.
  ovn: Implement basic ARP support for L3 logical routers.

 NEWS  |   2 +
 datapath/linux/compat/include/linux/openvswitch.h |   5 +-
 include/sparse/netinet/in.h   |   1 +
 lib/bfd.c |   2 +-
 lib/dpif-netdev.c |   3 +-
 lib/dpif.c|   1 +
 lib/netdev-dummy.c|  14 +-
 lib/odp-execute.c |  26 ++
 lib/odp-util.c|  12 +
 lib/ofp-actions.c | 129 ++-
 lib/ofp-actions.h |   3 +-
 lib/ofp-errors.h  |   4 +
 lib/ovs-router.c  |  59 +---
 lib/packets.c | 170 +++---
 lib/packets.h |   9 +
 lib/socket-util.c |  10 +-
 ofproto/ofproto-dpif-sflow.c  |   4 +
 ofproto/ofproto-dpif-xlate.c  |  70 
 ovn/TODO  |  55 +--
 ovn/controller/automake.mk|   2 +
 ovn/controller/lflow.c| 242 +++--
 ovn/controller/lflow.h|   8 +-
 ovn/controller/lport.c| 157 +
 ovn/controller/lport.h|  69 
 ovn/controller/ovn-controller.c   |  42 ++-
 ovn/controller/pinctrl.c  | 206 +--
 ovn/controller/pinctrl.h  |   5 +-
 ovn/lib/actions.c | 394 +++---
 ovn/lib/actions.h |  64 +++-
 ovn/lib/expr.c| 140 ++--
 ovn/lib/expr.h|  13 +-
 ovn/lib/lex.c |   9 +-
 ovn/northd/ovn-northd.8.xml   | 112 +++---
 ovn/northd/ovn-northd.c   | 105 --
 ovn/ovn-architecture.7.xml|  78 +++--
 ovn/ovn-sb.ovsschema  |  15 +-
 ovn/ovn-sb.xml| 158 -
 ovn/utilities/ovn-sbctl.c |   4 +
 tests/ofproto-dpif.at |  65 
 tests/ovn.at  | 268 ---
 tests/test-ovn.c  |  33 +-
 utilities/ovs-ofctl.8.in  |  26 ++
 vswitchd/bridge.c |  12 +-
 43 files changed, 2126 insertions(+), 680 deletions(-)
 create mode 100644 ovn/controller/lport.c
 create mode 100644 ovn/controller/lport.h

-- 
2.1.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 01/14] sparse: Define INET_ADDRSTRLEN.

[Ben Pfaff]

POSIX defines this but it was missing from the OVS header file definitions
for "sparse".

Signed-off-by: Ben Pfaff 
---
 include/sparse/netinet/in.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/sparse/netinet/in.h b/include/sparse/netinet/in.h
index 1223553..78e5981 100644
--- a/include/sparse/netinet/in.h
+++ b/include/sparse/netinet/in.h
@@ -116,6 +116,7 @@ struct sockaddr_in6 {
  (X)->s6_addr[10] == 0xff &&\
  (X)->s6_addr[11] == 0xff)
 
+#define INET_ADDRSTRLEN 16
 #define INET6_ADDRSTRLEN 46
 
 #define IPV6_TCLASS   67
-- 
2.1.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 04/14] Use ip_parse() and ipv6_parse() and variants in more places.

[Ben Pfaff]

This saves some code and improves clarity, in my opinion.

Some of these changes just change an inet_pton() call into a similar
ip_parse() or ipv6_parse() call.  In those cases the benefit is better
type safety, since inet_pton()'s output parameter is type "void *".

Signed-off-by: Ben Pfaff 
---
 lib/bfd.c  |  2 +-
 lib/netdev-dummy.c | 14 ++---
 lib/ovs-router.c   | 59 +-
 lib/packets.c  |  2 +-
 lib/socket-util.c  | 10 -
 ovn/lib/lex.c  |  9 +++--
 vswitchd/bridge.c  |  7 +++
 7 files changed, 33 insertions(+), 70 deletions(-)

diff --git a/lib/bfd.c b/lib/bfd.c
index adac666..66c99fb 100644
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -939,7 +939,7 @@ bfd_forwarding__(struct bfd *bfd) OVS_REQUIRES(mutex)
 static bool
 bfd_lookup_ip(const char *host_name, struct in_addr *addr)
 {
-if (!inet_pton(AF_INET, host_name, addr)) {
+if (!ip_parse(host_name, &addr->s_addr)) {
 VLOG_ERR_RL(&rl, "\"%s\" is not a valid IP address", host_name);
 return false;
 }
diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 5cd50a1..df319de 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -1426,18 +1426,16 @@ netdev_dummy_ip4addr(struct unixctl_conn *conn, int 
argc OVS_UNUSED,
 struct netdev *netdev = netdev_from_name(argv[1]);
 
 if (netdev && is_dummy_class(netdev->netdev_class)) {
-struct in_addr ip;
-uint16_t plen;
+struct in_addr ip, mask;
+char *error;
 
-if (ovs_scan(argv[2], IP_SCAN_FMT"/%"SCNi16,
- IP_SCAN_ARGS(&ip.s_addr), &plen)) {
-struct in_addr mask;
-
-mask.s_addr = be32_prefix_mask(plen);
+error = ip_parse_masked(argv[2], &ip.s_addr, &mask.s_addr);
+if (!error) {
 netdev_dummy_set_in4(netdev, ip, mask);
 unixctl_command_reply(conn, "OK");
 } else {
-unixctl_command_reply_error(conn, "Invalid parameters");
+unixctl_command_reply_error(conn, error);
+free(error);
 }
 } else {
 unixctl_command_reply_error(conn, "Unknown Dummy Interface");
diff --git a/lib/ovs-router.c b/lib/ovs-router.c
index 9a59a24..4ad30db 100644
--- a/lib/ovs-router.c
+++ b/lib/ovs-router.c
@@ -199,79 +199,48 @@ rt_entry_delete(uint8_t priority, const struct in6_addr 
*ip6_dst, uint8_t plen)
 static bool
 scan_ipv6_route(const char *s, struct in6_addr *addr, unsigned int *plen)
 {
-struct in6_addr mask;
-char *error;
-
-error = ipv6_parse_masked(s, addr, &mask);
+char *error = ipv6_parse_cidr(s, addr, plen);
 if (error) {
 free(error);
 return false;
 }
-
-if (!ipv6_is_cidr(&mask)) {
-return false;
-}
-
-*plen = ipv6_count_cidr_bits(&mask);
-
 return true;
 }
 
 static bool
 scan_ipv4_route(const char *s, ovs_be32 *addr, unsigned int *plen)
 {
-int len, max_plen, n;
-int slen = strlen(s);
-uint8_t *ip = (uint8_t *)addr;
-
-*addr = htonl(0);
-if (!ovs_scan(s, "%"SCNu8"%n", &ip[0], &n)) {
+char *error = ip_parse_cidr(s, addr, plen);
+if (error) {
+free(error);
 return false;
 }
-len = n;
-max_plen = 8;
-for (int i = 1; i < 4; i++) {
-if (ovs_scan(s + len, ".%"SCNu8"%n", &ip[i], &n)) {
-len += n;
-max_plen += 8;
-} else {
-break;
-}
-}
-if (len == slen && max_plen == 32) {
-*plen = 32;
-return true;
-}
-if (ovs_scan(s + len, "/%u%n", plen, &n)
-&& len + n == slen && *plen <= max_plen) {
-return true;
-}
-return false;
+return true;
 }
 
 static void
 ovs_router_add(struct unixctl_conn *conn, int argc,
   const char *argv[], void *aux OVS_UNUSED)
 {
-ovs_be32 ip, gw;
+ovs_be32 ip;
 unsigned int plen;
 struct in6_addr ip6;
 struct in6_addr gw6;
 
 if (scan_ipv4_route(argv[1], &ip, &plen)) {
-if (argc > 3) {
-inet_pton(AF_INET, argv[3], (struct in_addr *)&gw);
-} else {
-gw = 0;
+ovs_be32 gw = 0;
+if (argc > 3 && !ip_parse(argv[3], &gw)) {
+unixctl_command_reply_error(conn, "Invalid gateway");
+return;
 }
 in6_addr_set_mapped_ipv4(&ip6, ip);
 in6_addr_set_mapped_ipv4(&gw6, gw);
 plen += 96;
 } else if (scan_ipv6_route(argv[1], &ip6, &plen)) {
-if (argc > 3) {
-inet_pton(AF_INET6, argv[3], &gw6);
-} else {
-gw6 = in6addr_any;
+gw6 = in6addr_exact;
+if (argc > 3 && !ipv6_parse(argv[3], &gw6)) {
+unixctl_command_reply_error(conn, "Invalid IPv6 gateway");
+return;
 }
 } else {
 unixctl_command_reply_error(conn, "Invalid parameters");
diff --git a/lib/packets.c b/lib/packets.c
index 309ec0c..d82341d 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ 

[ovs-dev] [PATCH 02/14] packets: New macro ETH_ADDR_STRLEN.

[Ben Pfaff]

An upcoming commit will introduce another user.

Signed-off-by: Ben Pfaff 
---
 lib/packets.h | 1 +
 vswitchd/bridge.c | 5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/packets.h b/lib/packets.h
index edf140b..1e0417a 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -330,6 +330,7 @@ ovs_be32 set_mpls_lse_values(uint8_t ttl, uint8_t tc, 
uint8_t bos,
 #define ETH_ADDR_ARGS(EA) ETH_ADDR_BYTES_ARGS((EA).ea)
 #define ETH_ADDR_BYTES_ARGS(EAB) \
  (EAB)[0], (EAB)[1], (EAB)[2], (EAB)[3], (EAB)[4], (EAB)[5]
+#define ETH_ADDR_STRLEN 17
 
 /* Example:
  *
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index b966d92..af10352 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2220,9 +2220,10 @@ iface_refresh_netdev_status(struct iface *iface)
 
 error = netdev_get_etheraddr(iface->netdev, &mac);
 if (!error) {
-char mac_string[32];
+char mac_string[ETH_ADDR_STRLEN + 1];
 
-sprintf(mac_string, ETH_ADDR_FMT, ETH_ADDR_ARGS(mac));
+snprintf(mac_string, sizeof mac_string,
+ ETH_ADDR_FMT, ETH_ADDR_ARGS(mac));
 ovsrec_interface_set_mac_in_use(iface->cfg, mac_string);
 } else {
 ovsrec_interface_set_mac_in_use(iface->cfg, NULL);
-- 
2.1.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 06/14] actions: Factor out new helper function add_prerequisite().

[Ben Pfaff]

This will acquire new users in upcoming commits.

Signed-off-by: Ben Pfaff 
---
 ovn/lib/actions.c | 24 +++-
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 693b1c1..581dbae 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -183,6 +183,19 @@ parse_next_action(struct action_context *ctx)
 }
 }
 
+/* Parses 'prerequisite' as an expression in the context of 'ctx', then adds it
+ * as a conjunction with the existing 'ctx->prereqs'. */
+static void
+add_prerequisite(struct action_context *ctx, const char *prerequisite)
+{
+struct expr *expr;
+char *error;
+
+expr = expr_parse_string(prerequisite, ctx->symtab, &error);
+ovs_assert(!error);
+ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr);
+}
+
 static void
 emit_ct(struct action_context *ctx, bool recirc_next, bool commit)
 {
@@ -209,12 +222,7 @@ emit_ct(struct action_context *ctx, bool recirc_next, bool 
commit)
 ct->alg = 0;
 
 /* CT only works with IP, so set up a prerequisite. */
-struct expr *expr;
-char *error;
-
-expr = expr_parse_string("ip", ctx->symtab, &error);
-ovs_assert(!error);
-ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr);
+add_prerequisite(ctx, "ip");
 }
 
 static void
@@ -249,9 +257,7 @@ parse_actions(struct action_context *ctx)
 emit_resubmit(ctx, ctx->output_ptable);
 } else if (lexer_match_id(ctx->lexer, "ip.ttl")) {
 if (lexer_match(ctx->lexer, LEX_T_DECREMENT)) {
-struct expr *e = expr_parse_string("ip", ctx->symtab,
-   &ctx->error);
-ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, e);
+add_prerequisite(ctx, "ip");
 ofpact_put_DEC_TTL(ctx->ofpacts);
 } else {
 action_syntax_error(ctx, "expecting `--'");
-- 
2.1.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 07/14] actions: Factor parsing a single action into a new function parse_action().

[Ben Pfaff]

This will have another user in an upcoming commit.

Signed-off-by: Ben Pfaff 
---
 ovn/lib/actions.c | 70 +++
 1 file changed, 39 insertions(+), 31 deletions(-)

diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 581dbae..6bc452a 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -52,6 +52,8 @@ struct action_context {
 struct expr *prereqs;   /* Prerequisites to apply to match. */
 };
 
+static bool parse_action(struct action_context *);
+
 static bool
 action_error_handle_common(struct action_context *ctx)
 {
@@ -225,6 +227,42 @@ emit_ct(struct action_context *ctx, bool recirc_next, bool 
commit)
 add_prerequisite(ctx, "ip");
 }
 
+static bool
+parse_action(struct action_context *ctx)
+{
+if (ctx->lexer->token.type != LEX_T_ID) {
+action_syntax_error(ctx, NULL);
+return false;
+}
+
+enum lex_type lookahead = lexer_lookahead(ctx->lexer);
+if (lookahead == LEX_T_EQUALS || lookahead == LEX_T_EXCHANGE
+|| lookahead == LEX_T_LSQUARE) {
+parse_set_action(ctx);
+} else if (lexer_match_id(ctx->lexer, "next")) {
+parse_next_action(ctx);
+} else if (lexer_match_id(ctx->lexer, "output")) {
+emit_resubmit(ctx, ctx->output_ptable);
+} else if (lexer_match_id(ctx->lexer, "ip.ttl")) {
+if (lexer_match(ctx->lexer, LEX_T_DECREMENT)) {
+add_prerequisite(ctx, "ip");
+ofpact_put_DEC_TTL(ctx->ofpacts);
+} else {
+action_syntax_error(ctx, "expecting `--'");
+}
+} else if (lexer_match_id(ctx->lexer, "ct_next")) {
+emit_ct(ctx, true, false);
+} else if (lexer_match_id(ctx->lexer, "ct_commit")) {
+emit_ct(ctx, false, true);
+} else {
+action_syntax_error(ctx, "expecting action");
+}
+if (!lexer_match(ctx->lexer, LEX_T_SEMICOLON)) {
+action_syntax_error(ctx, "expecting ';'");
+}
+return !ctx->error;
+}
+
 static void
 parse_actions(struct action_context *ctx)
 {
@@ -242,37 +280,7 @@ parse_actions(struct action_context *ctx)
 }
 
 while (ctx->lexer->token.type != LEX_T_END) {
-if (ctx->lexer->token.type != LEX_T_ID) {
-action_syntax_error(ctx, NULL);
-break;
-}
-
-enum lex_type lookahead = lexer_lookahead(ctx->lexer);
-if (lookahead == LEX_T_EQUALS || lookahead == LEX_T_EXCHANGE
-|| lookahead == LEX_T_LSQUARE) {
-parse_set_action(ctx);
-} else if (lexer_match_id(ctx->lexer, "next")) {
-parse_next_action(ctx);
-} else if (lexer_match_id(ctx->lexer, "output")) {
-emit_resubmit(ctx, ctx->output_ptable);
-} else if (lexer_match_id(ctx->lexer, "ip.ttl")) {
-if (lexer_match(ctx->lexer, LEX_T_DECREMENT)) {
-add_prerequisite(ctx, "ip");
-ofpact_put_DEC_TTL(ctx->ofpacts);
-} else {
-action_syntax_error(ctx, "expecting `--'");
-}
-} else if (lexer_match_id(ctx->lexer, "ct_next")) {
-emit_ct(ctx, true, false);
-} else if (lexer_match_id(ctx->lexer, "ct_commit")) {
-emit_ct(ctx, false, true);
-} else {
-action_syntax_error(ctx, "expecting action");
-}
-if (!lexer_match(ctx->lexer, LEX_T_SEMICOLON)) {
-action_syntax_error(ctx, "expecting ';'");
-}
-if (ctx->error) {
+if (!parse_action(ctx)) {
 return;
 }
 }
-- 
2.1.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 05/14] Add OpenFlow support for new "arp" action.

[Ben Pfaff]

This is useful for generating an ARP request from the OpenFlow flow table,
which in turn is useful for implementing a router.  An upcoming commit
will use this feature to implement ARP request generationg in the OVN L3
logical router.

Signed-off-by: Ben Pfaff 
---
 NEWS  |   2 +
 datapath/linux/compat/include/linux/openvswitch.h |   5 +-
 lib/dpif-netdev.c |   3 +-
 lib/dpif.c|   1 +
 lib/odp-execute.c |  26 +
 lib/odp-util.c|  12 ++
 lib/ofp-actions.c | 129 +-
 lib/ofp-actions.h |   3 +-
 lib/ofp-errors.h  |   4 +
 lib/packets.c |  36 +++---
 lib/packets.h |   1 +
 ofproto/ofproto-dpif-sflow.c  |   4 +
 ofproto/ofproto-dpif-xlate.c  |  70 
 tests/ofproto-dpif.at |  65 +++
 utilities/ovs-ofctl.8.in  |  26 +
 15 files changed, 370 insertions(+), 17 deletions(-)

diff --git a/NEWS b/NEWS
index a6b1599..578171d 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,7 @@
 Post-v2.5.0
 -
+   - OpenFlow:
+ * New "arp" OpenFlow extension action for generating ARP packets.
 
 
 v2.5.0 - xx xxx 
diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 3b39ebb..5acf539 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2007-2014 Nicira, Inc.
+ * Copyright (c) 2007-2015 Nicira, Inc.
  *
  * This file is offered under your choice of two licenses: Apache 2.0 or GNU
  * GPL 2.0 or later.  The permission statements for each of these licenses is
@@ -783,6 +783,8 @@ enum ovs_nat_attr {
  * ovs_action_push_tnl.
  * @OVS_ACTION_ATTR_TUNNEL_POP: Lookup tunnel port by port-no passed and pop
  * tunnel header.
+ * @OVS_ACTION_ATTR_ARP: Transform IPv4 packet into ARP packet, execute nested
+ * actions, restore IPv4 packet.
  */
 
 enum ovs_action_attr {
@@ -806,6 +808,7 @@ enum ovs_action_attr {
 #ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
+   OVS_ACTION_ATTR_ARP,   /* Nested OVS_ACTION_ATTR_*. */
 #endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
   * from userspace. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index a67ef05..d382fde 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -3627,6 +3627,7 @@ dp_execute_cb(void *aux_, struct dp_packet **packets, int 
cnt,
 case OVS_ACTION_ATTR_SET_MASKED:
 case OVS_ACTION_ATTR_SAMPLE:
 case OVS_ACTION_ATTR_HASH:
+case OVS_ACTION_ATTR_ARP:
 case OVS_ACTION_ATTR_UNSPEC:
 case __OVS_ACTION_ATTR_MAX:
 OVS_NOT_REACHED();
diff --git a/lib/dpif.c b/lib/dpif.c
index 38e40ba..2926668 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1146,6 +1146,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet 
**packets, int cnt,
 case OVS_ACTION_ATTR_SET:
 case OVS_ACTION_ATTR_SET_MASKED:
 case OVS_ACTION_ATTR_SAMPLE:
+case OVS_ACTION_ATTR_ARP:
 case OVS_ACTION_ATTR_UNSPEC:
 case __OVS_ACTION_ATTR_MAX:
 OVS_NOT_REACHED();
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index b5204b2..5e5c3ca 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -34,6 +34,8 @@
 #include "unaligned.h"
 #include "util.h"
 
+#include "openvswitch/vlog.h"
+
 /* Masked copy of an ethernet address. 'src' is already properly masked. */
 static void
 ether_addr_copy_masked(struct eth_addr *dst, const struct eth_addr src,
@@ -480,6 +482,23 @@ odp_execute_sample(void *dp, struct dp_packet *packet, 
bool steal,
 nl_attr_get_size(subactions), dp_execute_action);
 }
 
+static void
+odp_execute_arp(void *dp, struct dp_packet *ip,
+const struct nlattr *action, odp_execute_cb dp_execute_action)
+{
+uint64_t stub[DIV_ROUND_UP(2 + ETH_HEADER_LEN + VLAN_HEADER_LEN +
+   ARP_ETH_HEADER_LEN, 8)];
+struct dp_packet arp;
+dp_packet_use_stub(&arp, stub, sizeof stub);
+compose_arp__(&arp);
+arp.md = ip->md;
+
+struct dp_packet *arpp = &arp;
+odp_execute_actions(dp, &arpp, 1, false, nl_attr_get(action),
+  

[ovs-dev] [PATCH 08/14] actions: Implement OVN "arp" action.

[Ben Pfaff]

An upcoming commit will use this as a building block in adding ARP support
to the OVN L3 logical router implementation.

Signed-off-by: Ben Pfaff 
---
 ovn/lib/actions.c | 33 +
 ovn/ovn-sb.xml| 21 +
 tests/ovn.at  |  3 +++
 3 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 6bc452a..4505b6b 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -199,6 +199,37 @@ add_prerequisite(struct action_context *ctx, const char 
*prerequisite)
 }
 
 static void
+parse_arp_action(struct action_context *ctx)
+{
+if (!lexer_match(ctx->lexer, LEX_T_LCURLY)) {
+action_syntax_error(ctx, "expecting `{'");
+return;
+}
+
+size_t arp_offset = ctx->ofpacts->size;
+ofpact_put_ARP(ctx->ofpacts);
+
+/* XXX What is the right thing to do about prerequisites? */
+struct expr *save_prereqs = ctx->prereqs;
+ctx->prereqs = NULL;
+
+while (!lexer_match(ctx->lexer, LEX_T_RCURLY)) {
+if (!parse_action(ctx)) {
+break;
+}
+}
+
+struct ofpact_nest *arp = ofpbuf_at(ctx->ofpacts, arp_offset, sizeof *arp);
+ctx->ofpacts->header = arp;
+ofpact_update_len(ctx->ofpacts, &arp->ofpact);
+
+/* Restore prerequisites. */
+expr_destroy(ctx->prereqs);
+ctx->prereqs = save_prereqs;
+add_prerequisite(ctx, "ip4");
+}
+
+static void
 emit_ct(struct action_context *ctx, bool recirc_next, bool commit)
 {
 struct ofpact_conntrack *ct = ofpact_put_CT(ctx->ofpacts);
@@ -254,6 +285,8 @@ parse_action(struct action_context *ctx)
 emit_ct(ctx, true, false);
 } else if (lexer_match_id(ctx->lexer, "ct_commit")) {
 emit_ct(ctx, false, true);
+} else if (lexer_match_id(ctx->lexer, "arp")) {
+parse_arp_action(ctx);
 } else {
 action_syntax_error(ctx, "expecting action");
 }
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index e674f3a..8ce2e74 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -907,14 +907,6 @@
   Commit the flow to the connection tracking entry associated
   with it by a previous call to ct_next.
 
-  
-
-  
-The following actions will likely be useful later, but they have not
-been thought out carefully.
-  
-
-  
 
 arp { action; ... };
 
@@ -942,9 +934,22 @@
 arp.tpa copied from ip4.dst
   
 
+  
+The ARP packet has the same VLAN header, if any, as the IP packet
+it replaces.
+  
+
   Prerequisite: ip4
 
 
+  
+
+  
+The following actions will likely be useful later, but they have not
+been thought out carefully.
+  
+
+  
 icmp4 { action; ... };
 
   
diff --git a/tests/ovn.at b/tests/ovn.at
index e00674d..6ce9339 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -462,6 +462,9 @@ outport <-> inport; => 
actions=push:NXM_NX_REG6[],push:NXM_NX_REG7[],pop:NXM_NX_
 ip.ttl--; => actions=dec_ttl, prereqs=ip
 ip.ttl = 4; => actions=set_field:4->nw_ttl, prereqs=eth.type == 0x800 || 
eth.type == 0x86dd
 
+# arp
+arp { eth.dst = ff:ff:ff:ff:ff:ff; output; }; => 
actions=arp(set_field:ff:ff:ff:ff:ff:ff->eth_dst,resubmit(,64)), prereqs=ip4
+
 # Contradictionary prerequisites (allowed but not useful):
 ip4.src = ip6.src[0..31]; => 
actions=move:NXM_NX_IPV6_SRC[0..31]->NXM_OF_IP_SRC[], prereqs=eth.type == 0x800 
&& eth.type == 0x86dd
 ip4.src <-> ip6.src[0..31]; => 
actions=push:NXM_NX_IPV6_SRC[0..31],push:NXM_OF_IP_SRC[],pop:NXM_NX_IPV6_SRC[0..31],pop:NXM_OF_IP_SRC[],
 prereqs=eth.type == 0x800 && eth.type == 0x86dd
-- 
2.1.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 09/14] actions: Bundle action parsing parameters into a structure.

[Ben Pfaff]

This will make it easier to add and change parameters, as done in an
upcoming commit.

Signed-off-by: Ben Pfaff 
---
 ovn/controller/lflow.c |  15 +--
 ovn/lib/actions.c  | 109 -
 ovn/lib/actions.h  |  51 ++-
 tests/test-ovn.c   |  15 +--
 4 files changed, 89 insertions(+), 101 deletions(-)

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 38c72c1..2f91bca 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -297,10 +297,17 @@ lflow_run(struct controller_ctx *ctx, struct hmap 
*flow_table,
 char *error;
 
 ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
-error = actions_parse_string(lflow->actions, &symtab, &ldp->ports,
- ct_zones, first_ptable, LOG_PIPELINE_LEN,
- lflow->table_id, output_ptable,
- &ofpacts, &prereqs);
+struct action_params ap = {
+.symtab = &symtab,
+.ports = &ldp->ports,
+.ct_zones = ct_zones,
+
+.n_tables = LOG_PIPELINE_LEN,
+.first_ptable = first_ptable,
+.cur_ltable = lflow->table_id,
+.output_ptable = output_ptable,
+};
+error = actions_parse_string(lflow->actions, &ap, &ofpacts, &prereqs);
 if (error) {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
 VLOG_WARN_RL(&rl, "error parsing actions \"%s\": %s",
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 4505b6b..be303f9 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -29,25 +29,9 @@
 
 /* Context maintained during actions_parse(). */
 struct action_context {
-/* Input. */
-
+const struct action_params *ap; /* Parameters. */
 struct lexer *lexer;/* Lexer for pulling more tokens. */
-const struct simap *ports;  /* Map from port name to number. */
-const struct shash *symtab; /* Symbol table. */
-
-/* OVN maps each logical flow table (ltable), one-to-one, onto a physical
- * OpenFlow flow table (ptable).  These members describe the mapping and
- * data related to flow tables. */
-uint8_t n_tables;   /* Number of flow tables. */
-uint8_t first_ptable;   /* First OpenFlow table. */
-uint8_t cur_ltable; /* 0 <= cur_ltable < n_tables. */
-uint8_t output_ptable;  /* OpenFlow table for 'output' to resubmit. */
-const struct simap *ct_zones; /* Map from port name to conntrack zone. */
-
-/* State. */
 char *error;/* Error, if any, otherwise NULL. */
-
-/* Output. */
 struct ofpbuf *ofpacts; /* Actions. */
 struct expr *prereqs;   /* Prerequisites to apply to match. */
 };
@@ -124,7 +108,7 @@ parse_set_action(struct action_context *ctx)
 struct expr *prereqs;
 char *error;
 
-error = expr_parse_assignment(ctx->lexer, ctx->symtab, ctx->ports,
+error = expr_parse_assignment(ctx->lexer, ctx->ap->symtab, ctx->ap->ports,
   ctx->ofpacts, &prereqs);
 if (error) {
 action_error(ctx, "%s", error);
@@ -156,7 +140,7 @@ action_get_int(struct action_context *ctx, int *value)
 static void
 parse_next_action(struct action_context *ctx)
 {
-if (!ctx->n_tables) {
+if (!ctx->ap->n_tables) {
 action_error(ctx, "\"next\" action not allowed here.");
 } else if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
 int ltable;
@@ -169,16 +153,17 @@ parse_next_action(struct action_context *ctx)
 return;
 }
 
-if (ltable >= ctx->n_tables) {
+if (ltable >= ctx->ap->n_tables) {
 action_error(ctx, "\"next\" argument must be in range 0 to %d.",
- ctx->n_tables - 1);
+ ctx->ap->n_tables - 1);
 return;
 }
 
-emit_resubmit(ctx, ctx->first_ptable + ltable);
+emit_resubmit(ctx, ctx->ap->first_ptable + ltable);
 } else {
-if (ctx->cur_ltable < ctx->n_tables) {
-emit_resubmit(ctx, ctx->first_ptable + ctx->cur_ltable + 1);
+if (ctx->ap->cur_ltable < ctx->ap->n_tables) {
+emit_resubmit(ctx,
+  ctx->ap->first_ptable + ctx->ap->cur_ltable + 1);
 } else {
 action_error(ctx, "\"next\" action not allowed in last table.");
 }
@@ -193,7 +178,7 @@ add_prerequisite(struct action_context *ctx, const char 
*prerequisite)
 struct expr *expr;
 char *error;
 
-expr = expr_parse_string(prerequisite, ctx->symtab, &error);
+expr = expr_parse_string(prerequisite, ctx->ap->symtab, &error);
 ovs_assert(!error);
 ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr);
 }
@@ -237,8 +222,8 @@ emit_ct(struct action_context *ctx, bool recirc_next, bool 
commit)
 
 /* If "recirc" is set, we automatically go to the next table. */

[ovs-dev] [PATCH 11/14] ovn-controller: Add data structure for indexing lports, multicast groups.

[Ben Pfaff]

This was more or less implemented inside lflow.c until now, but some
upcoming code that shouldn't be in that file needs to use it too.

This also adds a second index on lports, so that lports can be looked up
based on the logical datapath tunnel key and the logical port tunnel key.
An upcoming commit will add a user for this new index.

Signed-off-by: Ben Pfaff 
---
 ovn/controller/automake.mk  |   2 +
 ovn/controller/lflow.c  | 165 +---
 ovn/controller/lflow.h  |   7 +-
 ovn/controller/lport.c  | 157 ++
 ovn/controller/lport.h  |  69 +
 ovn/controller/ovn-controller.c |  33 
 6 files changed, 288 insertions(+), 145 deletions(-)
 create mode 100644 ovn/controller/lport.c
 create mode 100644 ovn/controller/lport.h

diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
index cadfa9c..cf57bbd 100644
--- a/ovn/controller/automake.mk
+++ b/ovn/controller/automake.mk
@@ -8,6 +8,8 @@ ovn_controller_ovn_controller_SOURCES = \
ovn/controller/encaps.h \
ovn/controller/lflow.c \
ovn/controller/lflow.h \
+   ovn/controller/lport.c \
+   ovn/controller/lport.h \
ovn/controller/ofctrl.c \
ovn/controller/ofctrl.h \
ovn/controller/pinctrl.c \
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index dcabe2f..2bfc9e1 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -15,12 +15,13 @@
 
 #include 
 #include "lflow.h"
+#include "lport.h"
 #include "dynamic-string.h"
 #include "ofctrl.h"
 #include "ofp-actions.h"
 #include "ofpbuf.h"
 #include "openvswitch/vlog.h"
-#include "ovn/controller/ovn-controller.h"
+#include "ovn-controller.h"
 #include "ovn/lib/actions.h"
 #include "ovn/lib/expr.h"
 #include "ovn/lib/ovn-sb-idl.h"
@@ -42,8 +43,8 @@ add_logical_register(struct shash *symtab, enum mf_field_id 
id)
 expr_symtab_add_field(symtab, name, id, NULL, false);
 }
 
-static void
-symtab_init(void)
+void
+lflow_init(void)
 {
 shash_init(&symtab);
 
@@ -147,149 +148,48 @@ symtab_init(void)
 expr_symtab_add_field(&symtab, "sctp.dst", MFF_SCTP_DST, "sctp", false);
 }
 
-/* Logical datapaths and logical port numbers. */
-
-/* A logical datapath.
- *
- * 'ports' maps 'logical_port' names to 'tunnel_key' values in the OVN_SB
- * Port_Binding table within the logical datapath. */
-struct logical_datapath {
-struct hmap_node hmap_node; /* Indexed on 'uuid'. */
-struct uuid uuid;   /* UUID from Datapath_Binding row. */
-uint32_t tunnel_key;/* 'tunnel_key' from Datapath_Binding row. */
-struct simap ports; /* Logical port name to port number. */
+struct lookup_port_aux {
+const struct lport_index *lports;
+const struct mcgroup_index *mcgroups;
+const struct sbrec_datapath_binding *dp;
 };
 
-/* Contains "struct logical_datapath"s. */
-static struct hmap logical_datapaths = HMAP_INITIALIZER(&logical_datapaths);
-
-/* Finds and returns the logical_datapath for 'binding', or NULL if no such
- * logical_datapath exists. */
-static struct logical_datapath *
-ldp_lookup(const struct sbrec_datapath_binding *binding)
-{
-struct logical_datapath *ldp;
-HMAP_FOR_EACH_IN_BUCKET (ldp, hmap_node, uuid_hash(&binding->header_.uuid),
- &logical_datapaths) {
-if (uuid_equals(&ldp->uuid, &binding->header_.uuid)) {
-return ldp;
-}
-}
-return NULL;
-}
-
-/* Creates a new logical_datapath for the given 'binding'. */
-static struct logical_datapath *
-ldp_create(const struct sbrec_datapath_binding *binding)
-{
-struct logical_datapath *ldp;
-
-ldp = xmalloc(sizeof *ldp);
-hmap_insert(&logical_datapaths, &ldp->hmap_node,
-uuid_hash(&binding->header_.uuid));
-ldp->uuid = binding->header_.uuid;
-ldp->tunnel_key = binding->tunnel_key;
-simap_init(&ldp->ports);
-return ldp;
-}
-
-static struct logical_datapath *
-ldp_lookup_or_create(const struct sbrec_datapath_binding *binding)
-{
-struct logical_datapath *ldp = ldp_lookup(binding);
-return ldp ? ldp : ldp_create(binding);
-}
-
-static void
-ldp_free(struct logical_datapath *ldp)
-{
-simap_destroy(&ldp->ports);
-hmap_remove(&logical_datapaths, &ldp->hmap_node);
-free(ldp);
-}
-
-/* Iterates through all of the records in the Port_Binding table, updating the
- * table of logical_datapaths to match the values found in active
- * Port_Bindings. */
-static void
-ldp_run(struct controller_ctx *ctx)
+static bool
+lookup_port_cb(const void *aux_, const char *port_name, unsigned int *portp)
 {
-struct logical_datapath *ldp;
-HMAP_FOR_EACH (ldp, hmap_node, &logical_datapaths) {
-simap_clear(&ldp->ports);
-}
-
-const struct sbrec_port_binding *binding;
-SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
-struct logical_datapath *ldp = ldp_lookup_or_create(binding

[ovs-dev] [PATCH 13/14] ovn: Reorganize action parsing test.

[Ben Pfaff]

It seems easier to understand if all of the tests for a given action
are grouped together.

Signed-off-by: Ben Pfaff 
---
 tests/ovn.at | 89 ++--
 1 file changed, 50 insertions(+), 39 deletions(-)

diff --git a/tests/ovn.at b/tests/ovn.at
index 522c4e4..de56c9f 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -437,55 +437,31 @@ AT_CLEANUP
 AT_SETUP([ovn -- action parsing])
 dnl Text before => is input, text after => is expected output.
 AT_DATA([test-cases.txt], [[
-# Positive tests.
+# drop
 drop; => actions=drop, prereqs=1
+drop; next; => Syntax error at `next' expecting end of input.
+next; drop; => Syntax error at `drop' expecting action.
+
+# output
+output; => actions=resubmit(,64), prereqs=1
+
+# next
 next; => actions=resubmit(,27), prereqs=1
 next(0); => actions=resubmit(,16), prereqs=1
 next(15); => actions=resubmit(,31), prereqs=1
-ct_next; => actions=ct(table=27,zone=NXM_NX_REG5[0..15]), prereqs=ip
-ct_commit; => actions=ct(commit,zone=NXM_NX_REG5[0..15]), prereqs=ip
-output; => actions=resubmit(,64), prereqs=1
-outport="eth0"; next; outport="LOCAL"; next; => 
actions=set_field:0x5->reg7,resubmit(,27),set_field:0xfffe->reg7,resubmit(,27), 
prereqs=1
+
+next(); => Syntax error at `)' expecting small integer.
+next(10; => Syntax error at `;' expecting `)'.
+next(16); => "next" argument must be in range 0 to 15.
+
+# Loading a constant value.
 tcp.dst=80; => actions=set_field:80->tcp_dst, prereqs=ip.proto == 0x6 && 
(eth.type == 0x800 || eth.type == 0x86dd)
 eth.dst[40] = 1; => 
actions=set_field:01:00:00:00:00:00/01:00:00:00:00:00->eth_dst, prereqs=1
 vlan.pcp = 2; => actions=set_field:0x4000/0xe000->vlan_tci, 
prereqs=vlan.tci[12]
 vlan.tci[13..15] = 2; => actions=set_field:0x4000/0xe000->vlan_tci, prereqs=1
-reg0 = reg1; => actions=move:OXM_OF_PKT_REG0[0..31]->OXM_OF_PKT_REG0[32..63], 
prereqs=1
-vlan.pcp = reg0[0..2]; => 
actions=move:OXM_OF_PKT_REG0[32..34]->NXM_OF_VLAN_TCI[13..15], 
prereqs=vlan.tci[12]
-reg0[10] = vlan.pcp[1]; => 
actions=move:NXM_OF_VLAN_TCI[14]->OXM_OF_PKT_REG0[42], prereqs=vlan.tci[12]
-outport = inport; => actions=move:NXM_NX_REG6[]->NXM_NX_REG7[], prereqs=1
 inport = ""; => actions=set_field:0->reg6,set_field:0->in_port, prereqs=1
-reg0 <-> reg1; => 
actions=push:OXM_OF_PKT_REG0[0..31],push:OXM_OF_PKT_REG0[32..63],pop:OXM_OF_PKT_REG0[0..31],pop:OXM_OF_PKT_REG0[32..63],
 prereqs=1
-vlan.pcp <-> reg0[0..2]; => 
actions=push:OXM_OF_PKT_REG0[32..34],push:NXM_OF_VLAN_TCI[13..15],pop:OXM_OF_PKT_REG0[32..34],pop:NXM_OF_VLAN_TCI[13..15],
 prereqs=vlan.tci[12]
-reg0[10] <-> vlan.pcp[1]; => 
actions=push:NXM_OF_VLAN_TCI[14],push:OXM_OF_PKT_REG0[42],pop:NXM_OF_VLAN_TCI[14],pop:OXM_OF_PKT_REG0[42],
 prereqs=vlan.tci[12]
-outport <-> inport; => 
actions=push:NXM_NX_REG6[],push:NXM_NX_REG7[],pop:NXM_NX_REG6[],pop:NXM_NX_REG7[],
 prereqs=1
-ip.ttl--; => actions=dec_ttl, prereqs=ip
 ip.ttl = 4; => actions=set_field:4->nw_ttl, prereqs=eth.type == 0x800 || 
eth.type == 0x86dd
-
-# arp
-arp { eth.dst = ff:ff:ff:ff:ff:ff; output; }; => 
actions=arp(set_field:ff:ff:ff:ff:ff:ff->eth_dst,resubmit(,64)), prereqs=ip4
-
-# Contradictionary prerequisites (allowed but not useful):
-ip4.src = ip6.src[0..31]; => 
actions=move:NXM_NX_IPV6_SRC[0..31]->NXM_OF_IP_SRC[], prereqs=eth.type == 0x800 
&& eth.type == 0x86dd
-ip4.src <-> ip6.src[0..31]; => 
actions=push:NXM_NX_IPV6_SRC[0..31],push:NXM_OF_IP_SRC[],pop:NXM_NX_IPV6_SRC[0..31],pop:NXM_OF_IP_SRC[],
 prereqs=eth.type == 0x800 && eth.type == 0x86dd
-
-## Negative tests.
-
-; => Syntax error at `;'.
-xyzzy; => Syntax error at `xyzzy' expecting action.
-next; 123; => Syntax error at `123'.
-next; xyzzy; => Syntax error at `xyzzy' expecting action.
-
-# "drop;" must be on its own:
-drop; next; => Syntax error at `next' expecting end of input.
-next; drop; => Syntax error at `drop' expecting action.
-
-# Missing ";":
-next => Syntax error at end of input expecting ';'.
-
-next(); => Syntax error at `)' expecting small integer.
-next(10; => Syntax error at `;' expecting `)'.
-next(16); => "next" argument must be in range 0 to 15.
+outport="eth0"; next; outport="LOCAL"; next; => 
actions=set_field:0x5->reg7,resubmit(,27),set_field:0xfffe->reg7,resubmit(,27), 
prereqs=1
 
 inport[1] = 1; => Cannot select subfield of string field inport.
 ip.proto[1] = 1; => Cannot select subfield of nominal field ip.proto.
@@ -497,18 +473,53 @@ inport = {}; => Syntax error at `}' expecting constant.
 bad_prereq = 123; => Error parsing expression `xyzzy' encountered as 
prerequisite or predicate of initial expression: Syntax error at `xyzzy' 
expecting field name.
 self_recurse = 123; => Error parsing expression `self_recurse != 0' 
encountered as prerequisite or predicate of initial expression: Error parsing 
expression `self_recurse != 0' encountered as prerequisite or predicate of 
initial expression: Recursive expansion of symbol `self_recurse'.
 vlan.present = 0; => Predicate symbol vlan.present used where lvalue r

[ovs-dev] [PATCH 10/14] ovn: Use callback function instead of simap for logical port number map.

[Ben Pfaff]

An simap is convenient but it isn't very flexible.  If the client wants to
keep extra data with each node then it has to build a second parallel data
structure.  A callback function is kind of a pain for the clients from the
point of view of having to write it and deal with auxiliary data, etc., but
it allows the storage to be more flexible.

An upcoming commit will make further use of this capability.

Signed-off-by: Ben Pfaff 
---
 ovn/controller/lflow.c | 18 ++--
 ovn/lib/actions.c  |  3 +-
 ovn/lib/actions.h  | 10 ---
 ovn/lib/expr.c | 74 +++---
 ovn/lib/expr.h | 10 +--
 tests/test-ovn.c   | 19 +++--
 6 files changed, 94 insertions(+), 40 deletions(-)

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 2f91bca..dcabe2f 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -255,6 +255,18 @@ lflow_init(void)
 symtab_init();
 }
 
+static bool
+lookup_port_cb(const void *ldp_, const char *port_name, unsigned int *portp)
+{
+const struct logical_datapath *ldp = ldp_;
+const struct simap_node *node = simap_find(&ldp->ports, port_name);
+if (!node) {
+return false;
+}
+*portp = node->data;
+return true;
+}
+
 /* Translates logical flows in the Logical_Flow table in the OVN_SB database
  * into OpenFlow flows.  See ovn-architecture(7) for more information. */
 void
@@ -299,7 +311,8 @@ lflow_run(struct controller_ctx *ctx, struct hmap 
*flow_table,
 ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
 struct action_params ap = {
 .symtab = &symtab,
-.ports = &ldp->ports,
+.lookup_port = lookup_port_cb,
+.aux = ldp,
 .ct_zones = ct_zones,
 
 .n_tables = LOG_PIPELINE_LEN,
@@ -340,7 +353,8 @@ lflow_run(struct controller_ctx *ctx, struct hmap 
*flow_table,
 
 expr = expr_simplify(expr);
 expr = expr_normalize(expr);
-uint32_t n_conjs = expr_to_matches(expr, &ldp->ports, &matches);
+uint32_t n_conjs = expr_to_matches(expr, lookup_port_cb, ldp,
+   &matches);
 expr_destroy(expr);
 
 /* Prepare the OpenFlow matches for adding to the flow table. */
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index be303f9..e7dea8d 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -108,7 +108,8 @@ parse_set_action(struct action_context *ctx)
 struct expr *prereqs;
 char *error;
 
-error = expr_parse_assignment(ctx->lexer, ctx->ap->symtab, ctx->ap->ports,
+error = expr_parse_assignment(ctx->lexer, ctx->ap->symtab,
+  ctx->ap->lookup_port, ctx->ap->aux,
   ctx->ofpacts, &prereqs);
 if (error) {
 action_error(ctx, "%s", error);
diff --git a/ovn/lib/actions.h b/ovn/lib/actions.h
index 2c3644a..6ca15c4 100644
--- a/ovn/lib/actions.h
+++ b/ovn/lib/actions.h
@@ -17,6 +17,7 @@
 #ifndef OVN_ACTIONS_H
 #define OVN_ACTIONS_H 1
 
+#include 
 #include 
 #include "compiler.h"
 
@@ -31,10 +32,11 @@ struct action_params {
  * expr_parse()). */
 const struct shash *symtab;
 
- /* 'ports' must be a map from strings (presumably names of ports) to
-  * integers (as one would provide to expr_to_matches()).  Strings used in
-  * the actions that are not in 'ports' are translated to zero. */
-const struct simap *ports;
+/* Looks up logical port 'name'.  If found, stores its port number in
+ * '*portp' and returns true; otherwise, returns false. */
+bool (*lookup_port)(const void *aux, const char *name,
+unsigned int *portp);
+const void *aux;
 
 /* A map from a port name to its connection tracking zone. */
 const struct simap *ct_zones;
diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index f30500e..0d7ba32 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -2286,17 +2286,18 @@ expr_match_add(struct hmap *matches, struct expr_match 
*match)
 }
 
 static bool
-constrain_match(const struct expr *expr, const struct simap *ports,
-struct match *m)
+constrain_match(const struct expr *expr,
+bool (*lookup_port)(const void *aux, const char *name,
+unsigned int *portp),
+const void *aux, struct match *m)
 {
 ovs_assert(expr->type == EXPR_T_CMP);
 if (expr->cmp.symbol->width) {
 mf_mask_subfield(expr->cmp.symbol->field, &expr->cmp.value,
  &expr->cmp.mask, m);
 } else {
-const struct simap_node *node;
-node = ports ? simap_find(ports, expr->cmp.string) : NULL;
-if (!node) {
+unsigned int port;
+if (!lookup_port(aux, expr->cmp.string, &port)) {
 return false;
 }
 
@@ -2307,7 +2308,7 @@ constrain_match(const struct expr *expr, const struct 
simap *ports,
 
 union 

[ovs-dev] [PATCH 12/14] expr: Generalize wording of error message in expand_symbol().

[Ben Pfaff]

The existing wording was very specific to the actual operation being
performed.  While this is nice for users, it becomes difficult to maintain
as more and more operations are added.  This commit makes the wording less
specific, because a third operation will start using this function in an
upcoming commit.

Signed-off-by: Ben Pfaff 
---
 ovn/lib/expr.c | 13 +
 tests/ovn.at   |  8 
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 0d7ba32..54e3085 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -2635,20 +2635,17 @@ expr_is_normalized(const struct expr *expr)
  *
  * If 'rw', verifies that 'f' is a read/write field.
  *
- * 'exchange' should be true if an exchange action is being parsed.  This is
- * only used to improve error message phrasing.
- *
  * Returns true if successful, false if an error was encountered (in which case
  * 'ctx->error' reports the particular error). */
 static bool
-expand_symbol(struct expr_context *ctx, bool rw, bool exchange,
+expand_symbol(struct expr_context *ctx, bool rw,
   struct expr_field *f, struct expr **prereqsp)
 {
 const struct expr_symbol *orig_symbol = f->symbol;
 
 if (f->symbol->expansion && f->symbol->level != EXPR_L_ORDINAL) {
-expr_error(ctx, "Predicate symbol %s cannot be used in %s.",
-   f->symbol->name, exchange ? "exchange" : "assignment");
+expr_error(ctx, "Predicate symbol %s used where lvalue required.",
+   f->symbol->name);
 return false;
 }
 
@@ -2727,7 +2724,7 @@ parse_assignment(struct expr_context *ctx,
 goto exit;
 }
 const struct expr_symbol *orig_dst = dst.symbol;
-if (!expand_symbol(ctx, true, exchange, &dst, &prereqs)) {
+if (!expand_symbol(ctx, true, &dst, &prereqs)) {
 goto exit;
 }
 
@@ -2737,7 +2734,7 @@ parse_assignment(struct expr_context *ctx,
 goto exit;
 }
 const struct expr_symbol *orig_src = src.symbol;
-if (!expand_symbol(ctx, exchange, exchange, &src, &prereqs)) {
+if (!expand_symbol(ctx, exchange, &src, &prereqs)) {
 goto exit;
 }
 
diff --git a/tests/ovn.at b/tests/ovn.at
index 6ce9339..522c4e4 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -490,19 +490,19 @@ next(16); => "next" argument must be in range 0 to 15.
 inport[1] = 1; => Cannot select subfield of string field inport.
 ip.proto[1] = 1; => Cannot select subfield of nominal field ip.proto.
 eth.dst[40] == 1; => Syntax error at `==' expecting `='.
-ip = 1; => Predicate symbol ip cannot be used in assignment.
+ip = 1; => Predicate symbol ip used where lvalue required.
 ip.proto = 6; => Field ip.proto is not modifiable.
 inport = {"a", "b"}; => Assignments require a single value.
 inport = {}; => Syntax error at `}' expecting constant.
 bad_prereq = 123; => Error parsing expression `xyzzy' encountered as 
prerequisite or predicate of initial expression: Syntax error at `xyzzy' 
expecting field name.
 self_recurse = 123; => Error parsing expression `self_recurse != 0' 
encountered as prerequisite or predicate of initial expression: Error parsing 
expression `self_recurse != 0' encountered as prerequisite or predicate of 
initial expression: Recursive expansion of symbol `self_recurse'.
-vlan.present = 0; => Predicate symbol vlan.present cannot be used in 
assignment.
-reg0[0] = vlan.present; => Predicate symbol vlan.present cannot be used in 
assignment.
+vlan.present = 0; => Predicate symbol vlan.present used where lvalue required.
+reg0[0] = vlan.present; => Predicate symbol vlan.present used where lvalue 
required.
 reg0 = reg1[0..10]; => Can't assign 11-bit value to 32-bit destination.
 inport = reg0; => Can't assign integer field (reg0) to string field (inport).
 inport = big_string; => String fields inport and big_string are incompatible 
for assignment.
 ip.proto = reg0[0..7]; => Field ip.proto is not modifiable.
-reg0[0] <-> vlan.present; => Predicate symbol vlan.present cannot be used in 
exchange.
+reg0[0] <-> vlan.present; => Predicate symbol vlan.present used where lvalue 
required.
 reg0 <-> reg1[0..10]; => Can't exchange 32-bit field with 11-bit field.
 inport <-> reg0; => Can't exchange string field (inport) with integer field 
(reg0).
 inport <-> big_string; => String fields inport and big_string are incompatible 
for exchange.
-- 
2.1.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 14/14] ovn: Implement basic ARP support for L3 logical routers.

[Ben Pfaff]

This is sufficient support that an L3 logical router can now transmit
packets to VMs (and other destinations) without having to know the
IP-to-MAC binding in advance.  The details are carefully documented in all
of the appropriate places.

There are several important caveats that need to be fixed before this can
be taken seriously in production.  These are documented in ovn/TODO.  The
most important of these are renewal, expiration, and limiting the size of
the ARP table.

Signed-off-by: Ben Pfaff 
---
 ovn/TODO|  55 ++-
 ovn/controller/lflow.c  |  82 ++--
 ovn/controller/lflow.h  |   1 +
 ovn/controller/ovn-controller.c |   9 +-
 ovn/controller/pinctrl.c| 206 ++--
 ovn/controller/pinctrl.h|   5 +-
 ovn/lib/actions.c   | 165 
 ovn/lib/actions.h   |  11 +++
 ovn/lib/expr.c  |  53 +++
 ovn/lib/expr.h  |   3 +
 ovn/northd/ovn-northd.8.xml | 112 +-
 ovn/northd/ovn-northd.c | 105 +++-
 ovn/ovn-architecture.7.xml  |  78 ++-
 ovn/ovn-sb.ovsschema|  15 ++-
 ovn/ovn-sb.xml  | 137 +-
 ovn/utilities/ovn-sbctl.c   |   4 +
 tests/ovn.at| 174 ++---
 tests/test-ovn.c|   1 +
 18 files changed, 1015 insertions(+), 201 deletions(-)

diff --git a/ovn/TODO b/ovn/TODO
index a827421..bdbf86f 100644
--- a/ovn/TODO
+++ b/ovn/TODO
@@ -4,18 +4,11 @@
 
 ** New OVN logical actions
 
-*** arp
-
-Generates an ARP packet based on the current IPv4 packet and allows it
-to be processed as part of the current pipeline (and then pop back to
-processing the original IPv4 packet).
+*** rate_limit
 
 TCP/IP stacks typically limit the rate at which ARPs are sent, e.g. to
 one per second for a given target.  We might need to do this too.
 
-We probably need to buffer the packet that generated the ARP.  I don't
-know where to do that.
-
 *** icmp4 { action... }
 
 Generates an ICMPv4 packet based on the current IPv4 packet and
@@ -117,37 +110,13 @@ userspace-only and no one has complained yet.)
 
 ** Dynamic IP to MAC bindings
 
-Some bindings from IP address to MAC will undoubtedly need to be
-discovered dynamically through ARP requests.  It's straightforward
-enough for a logical L3 router to generate ARP requests and forward
-them to the appropriate switch.
-
-It's more difficult to figure out where the reply should be processed
-and stored.  It might seem at first that a first-cut implementation
-could just keep track of the binding on the hypervisor that needs to
-know, but that can't happen easily because the VM that sends the reply
-might not be on the same HV as the VM that needs the answer (that is,
-the VM that sent the packet that needs the binding to be resolved) and
-there isn't an easy way for it to know which HV needs the answer.
-
-Thus, the HV that processes the ARP reply (which is unknown when the
-ARP is sent) has to tell all the HVs the binding.  The most obvious
-place for this in the OVN_Southbound database.
-
-Details need to be worked out, including:
-
-*** OVN_Southbound schema changes.
+OVN has basic support for establishing IP to MAC bindings dynamically,
+using ARP.
 
-Possibly bindings could be added to the Port_Binding table by adding
-or modifying columns.  Another possibility is that another table
-should be added.
+*** Ratelimiting.
 
-*** Logical_Flow representation
-
-It would be really nice to maintain the general-purpose nature of
-logical flows, but these bindings might have to include some
-hard-coded special cases, especially when it comes to the relationship
-with populating the bindings into the OVN_Southbound table.
+From casual observation, Linux appears to generate at most one ARP per
+second per destination.
 
 *** Tracking queries
 
@@ -161,16 +130,12 @@ into the database.
 Something needs to make sure that bindings remain valid and expire
 those that become stale.
 
-** MTU handling (fragmentation on output)
-
-** Ratelimiting.
+*** Table size limiting.
 
-*** ARP.
+The table of MAC bindings must not be allowed to grow unreasonably
+large.
 
-*** ICMP error generation, TCP reset, UDP unreachable, protocol unreachable, 
...
-
-As a point of comparison, Linux doesn't ratelimit TCP resets but I
-think it does everything else.
+** MTU handling (fragmentation on output)
 
 * ovn-controller
 
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 2bfc9e1..f6803df 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -178,14 +178,12 @@ lookup_port_cb(const void *aux_, const char *port_name, 
unsigned int *portp)
 return false;
 }
 
-/* Translates logical flows in the Logical_Flow table in the OVN_SB database
- * into OpenFlow flows.  See ovn-architecture(7) for more information. */
-void
-lfl