Nithin Raju | 20 Sep 03:31 2014

netdev-windows implementation approaches

hi Folks,
Following are my findings (not many) from looking at netdev and how to go about implementing it for Windows.
I'm posting this as followup to the discussion we had during IRC meeting. Pls. feel free to comment.

For reference, I looked at the code in the VMware repo (prior to opensourcing) where there was an
implementation of netdev-windows.c. I looked in the Cloudbase repo, but I could not find a
netdev-windows.c and the changes in netdev-linux.c were very minimal to call it ported code. Maybe it is,
and I missed something.

The hierarchy is something like this:

             netdev.c
                |
                -------> netdev-vport.c implements netdev for tunnel ports, eg "gre"/"vxlan" etc.
                |
                -------> netdev-[linux|bsd].c implement netdev for "system"/"tap"/"internal" ports.

To quote from the "PORTING" guide:
The "system" type corresponds to an existing network device on the system.

OVS_VPORT_TYPE_NETDEV maps to "system", and "OVS_VPORT_TYPE_INTERNAL" maps to "internal". Each
platform has to implement the ability to open these devices as a 'netdev'. So, basically, we'll have to
implement netdev-windows.c for the two types of ports - "system" and "internal". We cannot directly
leverage netdev-linux.c unlike we did for dpif-linux.c.

We have two options:
1) Use Windows APIs such as GetAdaptersInfo() to query the details of a network interface on the Windows
host. Eitan suggested that we use Object Notifier.
-----------
a) The upside of this approach is that we don't need to add new commands to the OVS kernel, and
(Continue reading)

Daniele Di Proietto | 20 Sep 01:30 2014

[PATCH] dpif-netdev: reduce netdev_flow_key size

Signed-off-by: Daniele Di Proietto <ddiproietto@...>
Signed-off-by: Jarno Rajahalme <jrajahalme@...>
---
This is based on a previous patch by Jarno
---
 lib/dpif-netdev.c |  9 +--------
 lib/flow.h        | 13 +++++++++++++
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 90fe01c..20d2cd2 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
 <at>  <at>  -90,16 +90,9  <at>  <at>  static struct vlog_rate_limit upcall_rl = VLOG_RATE_LIMIT_INIT(600, 600);

 /* Stores a miniflow with inline values */

-/* There are fields in the flow structure that we never use. Therefore we can
- * save a few words of memory */
-#define NETDEV_KEY_BUF_SIZE_U32 (FLOW_U32S                 \
-                                 - MINI_N_INLINE           \
-                                 - FLOW_U32_SIZE(regs)     \
-                                 - FLOW_U32_SIZE(metadata) \
-                                )
 struct netdev_flow_key {
     struct miniflow flow;
-    uint32_t buf[NETDEV_KEY_BUF_SIZE_U32];
+    uint32_t buf[FLOW_MAX_PACKET_U32S - MINI_N_INLINE];
 };

(Continue reading)

Daniele Di Proietto | 20 Sep 01:20 2014

[PATCH v2] dpif-netdev: Fix (packet) memory leaks in the slow path.

If a packet didn't match a rule in the fast path classifier its memory was
never freed. The issue was particularly clear with DPDK devices because it was
not possible to process more than ~250000 DPDK mbufs in the slow path.

This commit fixes the problem by:
* calling dpif_packet_delete() if the upcalls are disabled
* passing may_steal==true to dp_netdev_execute_actions() during normal upcall
  processing

Signed-off-by: Daniele Di Proietto <ddiproietto@...>
---
v2:
* (Pravin's suggestion) restructured dp_execute_cb to return immediately if
  successful and free packets at the end
---
 lib/dpif-netdev.c | 56 +++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 20 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 90fe01c..6b8201b 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
 <at>  <at>  -2681,7 +2681,7  <at>  <at>  fast_path_processing(struct dp_netdev_pmd_thread *pmd,
             /* We can't allow the packet batching in the next loop to execute
              * the actions.  Otherwise, if there are any slow path actions,
              * we'll send the packet up twice. */
-            dp_netdev_execute_actions(pmd, &packets[i], 1, false, md,
+            dp_netdev_execute_actions(pmd, &packets[i], 1, true, md,
                                       ofpbuf_data(&actions),
                                       ofpbuf_size(&actions));
(Continue reading)

Nithin Raju | 20 Sep 00:34 2014

[PATCH] lib/dpif-netlink.c: rename linux_flow variable to datapath_flow

In the flow related functions, there's a stack variable called
'linux_flow'. Since this code is not specific to Linux anymore,
in this patch, we rename the variable to 'datpath_flow'.

Signed-off-by: Nithin Raju <nithin@...>
---
 lib/dpif-netlink.c |   32 ++++++++++++++++----------------
 1 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 76041e7..d43aab0 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
 <at>  <at>  -1234,15 +1234,15  <at>  <at>  dpif_netlink_flow_dump_thread_destroy(struct dpif_flow_dump_thread *thread_)

 static void
 dpif_netlink_flow_to_dpif_flow(struct dpif_flow *dpif_flow,
-                               const struct dpif_netlink_flow *linux_flow)
+                               const struct dpif_netlink_flow *datapath_flow)
 {
-    dpif_flow->key = linux_flow->key;
-    dpif_flow->key_len = linux_flow->key_len;
-    dpif_flow->mask = linux_flow->mask;
-    dpif_flow->mask_len = linux_flow->mask_len;
-    dpif_flow->actions = linux_flow->actions;
-    dpif_flow->actions_len = linux_flow->actions_len;
-    dpif_netlink_flow_get_stats(linux_flow, &dpif_flow->stats);
+    dpif_flow->key = datapath_flow->key;
+    dpif_flow->key_len = datapath_flow->key_len;
+    dpif_flow->mask = datapath_flow->mask;
(Continue reading)

Nithin Raju | 19 Sep 23:30 2014

[PATCH 1/5 v2] datapath-windows: return TRUE on success in NlAttrValidate

Signed-off-by: Nithin Raju <nithin@...>
Acked-by: Samuel Ghinet <sghinet@...>
---
 datapath-windows/ovsext/Netlink/Netlink.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/datapath-windows/ovsext/Netlink/Netlink.c b/datapath-windows/ovsext/Netlink/Netlink.c
index 1ceb5e3..c286c2f 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
 <at>  <at>  -806,12 +806,12  <at>  <at>  NlAttrValidate(const PNL_ATTR nla, const PNL_POLICY policy)
     UINT32 minLen;
     UINT32 maxLen;
     UINT32 len;
-    BOOLEAN ret = FALSE;
+    BOOLEAN ret = TRUE;

     if ((policy->type == NL_A_NO_ATTR) ||
         (policy->type == NL_A_VAR_LEN)) {
         /* Do not validate anything for attributes of type var length */
-        ret = TRUE;
+        ret = FALSE;
         goto done;
     }

 <at>  <at>  -830,6 +830,7  <at>  <at>  NlAttrValidate(const PNL_ATTR nla, const PNL_POLICY policy)
     if (len < minLen || len > maxLen) {
         OVS_LOG_WARN("Attribute: %p, len: %d, not in valid range, "
                      "min: %d, max: %d", nla, len, minLen, maxLen);
+        ret = FALSE;
(Continue reading)

Daniele Di Proietto | 19 Sep 22:28 2014

[PATCH] dpif-netdev: Fix (packet) memory leaks in the slow path.

If a packet didn't match a rule in the fast path classifier its memory was
never freed. The issue was particularly clear with DPDK devices because it was
not possible to process more than ~250000 DPDK mbufs in the slow path.

This commit fixes the problem by:
* calling dpif_packet_delete() if the upcalls are disabled
* passing may_steal==true to dp_netdev_execute_actions() during normal upcall
  processing

Signed-off-by: Daniele Di Proietto <ddiproietto@...>
---
 lib/dpif-netdev.c | 47 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 15 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 7e27acf..8df3b81 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
 <at>  <at>  -2576,7 +2576,7  <at>  <at>  fast_path_processing(struct dp_netdev_pmd_thread *pmd,
             /* We can't allow the packet batching in the next loop to execute
              * the actions.  Otherwise, if there are any slow path actions,
              * we'll send the packet up twice. */
-            dp_netdev_execute_actions(pmd, &packets[i], 1, false, md,
+            dp_netdev_execute_actions(pmd, &packets[i], 1, true, md,
                                       ofpbuf_data(&actions),
                                       ofpbuf_size(&actions));

 <at>  <at>  -2602,6 +2602,17  <at>  <at>  fast_path_processing(struct dp_netdev_pmd_thread *pmd,
         ofpbuf_uninit(&actions);
         ofpbuf_uninit(&put_actions);
(Continue reading)

Ben Pfaff | 19 Sep 22:09 2014

[PATCH 1/2] travis: Include testsuite.log on failure.

CC: Thomas Graf <tgraf@...>
Signed-off-by: Ben Pfaff <blp@...>
---
 .travis/build.sh |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/.travis/build.sh b/.travis/build.sh
index 0a23969..db7a3d3 100755
--- a/.travis/build.sh
+++ b/.travis/build.sh
 <at>  <at>  -52,7 +52,13  <at>  <at>  if [ $CC = "clang" ]; then
     make CFLAGS="$CFLAGS -Wno-error=unused-command-line-argument"
 else
     make CFLAGS="$CFLAGS" C=1
-    [ "$TESTSUITE" ] && make distcheck
+    if [ $TESTSUITE ]; then
+        if ! make distcheck; then
+            # testsuite.log is necessary for debugging.
+            cat */_build/tests/testsuite.log
+            exit 1
+        fi
+    fi
 fi

 exit 0
--

-- 
1.7.10.4

Alex Wang | 19 Sep 19:57 2014

[PATCH 1/2] netdev-dpdk: Fix a typo.

Signed-off-by: Alex Wang <alexw@...>
---
 lib/netdev-dpdk.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index f2a42e8..ed39b9c 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
 <at>  <at>  -472,11 +472,11  <at>  <at>  netdev_dpdk_set_txq(struct netdev_dpdk *netdev, unsigned int n_txqs)
     /* Each index is considered as a cpu core id, since there should
      * be one tx queue for each cpu core. */
     for (i = 0; i < n_txqs; i++) {
-        int core_id = ovs_numa_get_numa_id(i);
+        int numa_id = ovs_numa_get_numa_id(i);

         /* If the corresponding core is not on the same numa node
          * as 'netdev', flags the 'flush_tx'. */
-        netdev->tx_q[i].flush_tx = netdev->socket_id == core_id;
+        netdev->tx_q[i].flush_tx = netdev->socket_id == numa_id;
     }
 }

--

-- 
1.7.9.5

YAMAMOTO Takashi | 19 Sep 18:27 2014
Picon

table features

hi,

it seems master produces table-features with NXM_0, NXM_1,
and even packet_regs for OF1.3.

i don't think it's a good idea to include NXM stuff
because the OF standard doesn't seem to define how to parse them.
(the size of OXM ids are class-dependant)

packet_regs seems like an OF1.5 stuff which should not be included
when speaking OF1.3.

i wrote a workaround for LINC but it's better to be fixed in OVS.
https://github.com/yamt/of_protocol/commit/8e0700190ecedde8f24772a9f40551a21d13973e

how do you think?

YAMAMOTO Takashi
Flynn, Dennis R (Dennis | 19 Sep 18:11 2014

Contribute Auto-Attach SPBm support to Open vSwitch

Greetings OVS Team,

We have been working on adding OVS support for the IEEE/IETF Auto-Attach SPBm draft standard. This
standard describes a compact method of using IEEE 802.1AB Link Layer Discovery Protocol (LLDP) together
with a IEEE 802.1aq Shortest Path Bridging (SPBm) network to automatically attach network devices to
individual services in a SPBm network.  The intent here is to allow network applications and devices using
OVS to be able to easily take advantage of features offered by industry standard SPBm networks.  Details of
the Auto-Attach standard can be found here.

http://tools.ietf.org/html/draft-unbehagen-lldp-spb-00

In addition to this standards work we have proposed Auto-Attach to the IEEE 802.1Q technical committee for
inclusion into that standard. This IEEE standards work is ongoing and the initial reaction from the
standards committee has been very positive.

Specifically we have modified the OVS source code to integrate basic LLDP protocol support as required to
implement the Auto-Attach (AA) standard.  We modeled our LLDP code changes after other protocols
currently supported by OVS (BFD, CFM, etc.).  We have chosen to base this OVS LLDP work on the open source
LLDPD project headed by Vincent Bernat.  Details of the LLDPD project can be found here.

http://vincentbernat.github.io/lldpd

Although the LLDPD project provides a full LLDP implementation as per the IEEE 802.1AB standard, our
initial offering focuses on the core pieces of LLDP required to provide AA support.  We have plans in place
to extend this work to include full LLDP support within OVS.

We have reached a point in our development where we would like to engage the OVS development community
regarding how best to contribute our work upstream. We have integrated core LLDP support into OVS and have
augmented that to support Auto-Attach.  We have extended OVSDB and unixctl adding new commands to support
the configuration, status display, and statistics display of Auto-Attach.  We have added the associated
(Continue reading)

Gurucharan Shetty | 19 Sep 16:29 2014

[PATCH v3 1/6] README.ovs-vtep: Remotes can be connected through VTEP's manager table.

Reported-by: Ziyou Wang <ziyouw@...>
Signed-off-by: Gurucharan Shetty <gshetty@...>
---
 AUTHORS              |    1 +
 vtep/README.ovs-vtep |    1 +
 2 files changed, 2 insertions(+)

diff --git a/AUTHORS b/AUTHORS
index e3fe7ba..e2db8db 100644
--- a/AUTHORS
+++ b/AUTHORS
 <at>  <at>  -297,6 +297,7  <at>  <at>  Voravit T.              voravit@...
 Yeming Zhao             zhaoyeming@...
 Ying Chen               yingchen@...
 Yongqiang Liu           liuyq7809@...
+Ziyou Wang              ziyouw@...
 ankur dwivedi           ankurengg2003@...
 chen zhang              3zhangchen9211@...
 kk yap                  yapkke@...
diff --git a/vtep/README.ovs-vtep b/vtep/README.ovs-vtep
index 60b39b7..5ce63c7 100644
--- a/vtep/README.ovs-vtep
+++ b/vtep/README.ovs-vtep
 <at>  <at>  -95,6 +95,7  <at>  <at>  step 2 of the "Requirements" section.

     ovsdb-server --pidfile --detach --log-file \
       --remote punix:/var/run/openvswitch/db.sock \
+      --remote=db:hardware_vtep,Global,managers \
       /etc/openvswitch/ovs.db /etc/openvswitch/vtep.db

(Continue reading)


Gmane