Re: [ovs-dev] [netdev v3 1/4] netdev: Factor restoring flags into new "struct netdev_saved_flags".
On 9 May 2013 18:35, Ben Pfaff wrote: > This gets rid of the only per-instance data in "struct netdev", which > will make it possible to merge "struct netdev_dev" into "struct netdev" in > a later commit. The four patches generally look good to me, and when I apply the whole set they pass the unit tests on FreeBSD. I tried rebasing the original BSD threaded userspace datapath patch on top of the set, but it didn't work properly. To further that investigation I planned to try merging each of these patches incrementally, and upon testing just this first one ran into a use-after-free (with the default malloc diagnostics enabled for FreeBSD in the unit tests): (gdb) bt #0 netdev_dev_unref (dev=0x5a5a5a5a5a5a5a5a) at lib/netdev.c:314 #1 0x00412158 in update_port (ofproto=0x801cce010, name=) at ofproto/ofproto.c:1975 #2 0x00414405 in reinit_ports (p=) at ofproto/ofproto.c:1660 #3 process_port_change (devname=, error=, ofproto=) at ofproto/ofproto.c:1115 #4 ofproto_run (p=0x801cce010) at ofproto/ofproto.c:1186 #5 0x0040ccd7 in bridge_run () at vswitchd/bridge.c:2327 #6 0x0040dcfa in main (argc=, argv=) at vswitchd/ovs-vswitchd.c:125 It looks like a ref_cnt increment is missing somewhere. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [netdev v3 1/4] netdev: Factor restoring flags into new "struct netdev_saved_flags".
On Fri, May 10, 2013 at 10:51:34AM -0400, Ed Maste wrote:
> On 9 May 2013 18:35, Ben Pfaff wrote:
> > This gets rid of the only per-instance data in "struct netdev", which
> > will make it possible to merge "struct netdev_dev" into "struct netdev" in
> > a later commit.
>
> The four patches generally look good to me, and when I apply the whole
> set they pass the unit tests on FreeBSD.
>
> I tried rebasing the original BSD threaded userspace datapath patch on
> top of the set, but it didn't work properly. To further that
> investigation I planned to try merging each of these patches
> incrementally, and upon testing just this first one ran into a
> use-after-free (with the default malloc diagnostics enabled for
> FreeBSD in the unit tests):
>
> (gdb) bt
> #0 netdev_dev_unref (dev=0x5a5a5a5a5a5a5a5a) at lib/netdev.c:314
> #1 0x00412158 in update_port (ofproto=0x801cce010,
> name=) at ofproto/ofproto.c:1975
> #2 0x00414405 in reinit_ports (p=)
> at ofproto/ofproto.c:1660
> #3 process_port_change (devname=, error=,
> ofproto=) at ofproto/ofproto.c:1115
> #4 ofproto_run (p=0x801cce010) at ofproto/ofproto.c:1186
> #5 0x0040ccd7 in bridge_run () at vswitchd/bridge.c:2327
> #6 0x0040dcfa in main (argc=, argv=)
> at vswitchd/ovs-vswitchd.c:125
>
> It looks like a ref_cnt increment is missing somewhere.
Thanks, I found the problem with valgrind, and here is an incremental
fix:
diff --git a/lib/netdev.c b/lib/netdev.c
index 9590f83..415cdb4 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -322,8 +322,10 @@ void
netdev_close(struct netdev *netdev)
{
if (netdev) {
+struct netdev_dev *dev = netdev_get_dev(netdev);
+
netdev_uninit(netdev, true);
-netdev_dev_unref(netdev_get_dev(netdev));
+netdev_dev_unref(dev);
}
}
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] Use updated dl_type when checking actions that use fields
On Thu, May 9, 2013 at 7:00 PM, Simon Horman wrote: > Update handling of the following actions to use the dl_type set by MPLS > push and pop actions if it differs from the original dl_type. This is > consistent with the existing checking of load actions and allows > their existing checks to enforce dl_type pre-requisites correctly. > > output_reg > bundle > reg_move > stack_push > stack_pop > learn > multipath > > In order to avoid the verbosity of updating the flow for each applicable > action the update is treated as a common case and performed in > ofpact_check(). This was suggested by Jesse Gross. > > Cc: Jesse Gross > Signed-off-by: Simon Horman Applied. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [netdev v4 0/4] refactor netdev code to make more sense
Changes v1->v2: - Initial two patches dropped because they have been applied. - Fixed merge conflicts against current master. Changes v2->v3: - Fixed merge conflicts against current master. Changes v3->v4: - Fix use-after-free in patch 1 reported by Ed Maste. Ben Pfaff (4): netdev: Factor restoring flags into new "struct netdev_saved_flags". netdev: Add new "struct netdev_rx" for capturing packets from a netdev. Rename superclass members to 'up'. netdev: Get rid of netdev_dev. lib/dpif-netdev.c | 19 +- lib/netdev-bsd.c | 530 +--- lib/netdev-dummy.c| 271 +++--- lib/netdev-linux.c| 940 ++--- lib/netdev-provider.h | 180 -- lib/netdev-vport.c| 145 lib/netdev.c | 542 +--- lib/netdev.h | 49 ++- ofproto/ofproto.c |4 +- utilities/ovs-dpctl.c |4 +- vswitchd/bridge.c |4 +- 11 files changed, 1225 insertions(+), 1463 deletions(-) -- 1.7.10.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [netdev v4 1/4] netdev: Factor restoring flags into new "struct netdev_saved_flags".
This gets rid of the only per-instance data in "struct netdev", which
will make it possible to merge "struct netdev_dev" into "struct netdev" in
a later commit.
Signed-off-by: Ben Pfaff
---
lib/dpif-netdev.c |6 +-
lib/netdev-dummy.c|5 +-
lib/netdev-linux.c| 22 +++---
lib/netdev-provider.h | 14 ++--
lib/netdev-vport.c|2 +-
lib/netdev.c | 177 +
lib/netdev.h | 28 +---
ofproto/ofproto.c |4 +-
utilities/ovs-dpctl.c |4 +-
vswitchd/bridge.c |4 +-
10 files changed, 152 insertions(+), 114 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 632a1de..40f59c3 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -106,6 +106,7 @@ struct dp_netdev_port {
int port_no;/* Index into dp_netdev's 'ports'. */
struct list node; /* Element in dp_netdev's 'port_list'. */
struct netdev *netdev;
+struct netdev_saved_flags *sf;
char *type; /* Port type as requested by user. */
};
@@ -374,6 +375,7 @@ static int
do_add_port(struct dp_netdev *dp, const char *devname, const char *type,
uint32_t port_no)
{
+struct netdev_saved_flags *sf;
struct dp_netdev_port *port;
struct netdev *netdev;
const char *open_type;
@@ -400,7 +402,7 @@ do_add_port(struct dp_netdev *dp, const char *devname,
const char *type,
return error;
}
-error = netdev_turn_flags_on(netdev, NETDEV_PROMISC, false);
+error = netdev_turn_flags_on(netdev, NETDEV_PROMISC, &sf);
if (error) {
netdev_close(netdev);
return error;
@@ -409,6 +411,7 @@ do_add_port(struct dp_netdev *dp, const char *devname,
const char *type,
port = xmalloc(sizeof *port);
port->port_no = port_no;
port->netdev = netdev;
+port->sf = sf;
port->type = xstrdup(type);
error = netdev_get_mtu(netdev, &mtu);
@@ -505,6 +508,7 @@ do_del_port(struct dp_netdev *dp, uint32_t port_no)
dp->serial++;
netdev_close(port->netdev);
+netdev_restore_flags(port->sf);
free(port->type);
free(port);
diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index bdb3ea1..de04f9a 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -316,12 +316,11 @@ netdev_dummy_get_ifindex(const struct netdev *netdev)
}
static int
-netdev_dummy_update_flags(struct netdev *netdev,
+netdev_dummy_update_flags(struct netdev_dev *dev_,
enum netdev_flags off, enum netdev_flags on,
enum netdev_flags *old_flagsp)
{
-struct netdev_dev_dummy *dev =
-netdev_dev_dummy_cast(netdev_get_dev(netdev));
+struct netdev_dev_dummy *dev = netdev_dev_dummy_cast(dev_);
return netdev_dev_dummy_update_flags(dev, off, on, old_flagsp);
}
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 722b88b..30cd0f6 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -422,7 +422,7 @@ static int netdev_linux_do_ioctl(const char *name, struct
ifreq *, int cmd,
static int netdev_linux_get_ipv4(const struct netdev *, struct in_addr *,
int cmd, const char *cmd_name);
static int get_flags(const struct netdev_dev *, unsigned int *flags);
-static int set_flags(struct netdev *, unsigned int flags);
+static int set_flags(const char *, unsigned int flags);
static int do_get_ifindex(const char *netdev_name);
static int get_ifindex(const struct netdev *, int *ifindexp);
static int do_set_addr(struct netdev *netdev,
@@ -1004,8 +1004,8 @@ netdev_linux_set_etheraddr(struct netdev *netdev_,
{
struct netdev_dev_linux *netdev_dev =
netdev_dev_linux_cast(netdev_get_dev(netdev_));
+struct netdev_saved_flags *sf = NULL;
int error;
-bool up_again = false;
if (netdev_dev->cache_valid & VALID_ETHERADDR) {
if (netdev_dev->ether_addr_error) {
@@ -1022,8 +1022,7 @@ netdev_linux_set_etheraddr(struct netdev *netdev_,
enum netdev_flags flags;
if (!netdev_get_flags(netdev_, &flags) && (flags & NETDEV_UP)) {
-netdev_turn_flags_off(netdev_, NETDEV_UP, false);
-up_again = true;
+netdev_turn_flags_off(netdev_, NETDEV_UP, &sf);
}
}
error = set_etheraddr(netdev_get_name(netdev_), mac);
@@ -1035,9 +1034,7 @@ netdev_linux_set_etheraddr(struct netdev *netdev_,
}
}
-if (up_again) {
-netdev_turn_flags_on(netdev_, NETDEV_UP, false);
-}
+netdev_restore_flags(sf);
return error;
}
@@ -2450,19 +2447,19 @@ iff_to_nd_flags(int iff)
}
static int
-netdev_linux_update_flags(struct netdev *netdev, enum netdev_flags off,
+netdev_linux_update_flags(struct netdev_dev *dev_, enum netdev_flags off,
enum netdev_flags on, enum netdev_flags *old_flagsp)
{
struct netdev_dev_linux *netdev_dev;
int old_flags, new_flags;
[ovs-dev] [netdev v4 3/4] Rename superclass members to 'up'.
---
lib/netdev-bsd.c | 24
lib/netdev-dummy.c | 18 +-
lib/netdev-linux.c | 38 +++---
lib/netdev-vport.c |8
4 files changed, 44 insertions(+), 44 deletions(-)
diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
index 8950a4c..3518df8 100644
--- a/lib/netdev-bsd.c
+++ b/lib/netdev-bsd.c
@@ -79,7 +79,7 @@ VLOG_DEFINE_THIS_MODULE(netdev_bsd);
* struct, and using CONTAINER_OF to access the parent structure.
*/
struct netdev_bsd {
-struct netdev netdev;
+struct netdev up;
};
struct netdev_rx_bsd {
@@ -97,7 +97,7 @@ struct netdev_rx_bsd {
static const struct netdev_rx_class netdev_rx_bsd_class;
struct netdev_dev_bsd {
-struct netdev_dev netdev_dev;
+struct netdev_dev up;
unsigned int cache_valid;
unsigned int change_seq;
@@ -172,14 +172,14 @@ netdev_bsd_cast(const struct netdev *netdev)
{
ovs_assert(is_netdev_bsd_class(netdev_dev_get_class(
netdev_get_dev(netdev;
-return CONTAINER_OF(netdev, struct netdev_bsd, netdev);
+return CONTAINER_OF(netdev, struct netdev_bsd, up);
}
static struct netdev_dev_bsd *
netdev_dev_bsd_cast(const struct netdev_dev *netdev_dev)
{
ovs_assert(is_netdev_bsd_class(netdev_dev_get_class(netdev_dev)));
-return CONTAINER_OF(netdev_dev, struct netdev_dev_bsd, netdev_dev);
+return CONTAINER_OF(netdev_dev, struct netdev_dev_bsd, up);
}
static struct netdev_rx_bsd *
@@ -318,9 +318,9 @@ netdev_bsd_create_system(const struct netdev_class *class,
const char *name,
netdev_dev = xzalloc(sizeof *netdev_dev);
netdev_dev->change_seq = 1;
-netdev_dev_init(&netdev_dev->netdev_dev, name, class);
+netdev_dev_init(&netdev_dev->up, name, class);
netdev_dev->tap_fd = -1;
-*netdev_devp = &netdev_dev->netdev_dev;
+*netdev_devp = &netdev_dev->up;
return 0;
}
@@ -390,8 +390,8 @@ netdev_bsd_create_tap(const struct netdev_class *class,
const char *name,
/* initialize the device structure and
* link the structure to its netdev */
-netdev_dev_init(&netdev_dev->netdev_dev, name, class);
-*netdev_devp = &netdev_dev->netdev_dev;
+netdev_dev_init(&netdev_dev->up, name, class);
+*netdev_devp = &netdev_dev->up;
return 0;
@@ -428,19 +428,19 @@ netdev_bsd_open_system(struct netdev_dev *netdev_dev_,
struct netdev **netdevp)
/* Allocate network device. */
netdev = xcalloc(1, sizeof *netdev);
-netdev_init(&netdev->netdev, netdev_dev_);
+netdev_init(&netdev->up, netdev_dev_);
/* Verify that the netdev really exists by attempting to read its flags */
-error = netdev_get_flags(&netdev->netdev, &flags);
+error = netdev_get_flags(&netdev->up, &flags);
if (error == ENXIO) {
goto error;
}
-*netdevp = &netdev->netdev;
+*netdevp = &netdev->up;
return 0;
error:
-netdev_uninit(&netdev->netdev, true);
+netdev_uninit(&netdev->up, true);
return error;
}
diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 3762d5c..dabff18 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -43,7 +43,7 @@ VLOG_DEFINE_THIS_MODULE(netdev_dummy);
#endif
struct netdev_dev_dummy {
-struct netdev_dev netdev_dev;
+struct netdev_dev up;
uint8_t hwaddr[ETH_ADDR_LEN];
int mtu;
struct netdev_stats stats;
@@ -55,7 +55,7 @@ struct netdev_dev_dummy {
};
struct netdev_dummy {
-struct netdev netdev;
+struct netdev up;
};
struct netdev_rx_dummy {
@@ -87,7 +87,7 @@ static struct netdev_dev_dummy *
netdev_dev_dummy_cast(const struct netdev_dev *netdev_dev)
{
ovs_assert(is_dummy_class(netdev_dev_get_class(netdev_dev)));
-return CONTAINER_OF(netdev_dev, struct netdev_dev_dummy, netdev_dev);
+return CONTAINER_OF(netdev_dev, struct netdev_dev_dummy, up);
}
static struct netdev_dummy *
@@ -95,7 +95,7 @@ netdev_dummy_cast(const struct netdev *netdev)
{
struct netdev_dev *netdev_dev = netdev_get_dev(netdev);
ovs_assert(is_dummy_class(netdev_dev_get_class(netdev_dev)));
-return CONTAINER_OF(netdev, struct netdev_dummy, netdev);
+return CONTAINER_OF(netdev, struct netdev_dummy, up);
}
static struct netdev_rx_dummy *
@@ -113,7 +113,7 @@ netdev_dummy_create(const struct netdev_class *class, const
char *name,
struct netdev_dev_dummy *netdev_dev;
netdev_dev = xzalloc(sizeof *netdev_dev);
-netdev_dev_init(&netdev_dev->netdev_dev, name, class);
+netdev_dev_init(&netdev_dev->up, name, class);
netdev_dev->hwaddr[0] = 0xaa;
netdev_dev->hwaddr[1] = 0x55;
netdev_dev->hwaddr[2] = n >> 24;
@@ -130,7 +130,7 @@ netdev_dummy_create(const struct netdev_class *class, const
char *name,
n++;
-*netdev_devp = &netdev_dev->netdev_dev;
+*netdev_devp = &netdev_dev->up;
return 0;
}
@@ -172,9 +172,9 @@ netdev_dummy_open(struct netdev_dev *netdev_dev_, struct
netdev **netde
[ovs-dev] [netdev v4 2/4] netdev: Add new "struct netdev_rx" for capturing packets from a netdev.
Separating packet capture from "struct netdev" means that there is no
remaining per-"struct netdev" state, which will allow us to get rid of
"struct netdev_dev" (by renaming it to "struct netdev").
Signed-off-by: Ben Pfaff
---
lib/dpif-netdev.c | 13 ++-
lib/netdev-bsd.c | 309 ++---
lib/netdev-dummy.c| 95 +--
lib/netdev-linux.c| 216 ++
lib/netdev-provider.h | 93 ---
lib/netdev-vport.c|5 +-
lib/netdev.c | 106 +
lib/netdev.h | 21 ++--
8 files changed, 469 insertions(+), 389 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 40f59c3..78bdedb 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -107,6 +107,7 @@ struct dp_netdev_port {
struct list node; /* Element in dp_netdev's 'port_list'. */
struct netdev *netdev;
struct netdev_saved_flags *sf;
+struct netdev_rx *rx;
char *type; /* Port type as requested by user. */
};
@@ -378,6 +379,7 @@ do_add_port(struct dp_netdev *dp, const char *devname,
const char *type,
struct netdev_saved_flags *sf;
struct dp_netdev_port *port;
struct netdev *netdev;
+struct netdev_rx *rx;
const char *open_type;
int mtu;
int error;
@@ -393,7 +395,7 @@ do_add_port(struct dp_netdev *dp, const char *devname,
const char *type,
/* XXX reject loopback devices */
/* XXX reject non-Ethernet devices */
-error = netdev_listen(netdev);
+error = netdev_rx_open(netdev, &rx);
if (error
&& !(error == EOPNOTSUPP && dpif_netdev_class_is_dummy(dp->class))) {
VLOG_ERR("%s: cannot receive packets on this network device (%s)",
@@ -404,6 +406,7 @@ do_add_port(struct dp_netdev *dp, const char *devname,
const char *type,
error = netdev_turn_flags_on(netdev, NETDEV_PROMISC, &sf);
if (error) {
+netdev_rx_close(rx);
netdev_close(netdev);
return error;
}
@@ -412,6 +415,7 @@ do_add_port(struct dp_netdev *dp, const char *devname,
const char *type,
port->port_no = port_no;
port->netdev = netdev;
port->sf = sf;
+port->rx = rx;
port->type = xstrdup(type);
error = netdev_get_mtu(netdev, &mtu);
@@ -509,6 +513,7 @@ do_del_port(struct dp_netdev *dp, uint32_t port_no)
netdev_close(port->netdev);
netdev_restore_flags(port->sf);
+netdev_rx_close(port->rx);
free(port->type);
free(port);
@@ -1063,7 +1068,7 @@ dpif_netdev_run(struct dpif *dpif)
ofpbuf_clear(&packet);
ofpbuf_reserve(&packet, DP_NETDEV_HEADROOM);
-error = netdev_recv(port->netdev, &packet);
+error = port->rx ? netdev_rx_recv(port->rx, &packet) : EOPNOTSUPP;
if (!error) {
dp_netdev_port_input(dp, port, &packet);
} else if (error != EAGAIN && error != EOPNOTSUPP) {
@@ -1083,7 +1088,9 @@ dpif_netdev_wait(struct dpif *dpif)
struct dp_netdev_port *port;
LIST_FOR_EACH (port, node, &dp->port_list) {
-netdev_recv_wait(port->netdev);
+if (port->rx) {
+netdev_rx_wait(port->rx);
+}
}
}
diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
index 7ab9d3e..8950a4c 100644
--- a/lib/netdev-bsd.c
+++ b/lib/netdev-bsd.c
@@ -16,6 +16,7 @@
#include
+#include "netdev-provider.h"
#include
#include
#include
@@ -41,7 +42,6 @@
#include "coverage.h"
#include "dynamic-string.h"
#include "fatal-signal.h"
-#include "netdev-provider.h"
#include "ofpbuf.h"
#include "openflow/openflow.h"
#include "packets.h"
@@ -57,7 +57,7 @@ VLOG_DEFINE_THIS_MODULE(netdev_bsd);
/*
* This file implements objects to access interfaces.
- * Externally, interfaces are represented by two structures:
+ * Externally, interfaces are represented by three structures:
* + struct netdev_dev, representing a network device,
* containing e.g. name and a refcount;
* We can have private variables by embedding the
@@ -66,26 +66,36 @@ VLOG_DEFINE_THIS_MODULE(netdev_bsd);
*
* + struct netdev, representing an instance of an open netdev_dev.
* The structure contains a pointer to the 'struct netdev'
- * representing the device. Again, private information
- * such as file descriptor etc. are stored in our
- * own struct netdev_bsd which includes a struct netdev.
+ * representing the device.
+ *
+ * + struct netdev_rx, which represents a netdev open to capture received
+ * packets. Again, private information such as file descriptor etc. are
+ * stored in our own struct netdev_rx_bsd which includes a struct
+ * netdev_rx.
*
- * Both 'struct netdev' and 'struct netdev_dev' are referenced
- * in containers which hold pointers to the data structures.
- * We can reach our own struct netdev_XXX_bsd by putting a
- * struct netdev_XXX within our own struct, and using CONTAINER_OF
- * to access the parent struc
Re: [ovs-dev] [netdev v3 1/4] netdev: Factor restoring flags into new "struct netdev_saved_flags".
On 10 May 2013 11:55, Ben Pfaff wrote:
> Thanks, I found the problem with valgrind, and here is an incremental
> fix:
>
> diff --git a/lib/netdev.c b/lib/netdev.c
> index 9590f83..415cdb4 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -322,8 +322,10 @@ void
> netdev_close(struct netdev *netdev)
> {
> if (netdev) {
> +struct netdev_dev *dev = netdev_get_dev(netdev);
> +
> netdev_uninit(netdev, true);
> -netdev_dev_unref(netdev_get_dev(netdev));
> +netdev_dev_unref(dev);
> }
> }
Ahh, of course. This fixes the segfaulting tests for me, but the unit
tests now get stuck in "tunnel - input."
(gdb) bt
#0 0x0008015b4c9c in poll () from /lib/libc.so.7
#1 0x0041f693 in time_poll (pollfds=0x801c1a850, n_pollfds=2,
timeout_when=9223372036854775807, elapsed=0x7fffca74) at
lib/timeval.c:388
#2 0x00416472 in poll_block () at lib/poll-loop.c:249
#3 0x0040ab55 in do_vsctl (args=Variable "args" is not available.
) at utilities/ovs-vsctl.c:4126
#4 0x0040b56f in main (argc=47, argv=0x7fffcef8) at
utilities/ovs-vsctl.c:220
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [netdev v3 1/4] netdev: Factor restoring flags into new "struct netdev_saved_flags".
On 10 May 2013 13:12, Ed Maste wrote: > Ahh, of course. This fixes the segfaulting tests for me, but the unit > tests now get stuck in "tunnel - input." Oh, and of course the reason ovs-vsctl is stuck is that ovs-vswitchd segfaulted. #0 0x00080164b560 in strncpy () from /lib/libc.so.7 #1 0x0049ee29 in netdev_bsd_do_ioctl (netdev=0x801c231d0, ifr=0x7fffc9a0, cmd=3223349521, cmd_name=0x4d3303 "SIOCGIFFLAGS") at lib/netdev-bsd.c:1480 #2 0x0049eef2 in get_flags (flags=, netdev=) at lib/netdev-bsd.c:1389 #3 netdev_bsd_update_flags (netdev=0x7fffc9a0, off=0, on=0, old_flagsp=0x7fffca24) at lib/netdev-bsd.c:1230 #4 0x0044f527 in do_update_flags (netdev=0x801c22310, off=0, on=0, old_flagsp=0x7fffca6c, sfp=0x0) at lib/netdev.c:816 #5 0x004a03ea in netdev_bsd_open_system (netdev_dev_=0x801c231d0, netdevp=0x7fffcad8) at lib/netdev-bsd.c:412 #6 0x0044fa19 in netdev_open (name=0x70e2d0 "alc0", type=0x4a9d1f "system", netdevp=0x7fffcad8) at lib/netdev.c:241 #7 0x0044d366 in tunnel_get_status (netdev=, smap=0x7fffcb20) at lib/netdev-vport.c:226 #8 0x00406bf9 in iface_refresh_status (iface=0x801c25220) at vswitchd/bridge.c:1799 #9 0x00408f3b in iface_create (br=0x801cbf100, if_cfg=0x801c221c0, ofp_port=3) at vswitchd/bridge.c:1520 #10 0x00409335 in bridge_reconfigure_ofp () at vswitchd/bridge.c:568 #11 0x0040941e in bridge_reconfigure_continue (ovs_cfg=0x801c1d280) at vswitchd/bridge.c:589 #12 0x0040c85e in bridge_run () at vswitchd/bridge.c:2374 #13 0x0040dcfa in main (argc=, argv=) at vswitchd/ovs-vswitchd.c:125 (gdb) frame 0 I'm taking a look at it, just wanted to reply because my previous email is clearly lacking sufficient detail. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [netdev v3 1/4] netdev: Factor restoring flags into new "struct netdev_saved_flags".
On Fri, May 10, 2013 at 01:21:11PM -0400, Ed Maste wrote: > On 10 May 2013 13:12, Ed Maste wrote: > > Ahh, of course. This fixes the segfaulting tests for me, but the unit > > tests now get stuck in "tunnel - input." > > Oh, and of course the reason ovs-vsctl is stuck is that ovs-vswitchd > segfaulted. > > #0 0x00080164b560 in strncpy () from /lib/libc.so.7 > #1 0x0049ee29 in netdev_bsd_do_ioctl (netdev=0x801c231d0, > ifr=0x7fffc9a0, cmd=3223349521, cmd_name=0x4d3303 "SIOCGIFFLAGS") > at lib/netdev-bsd.c:1480 > #2 0x0049eef2 in get_flags (flags=, > netdev=) at lib/netdev-bsd.c:1389 > #3 netdev_bsd_update_flags (netdev=0x7fffc9a0, off=0, on=0, > old_flagsp=0x7fffca24) at lib/netdev-bsd.c:1230 > #4 0x0044f527 in do_update_flags (netdev=0x801c22310, off=0, > on=0, old_flagsp=0x7fffca6c, sfp=0x0) at lib/netdev.c:816 > #5 0x004a03ea in netdev_bsd_open_system > (netdev_dev_=0x801c231d0, netdevp=0x7fffcad8) at > lib/netdev-bsd.c:412 > #6 0x0044fa19 in netdev_open (name=0x70e2d0 "alc0", > type=0x4a9d1f "system", netdevp=0x7fffcad8) at lib/netdev.c:241 > #7 0x0044d366 in tunnel_get_status (netdev=, > smap=0x7fffcb20) at lib/netdev-vport.c:226 > #8 0x00406bf9 in iface_refresh_status (iface=0x801c25220) at > vswitchd/bridge.c:1799 > #9 0x00408f3b in iface_create (br=0x801cbf100, > if_cfg=0x801c221c0, ofp_port=3) at vswitchd/bridge.c:1520 > #10 0x00409335 in bridge_reconfigure_ofp () at vswitchd/bridge.c:568 > #11 0x0040941e in bridge_reconfigure_continue > (ovs_cfg=0x801c1d280) at vswitchd/bridge.c:589 > #12 0x0040c85e in bridge_run () at vswitchd/bridge.c:2374 > #13 0x0040dcfa in main (argc=, argv= out>) at vswitchd/ovs-vswitchd.c:125 > (gdb) frame 0 > > I'm taking a look at it, just wanted to reply because my previous > email is clearly lacking sufficient detail. Thanks for looking into it. I did compile the FreeBSD version of this patch series at some point in development but obviously I didn't focus on it. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3] OpenFlow-level flow-based tunneling support.
On Thu, May 09, 2013 at 03:24:16PM +0300, Jarno Rajahalme wrote: > Adds tun_src and tun_dst match and set capabilities via new NXM fields > NXM_NX_TUN_IPV4_SRC and NXM_NX_TUN_IPV4_DST. This allows management of > large number of tunnels via the flow tables, without requiring the tunnels > to be pre-configured. > > Flow-based tunnels can be configured with options remote_ip=flow and > local_ip=flow. local_ip=flow requires remote_ip=flow. When set, the > tunnel remote IP address and/or local IP address is set from the flow, > instead of the tunnel configuration. Applied. I noticed that there was no NEWS entry. I didn't want to delay this through another round of review, so I wrote this one myself: post-v1.11.0 - - OpenFlow: * New support for matching outer source and destination IP address of tunneled packets, for tunnel ports configured with the newly added "remote_ip=flow" and "local_ip=flow" options. Feel free to send improvements if you have some. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] netdev: Incremental BSD fix for netdev refactoriong
This is an incremental to patch v3 of
netdev: Factor restoring flags into new "struct netdev_saved_flags",
implementing the equivalent change for netdev-bsd.
Signed-off-by: Ed Maste
---
With this, the first patch now passes unit tests for me on FreeBSD.
lib/netdev-bsd.c | 43 +++
1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
index 7ab9d3e..8b384ba 100644
--- a/lib/netdev-bsd.c
+++ b/lib/netdev-bsd.c
@@ -133,11 +133,11 @@ static int cache_notifier_refcount;
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
-static int netdev_bsd_do_ioctl(const struct netdev *, struct ifreq *,
- unsigned long cmd, const char *cmd_name);
+static int netdev_bsd_do_ioctl(const char *, struct ifreq *, unsigned long cmd,
+ const char *cmd_name);
static void destroy_tap(int fd, const char *name);
-static int get_flags(const struct netdev *, int *flagsp);
-static int set_flags(struct netdev *, int flags);
+static int get_flags(const struct netdev_dev *, int *flagsp);
+static int set_flags(const char *, int flags);
static int do_set_addr(struct netdev *netdev,
int ioctl_nr, const char *ioctl_name,
struct in_addr addr);
@@ -813,7 +813,8 @@ netdev_bsd_get_mtu(const struct netdev *netdev_, int *mtup)
struct ifreq ifr;
int error;
-error = netdev_bsd_do_ioctl(netdev_, &ifr, SIOCGIFMTU, "SIOCGIFMTU");
+error = netdev_bsd_do_ioctl(netdev_get_name(netdev_), &ifr, SIOCGIFMTU,
+"SIOCGIFMTU");
if (error) {
return error;
}
@@ -1089,7 +1090,7 @@ netdev_bsd_get_in4(const struct netdev *netdev_, struct
in_addr *in4,
int error;
ifr.ifr_addr.sa_family = AF_INET;
-error = netdev_bsd_do_ioctl(netdev_, &ifr,
+error = netdev_bsd_do_ioctl(netdev_get_name(netdev_), &ifr,
SIOCGIFADDR, "SIOCGIFADDR");
if (error) {
return error;
@@ -1098,7 +1099,7 @@ netdev_bsd_get_in4(const struct netdev *netdev_, struct
in_addr *in4,
sin = (struct sockaddr_in *) &ifr.ifr_addr;
netdev_dev->in4 = sin->sin_addr;
netdev_dev->cache_valid |= VALID_IN4;
-error = netdev_bsd_do_ioctl(netdev_, &ifr,
+error = netdev_bsd_do_ioctl(netdev_get_name(netdev_), &ifr,
SIOCGIFNETMASK, "SIOCGIFNETMASK");
if (error) {
return error;
@@ -1221,19 +1222,21 @@ iff_to_nd_flags(int iff)
}
static int
-netdev_bsd_update_flags(struct netdev *netdev, enum netdev_flags off,
+netdev_bsd_update_flags(struct netdev_dev *dev_, enum netdev_flags off,
enum netdev_flags on, enum netdev_flags *old_flagsp)
{
+struct netdev_dev_bsd *netdev_dev;
int old_flags, new_flags;
int error;
-error = get_flags(netdev, &old_flags);
+netdev_dev = netdev_dev_bsd_cast(dev_);
+error = get_flags(dev_, &old_flags);
if (!error) {
*old_flagsp = iff_to_nd_flags(old_flags);
new_flags = (old_flags & ~nd_to_iff_flags(off)) | nd_to_iff_flags(on);
if (new_flags != old_flags) {
-error = set_flags(netdev, new_flags);
-
netdev_dev_bsd_changed(netdev_dev_bsd_cast(netdev_get_dev(netdev)));
+error = set_flags(netdev_dev_get_name(dev_), new_flags);
+netdev_dev_bsd_changed(netdev_dev);
}
}
return error;
@@ -1381,12 +1384,12 @@ destroy_tap(int fd, const char *name)
}
static int
-get_flags(const struct netdev *netdev, int *flags)
+get_flags(const struct netdev_dev *dev, int *flags)
{
struct ifreq ifr;
int error;
-error = netdev_bsd_do_ioctl(netdev, &ifr, SIOCGIFFLAGS, "SIOCGIFFLAGS");
+error = netdev_bsd_do_ioctl(dev->name, &ifr, SIOCGIFFLAGS, "SIOCGIFFLAGS");
*flags = 0x & (ifr.ifr_flagshigh << 16);
*flags |= 0x & ifr.ifr_flags;
@@ -1395,14 +1398,14 @@ get_flags(const struct netdev *netdev, int *flags)
}
static int
-set_flags(struct netdev *netdev, int flags)
+set_flags(const char *name, int flags)
{
struct ifreq ifr;
ifr.ifr_flags = 0x & flags;
ifr.ifr_flagshigh = (0x & flags) >> 16;
-return netdev_bsd_do_ioctl(netdev, &ifr, SIOCSIFFLAGS, "SIOCSIFFLAGS");
+return netdev_bsd_do_ioctl(name, &ifr, SIOCSIFFLAGS, "SIOCSIFFLAGS");
}
static int
@@ -1474,13 +1477,13 @@ set_etheraddr(const char *netdev_name, int
hwaddr_family,
}
static int
-netdev_bsd_do_ioctl(const struct netdev *netdev, struct ifreq *ifr,
-unsigned long cmd, const char *cmd_name)
+netdev_bsd_do_ioctl(const char *name, struct ifreq *ifr, unsigned long cmd,
+const char *cmd_name)
{
-strncpy(ifr->ifr_name, netdev_get_name(netdev), sizeof ifr->ifr_name);
+s
Re: [ovs-dev] [PATCH] netdev: Incremental BSD fix for netdev refactoriong
On Fri, May 10, 2013 at 02:15:50PM -0400, Ed Maste wrote: > This is an incremental to patch v3 of > netdev: Factor restoring flags into new "struct netdev_saved_flags", > implementing the equivalent change for netdev-bsd. > > Signed-off-by: Ed Maste > --- > With this, the first patch now passes unit tests for me on FreeBSD. Great, I folded that in and applied this patch to master. There were merge conflicts in netdev-bsd when I continued to rebase through the rest of the series, and I am not confident that I resolved them correctly (I did not try a build on FreeBSD), so I will repost the rest for your review. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [netdev v5 1/3] netdev: Add new "struct netdev_rx" for capturing packets from a netdev.
Separating packet capture from "struct netdev" means that there is no
remaining per-"struct netdev" state, which will allow us to get rid of
"struct netdev_dev" (by renaming it to "struct netdev").
Signed-off-by: Ben Pfaff
---
lib/dpif-netdev.c | 13 ++-
lib/netdev-bsd.c | 309 ++---
lib/netdev-dummy.c| 95 +--
lib/netdev-linux.c| 216 ++
lib/netdev-provider.h | 93 ---
lib/netdev-vport.c|5 +-
lib/netdev.c | 106 +
lib/netdev.h | 21 ++--
8 files changed, 469 insertions(+), 389 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 40f59c3..78bdedb 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -107,6 +107,7 @@ struct dp_netdev_port {
struct list node; /* Element in dp_netdev's 'port_list'. */
struct netdev *netdev;
struct netdev_saved_flags *sf;
+struct netdev_rx *rx;
char *type; /* Port type as requested by user. */
};
@@ -378,6 +379,7 @@ do_add_port(struct dp_netdev *dp, const char *devname,
const char *type,
struct netdev_saved_flags *sf;
struct dp_netdev_port *port;
struct netdev *netdev;
+struct netdev_rx *rx;
const char *open_type;
int mtu;
int error;
@@ -393,7 +395,7 @@ do_add_port(struct dp_netdev *dp, const char *devname,
const char *type,
/* XXX reject loopback devices */
/* XXX reject non-Ethernet devices */
-error = netdev_listen(netdev);
+error = netdev_rx_open(netdev, &rx);
if (error
&& !(error == EOPNOTSUPP && dpif_netdev_class_is_dummy(dp->class))) {
VLOG_ERR("%s: cannot receive packets on this network device (%s)",
@@ -404,6 +406,7 @@ do_add_port(struct dp_netdev *dp, const char *devname,
const char *type,
error = netdev_turn_flags_on(netdev, NETDEV_PROMISC, &sf);
if (error) {
+netdev_rx_close(rx);
netdev_close(netdev);
return error;
}
@@ -412,6 +415,7 @@ do_add_port(struct dp_netdev *dp, const char *devname,
const char *type,
port->port_no = port_no;
port->netdev = netdev;
port->sf = sf;
+port->rx = rx;
port->type = xstrdup(type);
error = netdev_get_mtu(netdev, &mtu);
@@ -509,6 +513,7 @@ do_del_port(struct dp_netdev *dp, uint32_t port_no)
netdev_close(port->netdev);
netdev_restore_flags(port->sf);
+netdev_rx_close(port->rx);
free(port->type);
free(port);
@@ -1063,7 +1068,7 @@ dpif_netdev_run(struct dpif *dpif)
ofpbuf_clear(&packet);
ofpbuf_reserve(&packet, DP_NETDEV_HEADROOM);
-error = netdev_recv(port->netdev, &packet);
+error = port->rx ? netdev_rx_recv(port->rx, &packet) : EOPNOTSUPP;
if (!error) {
dp_netdev_port_input(dp, port, &packet);
} else if (error != EAGAIN && error != EOPNOTSUPP) {
@@ -1083,7 +1088,9 @@ dpif_netdev_wait(struct dpif *dpif)
struct dp_netdev_port *port;
LIST_FOR_EACH (port, node, &dp->port_list) {
-netdev_recv_wait(port->netdev);
+if (port->rx) {
+netdev_rx_wait(port->rx);
+}
}
}
diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
index 8b384ba..2e870d4 100644
--- a/lib/netdev-bsd.c
+++ b/lib/netdev-bsd.c
@@ -16,6 +16,7 @@
#include
+#include "netdev-provider.h"
#include
#include
#include
@@ -41,7 +42,6 @@
#include "coverage.h"
#include "dynamic-string.h"
#include "fatal-signal.h"
-#include "netdev-provider.h"
#include "ofpbuf.h"
#include "openflow/openflow.h"
#include "packets.h"
@@ -57,7 +57,7 @@ VLOG_DEFINE_THIS_MODULE(netdev_bsd);
/*
* This file implements objects to access interfaces.
- * Externally, interfaces are represented by two structures:
+ * Externally, interfaces are represented by three structures:
* + struct netdev_dev, representing a network device,
* containing e.g. name and a refcount;
* We can have private variables by embedding the
@@ -66,26 +66,36 @@ VLOG_DEFINE_THIS_MODULE(netdev_bsd);
*
* + struct netdev, representing an instance of an open netdev_dev.
* The structure contains a pointer to the 'struct netdev'
- * representing the device. Again, private information
- * such as file descriptor etc. are stored in our
- * own struct netdev_bsd which includes a struct netdev.
+ * representing the device.
+ *
+ * + struct netdev_rx, which represents a netdev open to capture received
+ * packets. Again, private information such as file descriptor etc. are
+ * stored in our own struct netdev_rx_bsd which includes a struct
+ * netdev_rx.
*
- * Both 'struct netdev' and 'struct netdev_dev' are referenced
- * in containers which hold pointers to the data structures.
- * We can reach our own struct netdev_XXX_bsd by putting a
- * struct netdev_XXX within our own struct, and using CONTAINER_OF
- * to access the parent struc
[ovs-dev] [netdev v5 2/3] Rename superclass members to 'up'.
---
lib/netdev-bsd.c | 24
lib/netdev-dummy.c | 18 +-
lib/netdev-linux.c | 38 +++---
lib/netdev-vport.c |8
4 files changed, 44 insertions(+), 44 deletions(-)
diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
index 2e870d4..36cacdd 100644
--- a/lib/netdev-bsd.c
+++ b/lib/netdev-bsd.c
@@ -79,7 +79,7 @@ VLOG_DEFINE_THIS_MODULE(netdev_bsd);
* struct, and using CONTAINER_OF to access the parent structure.
*/
struct netdev_bsd {
-struct netdev netdev;
+struct netdev up;
};
struct netdev_rx_bsd {
@@ -97,7 +97,7 @@ struct netdev_rx_bsd {
static const struct netdev_rx_class netdev_rx_bsd_class;
struct netdev_dev_bsd {
-struct netdev_dev netdev_dev;
+struct netdev_dev up;
unsigned int cache_valid;
unsigned int change_seq;
@@ -172,14 +172,14 @@ netdev_bsd_cast(const struct netdev *netdev)
{
ovs_assert(is_netdev_bsd_class(netdev_dev_get_class(
netdev_get_dev(netdev;
-return CONTAINER_OF(netdev, struct netdev_bsd, netdev);
+return CONTAINER_OF(netdev, struct netdev_bsd, up);
}
static struct netdev_dev_bsd *
netdev_dev_bsd_cast(const struct netdev_dev *netdev_dev)
{
ovs_assert(is_netdev_bsd_class(netdev_dev_get_class(netdev_dev)));
-return CONTAINER_OF(netdev_dev, struct netdev_dev_bsd, netdev_dev);
+return CONTAINER_OF(netdev_dev, struct netdev_dev_bsd, up);
}
static struct netdev_rx_bsd *
@@ -318,9 +318,9 @@ netdev_bsd_create_system(const struct netdev_class *class,
const char *name,
netdev_dev = xzalloc(sizeof *netdev_dev);
netdev_dev->change_seq = 1;
-netdev_dev_init(&netdev_dev->netdev_dev, name, class);
+netdev_dev_init(&netdev_dev->up, name, class);
netdev_dev->tap_fd = -1;
-*netdev_devp = &netdev_dev->netdev_dev;
+*netdev_devp = &netdev_dev->up;
return 0;
}
@@ -390,8 +390,8 @@ netdev_bsd_create_tap(const struct netdev_class *class,
const char *name,
/* initialize the device structure and
* link the structure to its netdev */
-netdev_dev_init(&netdev_dev->netdev_dev, name, class);
-*netdev_devp = &netdev_dev->netdev_dev;
+netdev_dev_init(&netdev_dev->up, name, class);
+*netdev_devp = &netdev_dev->up;
return 0;
@@ -428,19 +428,19 @@ netdev_bsd_open_system(struct netdev_dev *netdev_dev_,
struct netdev **netdevp)
/* Allocate network device. */
netdev = xcalloc(1, sizeof *netdev);
-netdev_init(&netdev->netdev, netdev_dev_);
+netdev_init(&netdev->up, netdev_dev_);
/* Verify that the netdev really exists by attempting to read its flags */
-error = netdev_get_flags(&netdev->netdev, &flags);
+error = netdev_get_flags(&netdev->up, &flags);
if (error == ENXIO) {
goto error;
}
-*netdevp = &netdev->netdev;
+*netdevp = &netdev->up;
return 0;
error:
-netdev_uninit(&netdev->netdev, true);
+netdev_uninit(&netdev->up, true);
return error;
}
diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 3762d5c..dabff18 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -43,7 +43,7 @@ VLOG_DEFINE_THIS_MODULE(netdev_dummy);
#endif
struct netdev_dev_dummy {
-struct netdev_dev netdev_dev;
+struct netdev_dev up;
uint8_t hwaddr[ETH_ADDR_LEN];
int mtu;
struct netdev_stats stats;
@@ -55,7 +55,7 @@ struct netdev_dev_dummy {
};
struct netdev_dummy {
-struct netdev netdev;
+struct netdev up;
};
struct netdev_rx_dummy {
@@ -87,7 +87,7 @@ static struct netdev_dev_dummy *
netdev_dev_dummy_cast(const struct netdev_dev *netdev_dev)
{
ovs_assert(is_dummy_class(netdev_dev_get_class(netdev_dev)));
-return CONTAINER_OF(netdev_dev, struct netdev_dev_dummy, netdev_dev);
+return CONTAINER_OF(netdev_dev, struct netdev_dev_dummy, up);
}
static struct netdev_dummy *
@@ -95,7 +95,7 @@ netdev_dummy_cast(const struct netdev *netdev)
{
struct netdev_dev *netdev_dev = netdev_get_dev(netdev);
ovs_assert(is_dummy_class(netdev_dev_get_class(netdev_dev)));
-return CONTAINER_OF(netdev, struct netdev_dummy, netdev);
+return CONTAINER_OF(netdev, struct netdev_dummy, up);
}
static struct netdev_rx_dummy *
@@ -113,7 +113,7 @@ netdev_dummy_create(const struct netdev_class *class, const
char *name,
struct netdev_dev_dummy *netdev_dev;
netdev_dev = xzalloc(sizeof *netdev_dev);
-netdev_dev_init(&netdev_dev->netdev_dev, name, class);
+netdev_dev_init(&netdev_dev->up, name, class);
netdev_dev->hwaddr[0] = 0xaa;
netdev_dev->hwaddr[1] = 0x55;
netdev_dev->hwaddr[2] = n >> 24;
@@ -130,7 +130,7 @@ netdev_dummy_create(const struct netdev_class *class, const
char *name,
n++;
-*netdev_devp = &netdev_dev->netdev_dev;
+*netdev_devp = &netdev_dev->up;
return 0;
}
@@ -172,9 +172,9 @@ netdev_dummy_open(struct netdev_dev *netdev_dev_, struct
netdev **netde
[ovs-dev] [netdev v5 0/3] refactor netdev code to make more sense
Changes v1->v2: - Initial two patches dropped because they have been applied. - Fixed merge conflicts against current master. Changes v2->v3: - Fixed merge conflicts against current master. Changes v3->v4: - Fix use-after-free in patch 1 reported by Ed Maste. Changes v4->v5: - First patch merged and dropped from series. - Merge conflicts resolved on later patches, but I'm suspicious. Ben Pfaff (3): netdev: Add new "struct netdev_rx" for capturing packets from a netdev. Rename superclass members to 'up'. netdev: Get rid of netdev_dev. lib/dpif-netdev.c | 13 +- lib/netdev-bsd.c | 550 + lib/netdev-dummy.c| 270 +++ lib/netdev-linux.c| 924 +++-- lib/netdev-provider.h | 172 - lib/netdev-vport.c| 147 lib/netdev.c | 421 ++ lib/netdev.h | 21 +- 8 files changed, 1121 insertions(+), 1397 deletions(-) -- 1.7.10.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] meta-flow: Simplify mf_from_ofp_port_string()
ofputil_port_from_string() does all the work already.
Signed-off-by: Jarno Rajahalme
---
lib/meta-flow.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index a75e526..e5809d5 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -2166,17 +2166,13 @@ mf_from_ofp_port_string(const struct mf_field *mf,
const char *s,
uint16_t port;
ovs_assert(mf->n_bytes == sizeof(ovs_be16));
-if (*s == '-') {
-return xasprintf("%s: negative values not supported for %s",
- s, mf->name);
-} else if (ofputil_port_from_string(s, &port)) {
+
+if (ofputil_port_from_string(s, &port)) {
*valuep = htons(port);
*maskp = htons(UINT16_MAX);
return NULL;
-} else {
-return mf_from_integer_string(mf, s,
- (uint8_t *) valuep, (uint8_t *) maskp);
}
+return xasprintf("%s: port value out of range for %s", s, mf->name);
}
struct frag_handling {
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [RFC PATCH 2/2] meta-flow: Add MFF_IN_PORT_OXM, a 32-bit in_port.
This helps get rid of one special case in nx_pull_raw() and allows
loading of 32-bit values to OXM_OF_IN_PORT in NXAST_LEARN actions.
Previously the 16-bit limit acted the same on both NXM_OF_IN_PORT and
OXM_OF_IN_PORT, even though OF1.1+ controllers would expect OXM_OF_IN_PORT
to be 32 bits wide.
Signed-off-by: Jarno Rajahalme
---
I considered simply making the MFF_IN_PORT 32-bit instead, but that could
have resulted in surprises for any existing use expecting it to be 16 bits
(such as register load actions). Maybe there is no such conflict, and indeed
it would be better to just widen the MFF_IN_PORT and add a special case for
NXM_OF_IN_PORT into nx_pull_raw().
lib/meta-flow.c | 69 +++
lib/meta-flow.h |2 ++
lib/nx-match.c | 13 +--
tests/learn.at | 22 ++
4 files changed, 94 insertions(+), 12 deletions(-)
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index e5809d5..f8f02ed 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -116,6 +116,15 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
MFP_NONE,
true,
NXM_OF_IN_PORT, "NXM_OF_IN_PORT",
+NXM_OF_IN_PORT, "NXM_OF_IN_PORT",
+}, {
+MFF_IN_PORT_OXM, "in_port_oxm", NULL,
+MF_FIELD_SIZES(be32),
+MFM_NONE,
+MFS_OFP_PORT_OXM,
+MFP_NONE,
+true,
+OXM_OF_IN_PORT, "OXM_OF_IN_PORT",
OXM_OF_IN_PORT, "OXM_OF_IN_PORT",
}, {
MFF_SKB_PRIORITY, "skb_priority", NULL,
@@ -686,6 +695,7 @@ mf_is_all_wild(const struct mf_field *mf, const struct
flow_wildcards *wc)
case MFF_METADATA:
return !wc->masks.metadata;
case MFF_IN_PORT:
+case MFF_IN_PORT_OXM:
return !wc->masks.in_port;
case MFF_SKB_PRIORITY:
return !wc->masks.skb_priority;
@@ -924,6 +934,11 @@ mf_is_value_valid(const struct mf_field *mf, const union
mf_value *value)
case MFF_ND_TLL:
return true;
+case MFF_IN_PORT_OXM: {
+uint32_t port = ntohl(value->be32);
+return port < OFPP_MAX || port >= OFPP11_MAX;
+}
+
case MFF_IP_DSCP:
return !(value->u8 & ~IP_DSCP_MASK);
case MFF_IP_DSCP_SHIFTED:
@@ -997,6 +1012,10 @@ mf_get_value(const struct mf_field *mf, const struct flow
*flow,
value->be16 = htons(flow->in_port);
break;
+case MFF_IN_PORT_OXM:
+value->be32 = ofputil_port_to_ofp11(flow->in_port);
+break;
+
case MFF_SKB_PRIORITY:
value->be32 = htonl(flow->skb_priority);
break;
@@ -1145,6 +1164,16 @@ mf_get_value(const struct mf_field *mf, const struct
flow *flow,
}
}
+/* Map a 32 port number to 16-bit range. */
+static inline uint16_t
+port_from_ofp11(ovs_be32 ofp11_port)
+{
+ uint32_t port = ntohl(ofp11_port);
+ return port < OFPP_MAX ? port
+: port >= OFPP11_MAX ? port - OFPP11_OFFSET
+: OFPP_NONE;
+}
+
/* Makes 'match' match field 'mf' exactly, with the value matched taken from
* 'value'. The caller is responsible for ensuring that 'match' meets 'mf''s
* prerequisites. */
@@ -1180,6 +1209,10 @@ mf_set_value(const struct mf_field *mf,
match_set_in_port(match, ntohs(value->be16));
break;
+case MFF_IN_PORT_OXM:
+match_set_in_port(match, port_from_ofp11(value->be32));
+break;
+
case MFF_SKB_PRIORITY:
match_set_skb_priority(match, ntohl(value->be32));
break;
@@ -1363,6 +1396,10 @@ mf_set_flow_value(const struct mf_field *mf,
flow->in_port = ntohs(value->be16);
break;
+case MFF_IN_PORT_OXM:
+flow->in_port = port_from_ofp11(value->be32);
+break;
+
case MFF_SKB_PRIORITY:
flow->skb_priority = ntohl(value->be32);
break;
@@ -1559,6 +1596,7 @@ mf_set_wild(const struct mf_field *mf, struct match
*match)
break;
case MFF_IN_PORT:
+case MFF_IN_PORT_OXM:
match->flow.in_port = 0;
match->wc.masks.in_port = 0;
break;
@@ -1740,6 +1778,7 @@ mf_set(const struct mf_field *mf,
switch (mf->id) {
case MFF_IN_PORT:
+case MFF_IN_PORT_OXM:
case MFF_SKB_MARK:
case MFF_SKB_PRIORITY:
case MFF_ETH_TYPE:
@@ -1975,6 +2014,12 @@ mf_random_value(const struct mf_field *mf, union
mf_value *value)
case MFF_ND_TLL:
break;
+case MFF_IN_PORT_OXM:
+value->be32 &= htonl(0x);
+ if (ntohl(value->be32) > OFPP_MAX)
+ value->be32 |= htonl(0x);
+break;
+
case MFF_IPV6_LABEL:
value->be32 &= ~htonl(IPV6_LABEL_MASK);
break;
@@ -2175,6 +2220,21 @@ mf_from_ofp_port_string(const struct mf_field *mf, const
char *s,
return xasprintf("%s: port value out of range for %s", s, mf->name);
}
+static char *
+mf_from_ofp_port_string32(const struct mf_field *mf, const char *s,
+ ovs_be32 *valuep, ovs_be32 *maskp)
+{
+uint16_t port;
+
+ovs_assert(mf->n_by
Re: [ovs-dev] [netdev v3 1/4] netdev: Factor restoring flags into new "struct netdev_saved_flags".
The indentation is newly incorrect in the calls to netdev_bsd_do_ioctl(), the prototype for netdev_bsd_update_flags(), and the prototype for netdev_vport_update_flags(). In netdev_linux_set_etheraddr(), Do we still need to check if 'netdev_' is NETDEV_UP before turning it off, or can we skip the check and always call netdev_turn_flags_off()? It's clear how restore_all_flags() is called in the case that ovs-vswitchd dies due to a fatal signa. However, it's not obvious to me that it's called when ovs-vswitchd exists gracefully (i.e. due to ovs-appctl exit). Perhaps I'm missing it? Out of curiosity, is it that important that we even have this flag restoring logic? Is it a big deal if ovs leaves devices in promiscuous mode when it dies? Acked-by: Ethan Jackson On Fri, May 10, 2013 at 10:26 AM, Ben Pfaff wrote: > On Fri, May 10, 2013 at 01:21:11PM -0400, Ed Maste wrote: >> On 10 May 2013 13:12, Ed Maste wrote: >> > Ahh, of course. This fixes the segfaulting tests for me, but the unit >> > tests now get stuck in "tunnel - input." >> >> Oh, and of course the reason ovs-vsctl is stuck is that ovs-vswitchd >> segfaulted. >> >> #0 0x00080164b560 in strncpy () from /lib/libc.so.7 >> #1 0x0049ee29 in netdev_bsd_do_ioctl (netdev=0x801c231d0, >> ifr=0x7fffc9a0, cmd=3223349521, cmd_name=0x4d3303 "SIOCGIFFLAGS") >> at lib/netdev-bsd.c:1480 >> #2 0x0049eef2 in get_flags (flags=, >> netdev=) at lib/netdev-bsd.c:1389 >> #3 netdev_bsd_update_flags (netdev=0x7fffc9a0, off=0, on=0, >> old_flagsp=0x7fffca24) at lib/netdev-bsd.c:1230 >> #4 0x0044f527 in do_update_flags (netdev=0x801c22310, off=0, >> on=0, old_flagsp=0x7fffca6c, sfp=0x0) at lib/netdev.c:816 >> #5 0x004a03ea in netdev_bsd_open_system >> (netdev_dev_=0x801c231d0, netdevp=0x7fffcad8) at >> lib/netdev-bsd.c:412 >> #6 0x0044fa19 in netdev_open (name=0x70e2d0 "alc0", >> type=0x4a9d1f "system", netdevp=0x7fffcad8) at lib/netdev.c:241 >> #7 0x0044d366 in tunnel_get_status (netdev=, >> smap=0x7fffcb20) at lib/netdev-vport.c:226 >> #8 0x00406bf9 in iface_refresh_status (iface=0x801c25220) at >> vswitchd/bridge.c:1799 >> #9 0x00408f3b in iface_create (br=0x801cbf100, >> if_cfg=0x801c221c0, ofp_port=3) at vswitchd/bridge.c:1520 >> #10 0x00409335 in bridge_reconfigure_ofp () at vswitchd/bridge.c:568 >> #11 0x0040941e in bridge_reconfigure_continue >> (ovs_cfg=0x801c1d280) at vswitchd/bridge.c:589 >> #12 0x0040c85e in bridge_run () at vswitchd/bridge.c:2374 >> #13 0x0040dcfa in main (argc=, argv=> out>) at vswitchd/ovs-vswitchd.c:125 >> (gdb) frame 0 >> >> I'm taking a look at it, just wanted to reply because my previous >> email is clearly lacking sufficient detail. > > Thanks for looking into it. I did compile the FreeBSD version of this > patch series at some point in development but obviously I didn't focus > on it. > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] netdev: Incremental BSD fix for netdev refactoriong
On 10 May 2013 14:38, Ben Pfaff wrote: > On Fri, May 10, 2013 at 02:15:50PM -0400, Ed Maste wrote: >> This is an incremental to patch v3 of >> netdev: Factor restoring flags into new "struct netdev_saved_flags", >> implementing the equivalent change for netdev-bsd. >> >> Signed-off-by: Ed Maste >> --- >> With this, the first patch now passes unit tests for me on FreeBSD. > > Great, I folded that in and applied this patch to master. Thanks. I find it a little ironic that the trouble here was caused by the netdev vs. netdev_dev issue itself. This also explains why the end result is almost the same (when netdev_dev is removed in patch 4). > There were merge conflicts in netdev-bsd when I continued to rebase > through the rest of the series, and I am not confident that I resolved > them correctly (I did not try a build on FreeBSD), so I will repost the > rest for your review. The conflict resolutions match what I ended up with when I reapplied patches 2-4 on top of my incremental, at least. All unit tests now pass on FreeBSD for patches 2-4 in v4; I haven't tried an actual system test. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] tests/ofproto-dpif.at: Add tests for the ofproto/trace command
Two testcases are added to the testsuite, which test the new
command syntax and the corresponding corner cases.
Signed-off-by: Alex Wang
---
tests/ofproto-dpif.at | 289 +
1 file changed, 289 insertions(+)
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 6a46ddb..4bed100 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -1044,6 +1044,295 @@ AT_CHECK([ovs-dpctl normalize-actions "$flow"
"$actual"], [0], [expout])
OVS_VSWITCHD_STOP
AT_CLEANUP
+# Two testcases below for the ofproto/trace command
+# The first one tests all correct syntax:
+# ofproto/trace [dp_name] odp_flow [-generate|packet]
+# ofproto/trace br_name br_flow [-generate|packet]
+AT_SETUP([ofproto-dpif - ofproto/trace command 1])
+OVS_VSWITCHD_START([set bridge br0 fail-mode=standalone])
+ADD_OF_PORTS([br0], 1, 2, 3)
+
+AT_DATA([flows.txt], [dnl
+in_port=1 actions=output:2
+in_port=2 actions=output:1
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+odp_flow="in_port(1)"
+br_flow="in_port=1"
+# Test command: ofproto/trace odp_flow
+AT_CHECK([ovs-appctl ofproto/trace "$odp_flow"], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0], [dnl
+Datapath actions: 2
+])
+
+# Test command: ofproto/trace dp_name odp_flow
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "$odp_flow"], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0], [dnl
+Datapath actions: 2
+])
+# Test commmand: ofproto/trace br_name br_flow
+AT_CHECK([ovs-appctl ofproto/trace br0 "$br_flow"], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0], [dnl
+Datapath actions: 2
+])
+
+# Delete the inserted flows
+AT_CHECK([ovs-ofctl del-flows br0 "in_port=1"], [0], [stdout])
+AT_CHECK([ovs-ofctl del-flows br0 "in_port=2"], [0], [stdout])
+
+# This section beflow tests the [-generate] option
+odp_flow="in_port(3),eth(src=50:54:00:00:00:05,dst=ff:ff:ff:ff:ff:ff)"
+br_flow="arp,metadata=0,in_port=3,vlan_tci=0x,dl_src=50:54:00:00:00:05,dl_dst=ff:ff:ff:ff:ff:ff"
+
+# Test command: ofproto/trace odp_flow
+AT_CHECK([ovs-appctl ofproto/trace "$odp_flow"], [0], [stdout])
+# Check for no MAC learning entry.
+AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], [0],
[dnl
+ port VLAN MACAge
+])
+
+# Test command: ofproto/trace br_name br_flow
+AT_CHECK([ovs-appctl ofproto/trace br0 "$br_flow"], [0], [stdout])
+# Check for no MAC learning entry.
+AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], [0],
[dnl
+ port VLAN MACAge
+])
+
+# Test command: ofproto/trace odp_flow -generate
+AT_CHECK([ovs-appctl ofproto/trace "$odp_flow" -generate], [0], [stdout])
+# Check for the MAC learning entry.
+AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], [0],
[dnl
+ port VLAN MACAge
+3 0 50:54:00:00:00:05?
+])
+
+# Test command: ofproto/trace dp_name odp_flow -generate
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy \
+ "in_port(1),eth(src=50:54:00:00:00:06,dst=50:54:00:00:00:05)" \
+ -generate], [0], [stdout])
+# Check for both MAC learning entries.
+AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], [0],
[dnl
+ port VLAN MACAge
+3 0 50:54:00:00:00:05?
+1 0 50:54:00:00:00:06?
+])
+
+# Test command: ofproto/trace br_name br_flow -generate
+AT_CHECK([ovs-appctl ofproto/trace br0 \
+ "in_port=2,dl_src=50:54:00:00:00:07,dl_dst=50:54:00:00:00:06" \
+ -generate], [0], [stdout])
+# Check for both MAC learning entries.
+AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], [0],
[dnl
+ port VLAN MACAge
+3 0 50:54:00:00:00:05?
+1 0 50:54:00:00:00:06?
+2 0 50:54:00:00:00:07?
+])
+
+# This section beflow tests the [packet] option
+# The ovs-tcpundump of packets between port1 and port2
+pkt1to2="50540002505400010806451C00014001F98CC0A80001C0A800020800F7FF"
+pkt2to1="50540001505400020806451C00014001F98CC0A80002C0A800010800F7FF"
+
+# Construct the MAC learning table
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy \
+ "in_port(1),eth(src=50:54:00:00:00:01,dst=ff:ff:ff:ff:ff:ff)" \
+ -generate], [0], [stdout])
+
+# Construct the MAC learning table
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy \
+ "in_port(2),eth(src=50:54:00:00:00:02,dst=ff:ff:ff:ff:ff:ff)" \
+ -generate], [0], [stdout])
+
+# Test command: ofproto/trace odp_flow packet
+AT_CHECK([ovs-appctl ofproto/trace \
+ "in_port(1),skb_priority(1),skb_mark(2)" "$pkt1to2"], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0], [dnl
+Datapath actions: 2
+])
+AT_CHECK([head -n 3 stdout], [0], [dnl
+Bridge: br0
+Packet:
arp,metadata=0,in_port=0,vlan_tci=0x,dl_src=50:54:00:00:00:01,dl_dst=50:54:00:00:00:02,arp_spa=0.0.0.0,arp_tpa=0.0.0.0,arp_sha=00:00:00:00:00:00,arp_tha=00:00:00:00:00:00
+Flow:
skb_mark=0x2,skb_priority=0x1,arp,metadata=0,in_port=1,vlan_tci=0x,dl_src=50:54:00:0
[ovs-dev] [PATCH] netdev-bsd: Adjust argument line wrapping
This file presumably started out life as a copy of netdev-linux.c, and
some indentation was not updated after s/linux/bsd/.
Signed-off-by: Ed Maste
---
There are still a few cases that seem not to match style; I adjusted these
because Ethan noticed them in commit 4b60911.
lib/netdev-bsd.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
index ea0adb1..3ddd1d5 100644
--- a/lib/netdev-bsd.c
+++ b/lib/netdev-bsd.c
@@ -221,7 +221,7 @@ netdev_dev_bsd_changed(struct netdev_dev_bsd *dev)
/* Invalidate cache in case of interface status change. */
static void
netdev_bsd_cache_cb(const struct rtbsd_change *change,
- void *aux OVS_UNUSED)
+void *aux OVS_UNUSED)
{
struct netdev_dev_bsd *dev;
@@ -836,7 +836,7 @@ netdev_bsd_send_wait(struct netdev *netdev_)
*/
static int
netdev_bsd_set_etheraddr(struct netdev *netdev_,
- const uint8_t mac[ETH_ADDR_LEN])
+ const uint8_t mac[ETH_ADDR_LEN])
{
struct netdev_dev_bsd *netdev_dev =
netdev_dev_bsd_cast(netdev_get_dev(netdev_));
@@ -863,7 +863,7 @@ netdev_bsd_set_etheraddr(struct netdev *netdev_,
*/
static int
netdev_bsd_get_etheraddr(const struct netdev *netdev_,
- uint8_t mac[ETH_ADDR_LEN])
+ uint8_t mac[ETH_ADDR_LEN])
{
struct netdev_dev_bsd *netdev_dev =
netdev_dev_bsd_cast(netdev_get_dev(netdev_));
@@ -1095,8 +1095,8 @@ netdev_bsd_parse_media(int media)
*/
static int
netdev_bsd_get_features(const struct netdev *netdev,
- enum netdev_features *current, uint32_t *advertised,
- enum netdev_features *supported, uint32_t *peer)
+enum netdev_features *current, uint32_t *advertised,
+enum netdev_features *supported, uint32_t *peer)
{
struct ifmediareq ifmr;
int *media_list;
@@ -1174,7 +1174,7 @@ netdev_bsd_get_in4(const struct netdev *netdev_, struct
in_addr *in4,
ifr.ifr_addr.sa_family = AF_INET;
error = netdev_bsd_do_ioctl(netdev_get_name(netdev_), &ifr,
- SIOCGIFADDR, "SIOCGIFADDR");
+SIOCGIFADDR, "SIOCGIFADDR");
if (error) {
return error;
}
@@ -1183,7 +1183,7 @@ netdev_bsd_get_in4(const struct netdev *netdev_, struct
in_addr *in4,
netdev_dev->in4 = sin->sin_addr;
netdev_dev->cache_valid |= VALID_IN4;
error = netdev_bsd_do_ioctl(netdev_get_name(netdev_), &ifr,
- SIOCGIFNETMASK, "SIOCGIFNETMASK");
+SIOCGIFNETMASK, "SIOCGIFNETMASK");
if (error) {
return error;
}
@@ -1201,7 +1201,7 @@ netdev_bsd_get_in4(const struct netdev *netdev_, struct
in_addr *in4,
*/
static int
netdev_bsd_set_in4(struct netdev *netdev_, struct in_addr addr,
- struct in_addr mask)
+ struct in_addr mask)
{
struct netdev_dev_bsd *netdev_dev =
netdev_dev_bsd_cast(netdev_get_dev(netdev_));
@@ -1306,7 +1306,7 @@ iff_to_nd_flags(int iff)
static int
netdev_bsd_update_flags(struct netdev_dev *dev_, enum netdev_flags off,
- enum netdev_flags on, enum netdev_flags *old_flagsp)
+enum netdev_flags on, enum netdev_flags *old_flagsp)
{
struct netdev_dev_bsd *netdev_dev;
int old_flags, new_flags;
--
1.7.11.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] netdev-bsd: Adjust argument line wrapping
On Fri, May 10, 2013 at 04:23:03PM -0400, Ed Maste wrote: > This file presumably started out life as a copy of netdev-linux.c, and > some indentation was not updated after s/linux/bsd/. > > Signed-off-by: Ed Maste > --- > There are still a few cases that seem not to match style; I adjusted these > because Ethan noticed them in commit 4b60911. Applied, thanks! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [netdev v5 2/3] Rename superclass members to 'up'.
Acked-by: Ethan Jackson
On Fri, May 10, 2013 at 11:50 AM, Ben Pfaff wrote:
> ---
> lib/netdev-bsd.c | 24
> lib/netdev-dummy.c | 18 +-
> lib/netdev-linux.c | 38 +++---
> lib/netdev-vport.c |8
> 4 files changed, 44 insertions(+), 44 deletions(-)
>
> diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
> index 2e870d4..36cacdd 100644
> --- a/lib/netdev-bsd.c
> +++ b/lib/netdev-bsd.c
> @@ -79,7 +79,7 @@ VLOG_DEFINE_THIS_MODULE(netdev_bsd);
> * struct, and using CONTAINER_OF to access the parent structure.
> */
> struct netdev_bsd {
> -struct netdev netdev;
> +struct netdev up;
> };
>
> struct netdev_rx_bsd {
> @@ -97,7 +97,7 @@ struct netdev_rx_bsd {
> static const struct netdev_rx_class netdev_rx_bsd_class;
>
> struct netdev_dev_bsd {
> -struct netdev_dev netdev_dev;
> +struct netdev_dev up;
> unsigned int cache_valid;
> unsigned int change_seq;
>
> @@ -172,14 +172,14 @@ netdev_bsd_cast(const struct netdev *netdev)
> {
> ovs_assert(is_netdev_bsd_class(netdev_dev_get_class(
> netdev_get_dev(netdev;
> -return CONTAINER_OF(netdev, struct netdev_bsd, netdev);
> +return CONTAINER_OF(netdev, struct netdev_bsd, up);
> }
>
> static struct netdev_dev_bsd *
> netdev_dev_bsd_cast(const struct netdev_dev *netdev_dev)
> {
> ovs_assert(is_netdev_bsd_class(netdev_dev_get_class(netdev_dev)));
> -return CONTAINER_OF(netdev_dev, struct netdev_dev_bsd, netdev_dev);
> +return CONTAINER_OF(netdev_dev, struct netdev_dev_bsd, up);
> }
>
> static struct netdev_rx_bsd *
> @@ -318,9 +318,9 @@ netdev_bsd_create_system(const struct netdev_class
> *class, const char *name,
>
> netdev_dev = xzalloc(sizeof *netdev_dev);
> netdev_dev->change_seq = 1;
> -netdev_dev_init(&netdev_dev->netdev_dev, name, class);
> +netdev_dev_init(&netdev_dev->up, name, class);
> netdev_dev->tap_fd = -1;
> -*netdev_devp = &netdev_dev->netdev_dev;
> +*netdev_devp = &netdev_dev->up;
>
> return 0;
> }
> @@ -390,8 +390,8 @@ netdev_bsd_create_tap(const struct netdev_class *class,
> const char *name,
>
> /* initialize the device structure and
> * link the structure to its netdev */
> -netdev_dev_init(&netdev_dev->netdev_dev, name, class);
> -*netdev_devp = &netdev_dev->netdev_dev;
> +netdev_dev_init(&netdev_dev->up, name, class);
> +*netdev_devp = &netdev_dev->up;
>
> return 0;
>
> @@ -428,19 +428,19 @@ netdev_bsd_open_system(struct netdev_dev *netdev_dev_,
> struct netdev **netdevp)
>
> /* Allocate network device. */
> netdev = xcalloc(1, sizeof *netdev);
> -netdev_init(&netdev->netdev, netdev_dev_);
> +netdev_init(&netdev->up, netdev_dev_);
>
> /* Verify that the netdev really exists by attempting to read its flags
> */
> -error = netdev_get_flags(&netdev->netdev, &flags);
> +error = netdev_get_flags(&netdev->up, &flags);
> if (error == ENXIO) {
> goto error;
> }
>
> -*netdevp = &netdev->netdev;
> +*netdevp = &netdev->up;
> return 0;
>
> error:
> -netdev_uninit(&netdev->netdev, true);
> +netdev_uninit(&netdev->up, true);
> return error;
> }
>
> diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
> index 3762d5c..dabff18 100644
> --- a/lib/netdev-dummy.c
> +++ b/lib/netdev-dummy.c
> @@ -43,7 +43,7 @@ VLOG_DEFINE_THIS_MODULE(netdev_dummy);
> #endif
>
> struct netdev_dev_dummy {
> -struct netdev_dev netdev_dev;
> +struct netdev_dev up;
> uint8_t hwaddr[ETH_ADDR_LEN];
> int mtu;
> struct netdev_stats stats;
> @@ -55,7 +55,7 @@ struct netdev_dev_dummy {
> };
>
> struct netdev_dummy {
> -struct netdev netdev;
> +struct netdev up;
> };
>
> struct netdev_rx_dummy {
> @@ -87,7 +87,7 @@ static struct netdev_dev_dummy *
> netdev_dev_dummy_cast(const struct netdev_dev *netdev_dev)
> {
> ovs_assert(is_dummy_class(netdev_dev_get_class(netdev_dev)));
> -return CONTAINER_OF(netdev_dev, struct netdev_dev_dummy, netdev_dev);
> +return CONTAINER_OF(netdev_dev, struct netdev_dev_dummy, up);
> }
>
> static struct netdev_dummy *
> @@ -95,7 +95,7 @@ netdev_dummy_cast(const struct netdev *netdev)
> {
> struct netdev_dev *netdev_dev = netdev_get_dev(netdev);
> ovs_assert(is_dummy_class(netdev_dev_get_class(netdev_dev)));
> -return CONTAINER_OF(netdev, struct netdev_dummy, netdev);
> +return CONTAINER_OF(netdev, struct netdev_dummy, up);
> }
>
> static struct netdev_rx_dummy *
> @@ -113,7 +113,7 @@ netdev_dummy_create(const struct netdev_class *class,
> const char *name,
> struct netdev_dev_dummy *netdev_dev;
>
> netdev_dev = xzalloc(sizeof *netdev_dev);
> -netdev_dev_init(&netdev_dev->netdev_dev, name, class);
> +netdev_dev_init(&netdev_dev->up, name, class);
> netdev_dev->hwaddr[0] = 0xaa;
> netdev_dev->hwaddr[1] = 0x55;
> netd
[ovs-dev] [PATCH] netdev-linux: Skip NETDEV_UP test in netdev_linux_set_etheraddr() for taps.
netdev_turn_flags_off() does nothing if the flags that one turns off are
already off.
Reported-by: Ethan Jackson
Signed-off-by: Ben Pfaff
---
lib/netdev-linux.c |6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 30cd0f6..9e2708d 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1019,11 +1019,7 @@ netdev_linux_set_etheraddr(struct netdev *netdev_,
/* Tap devices must be brought down before setting the address. */
if (!strcmp(netdev_get_type(netdev_), "tap")) {
-enum netdev_flags flags;
-
-if (!netdev_get_flags(netdev_, &flags) && (flags & NETDEV_UP)) {
-netdev_turn_flags_off(netdev_, NETDEV_UP, &sf);
-}
+netdev_turn_flags_off(netdev_, NETDEV_UP, &sf);
}
error = set_etheraddr(netdev_get_name(netdev_), mac);
if (!error || error == ENODEV) {
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [netdev v3 1/4] netdev: Factor restoring flags into new "struct netdev_saved_flags".
I already applied this on the strength of Ed's review. However, you make good points, see below... On Fri, May 10, 2013 at 01:04:26PM -0700, Ethan Jackson wrote: > The indentation is newly incorrect in the calls to > netdev_bsd_do_ioctl(), the prototype for netdev_bsd_update_flags(), > and the prototype for netdev_vport_update_flags(). I applied Ed's patch to fix that up. > In netdev_linux_set_etheraddr(), Do we still need to check if > 'netdev_' is NETDEV_UP before turning it off, or can we skip the check > and always call netdev_turn_flags_off()? Doesn't look like the check is needed. I sent out a fix. > It's clear how restore_all_flags() is called in the case that > ovs-vswitchd dies due to a fatal signa. However, it's not obvious to > me that it's called when ovs-vswitchd exists gracefully (i.e. due to > ovs-appctl exit). Perhaps I'm missing it? Passing true for the run_at_exit argument to fatal_signal_add_hook() causes restore_all_flags() to get called from an atexit() handler installed by fatal_signal_init(). > Out of curiosity, is it that important that we even have this flag > restoring logic? Is it a big deal if ovs leaves devices in > promiscuous mode when it dies? Restoring flags is probably not necessary. It only seems polite to me, though, to try. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [netdev v3 1/4] netdev: Factor restoring flags into new "struct netdev_saved_flags".
> Restoring flags is probably not necessary. It only seems polite to me, > though, to try. Seems like an awful lot of trouble, but we've already got the code so it seems fine to me. Ethan ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] meta-flow: Simplify mf_from_ofp_port_string()
On Fri, May 10, 2013 at 10:54:35PM +0300, Jarno Rajahalme wrote: > ofputil_port_from_string() does all the work already. > > Signed-off-by: Jarno Rajahalme Applied, thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] ofproto: Re-implement the ofproto/trace command
On Fri, May 10, 2013 at 01:39:39PM -0700, Alex Wang wrote: > Since the use of single datapath, all bridges belonging to the same type of > datapath will use the same (single) datapath. This causes confusion in the > current 'ofproto/trace' command. Especially, when given the unrelated > 'bridge' and 'in_port' combination, the current implementation will still > be able to process and give misleading output. Thusly, this patch changes > the 'ofproto/trace' command syntax to formats shown as follow. > > ofproto/trace [datapath] odp_flow [-generate|packet] > ofproto/trace bridge br_flow [-generate|packet] > > Also, this patch updates the tests and the manpages accordingly. > > Signed-off-by: Alex Wang Thanks, Alex. I think that the argument parsing is more complicated than necessary. As a first step, we can detect whether the last argument is either -generate or a sequence of hex digits. After we've removed that argument, if present, then we can look at what is now the last argument. If it is an odp_flow, then we can consider the first optional "datapath" argument if present; otherwise we need to have a first "bridge" argument. I don't think it is a good idea to hard-code all the currently known datapath types (because then we have to maintain that list), but I don't think that it is necessary, either. I had in mine that the user would provide a datapath name (e.g. ovs-system) rather than a datapath type name (e.g. system). Your way is fine also, but in that case please update the documentation to make it clear. Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [netdev v5 1/3] netdev: Add new "struct netdev_rx" for capturing packets from a netdev.
On Fri, May 10, 2013 at 01:30:48PM -0700, Ethan Jackson wrote: > You may disagree, but I think 'renaming it "struct netdev"' sounds > better than 'renaming it top "struct netdev"' You mean, grammatically? OK, I changed the commit message. > In netdev_bsd_rx_open(), should we call netdev_dev_bsd_changed() for > both tap devices and non tap devices? I don't know. Looking at the change, I believe my intent was to not change existing behavior in this respect. > This was pre-existing, but the call to pcap_dispatch() in > netdev_rx_bsd_recv_pcap() needs a space after the cast. I've now added the space, thanks. > Acked-by: Ethan Jackson ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] netdev: Incremental BSD fix for netdev refactoriong
On Fri, May 10, 2013 at 04:06:41PM -0400, Ed Maste wrote: > On 10 May 2013 14:38, Ben Pfaff wrote: > > On Fri, May 10, 2013 at 02:15:50PM -0400, Ed Maste wrote: > >> This is an incremental to patch v3 of > >> netdev: Factor restoring flags into new "struct netdev_saved_flags", > >> implementing the equivalent change for netdev-bsd. > >> > >> Signed-off-by: Ed Maste > >> --- > >> With this, the first patch now passes unit tests for me on FreeBSD. > > > > Great, I folded that in and applied this patch to master. > > Thanks. I find it a little ironic that the trouble here was caused by > the netdev vs. netdev_dev issue itself. This also explains why the > end result is almost the same (when netdev_dev is removed in patch 4). > > > There were merge conflicts in netdev-bsd when I continued to rebase > > through the rest of the series, and I am not confident that I resolved > > them correctly (I did not try a build on FreeBSD), so I will repost the > > rest for your review. > > The conflict resolutions match what I ended up with when I reapplied > patches 2-4 on top of my incremental, at least. All unit tests now > pass on FreeBSD for patches 2-4 in v4; I haven't tried an actual > system test. Thanks for testing. I applied this series to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] Send packet timestamps as export times in IPFIX flow records
On Wed, Apr 24, 2013 at 11:29 AM, Romain Lenglet wrote: > Send an optional OVS_PACKET_ATTR_TIMESTAMP attribute in upcalls with > the packet's timestamp. Add a corresponding tstamp attribute into > struct dpif_upcall. > > Enable Linux packet timestamping, only if at least one flow requires > it since Linux packet timestamping is set on or off for all packets. > Define a new DPIF_FP_TIMESTAMP flag for dpif_flow_put.flags to enable > timestamping for the lifetime of a flow. Add a new netlink > OVS_FLOW_ATTR_TIMESTAMP flag attribute to the Linux datapath's flow > set/mod/get/dump commands. Call net_enable_timestamp() and > net_disable_timestamp() only if that flag is set. > > Enable packet timestamping in the Linux datapath for flows with > "sample" or "ipfix" actions. Send microsecond-precision timestamps > received in upcalls in IPFIX flow records. > > Signed-off-by: Romain Lenglet The biggest question which I have about this is how it interacts with hardware timestamping if it is available. It looks like this only turns on software timestamps and while that is probably often sufficient, it would be nice to take advantage of hardware if possible. The other example of using timestamps is in the socket layer and it seems like there are different methods for both enabling and retrieving timestamps for both hardware and software. I wonder how useful per-flow granularity is on enabling timestamps. It seems like per-port control is the finest granularity that might actually be useful. I think if we did that then the refcounting could be simplified as well. Is there really a need to define a new OVS time struct? It seems like using timespec or timeval would be preferable. What is the performance impact of turning on timestamps? If it is high (I remember that it is definitely measurable since we used to get timestamps on every packet for the purposes of flow expiration before it was optimized) then I wonder whether it is wise to require it whenever IPFIX is enabled since many use cases won't care. Actually, on a greater level I'm not sure that I quite see the value in very precise timestamps on random samples. How exactly would you analyze them? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [flow_metadata 1/5] flow: Use struct flow_tnl in struct flow_metadata.
We can't currently match on some of the fields in struct flow_tnl via
OpenFlow, but it still seems reasonable to use struct flow_tnl in place of
duplicating much of it.
Signed-off-by: Ben Pfaff
---
lib/flow.c|6 ++
lib/flow.h|4 +---
lib/learning-switch.c |4 ++--
lib/ofp-print.c | 15 +--
lib/ofp-util.c| 16 +++-
5 files changed, 21 insertions(+), 24 deletions(-)
diff --git a/lib/flow.c b/lib/flow.c
index 3c12984..680b3230 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -493,9 +493,7 @@ flow_get_metadata(const struct flow *flow, struct
flow_metadata *fmd)
{
BUILD_ASSERT_DECL(FLOW_WC_SEQ == 20);
-fmd->tun_id = flow->tunnel.tun_id;
-fmd->tun_src = flow->tunnel.ip_src;
-fmd->tun_dst = flow->tunnel.ip_dst;
+fmd->tunnel = flow->tunnel;
fmd->metadata = flow->metadata;
memcpy(fmd->regs, flow->regs, sizeof fmd->regs);
fmd->in_port = flow->in_port;
diff --git a/lib/flow.h b/lib/flow.h
index 0fdf4ef..2271418 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -117,9 +117,7 @@ BUILD_ASSERT_DECL(sizeof(struct flow) == sizeof(struct
flow_tnl) + 160 &&
/* Represents the metadata fields of struct flow. */
struct flow_metadata {
-ovs_be64 tun_id; /* Encapsulating tunnel ID. */
-ovs_be32 tun_src;/* Tunnel outer IPv4 src addr */
-ovs_be32 tun_dst;/* Tunnel outer IPv4 dst addr */
+struct flow_tnl tunnel; /* Encapsulating tunnel parameters. */
ovs_be64 metadata; /* OpenFlow 1.1+ metadata field. */
uint32_t regs[FLOW_N_REGS]; /* Registers. */
uint16_t in_port;/* OpenFlow port or zero. */
diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index 4a95dc1..98923fb 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -559,7 +559,7 @@ process_packet_in(struct lswitch *sw, const struct
ofp_header *oh)
/* Extract flow data from 'opi' into 'flow'. */
ofpbuf_use_const(&pkt, pi.packet, pi.packet_len);
flow_extract(&pkt, 0, 0, NULL, pi.fmd.in_port, &flow);
-flow.tunnel.tun_id = pi.fmd.tun_id;
+flow.tunnel = pi.fmd.tunnel;
/* Choose output port. */
out_port = lswitch_choose_destination(sw, &flow);
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 8d99844..1b89d3d 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -107,16 +107,19 @@ ofp_print_packet_in(struct ds *string, const struct
ofp_header *oh,
ds_put_format(string, " total_len=%"PRIu16" in_port=", pin.total_len);
ofputil_format_port(pin.fmd.in_port, string);
-if (pin.fmd.tun_id != htonll(0)) {
-ds_put_format(string, " tun_id=0x%"PRIx64, ntohll(pin.fmd.tun_id));
+if (pin.fmd.tunnel.tun_id != htonll(0)) {
+ds_put_format(string, " tun_id=0x%"PRIx64,
+ ntohll(pin.fmd.tunnel.tun_id));
}
-if (pin.fmd.tun_src != htonl(0)) {
-ds_put_format(string, " tun_src="IP_FMT, IP_ARGS(pin.fmd.tun_src));
+if (pin.fmd.tunnel.ip_src != htonl(0)) {
+ds_put_format(string, " tun_src="IP_FMT,
+ IP_ARGS(pin.fmd.tunnel.ip_src));
}
-if (pin.fmd.tun_dst != htonl(0)) {
-ds_put_format(string, " tun_dst="IP_FMT, IP_ARGS(pin.fmd.tun_dst));
+if (pin.fmd.tunnel.ip_dst != htonl(0)) {
+ds_put_format(string, " tun_dst="IP_FMT,
+ IP_ARGS(pin.fmd.tunnel.ip_dst));
}
if (pin.fmd.metadata != htonll(0)) {
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 2ca0077..658b541 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -2442,9 +2442,7 @@ ofputil_decode_packet_in_finish(struct ofputil_packet_in
*pin,
pin->packet_len = b->size;
pin->fmd.in_port = match->flow.in_port;
-pin->fmd.tun_id = match->flow.tunnel.tun_id;
-pin->fmd.tun_src = match->flow.tunnel.ip_src;
-pin->fmd.tun_dst = match->flow.tunnel.ip_dst;
+pin->fmd.tunnel = match->flow.tunnel;
pin->fmd.metadata = match->flow.metadata;
memcpy(pin->fmd.regs, match->flow.regs, sizeof pin->fmd.regs);
}
@@ -2542,14 +2540,14 @@ ofputil_packet_in_to_match(const struct
ofputil_packet_in *pin,
int i;
match_init_catchall(match);
-if (pin->fmd.tun_id != htonll(0)) {
-match_set_tun_id(match, pin->fmd.tun_id);
+if (pin->fmd.tunnel.tun_id != htonll(0)) {
+match_set_tun_id(match, pin
[ovs-dev] [flow_metadata 2/5] flow: Extend flow_metadata member 'in_port' from 16 to 32 bits.
This matches the size in struct flow and allows for future extensions to
OpenFlow 1.1+ (which uses 32-bit port numbers) and to hold kernel port
numbers (which also have a 32-bit range).
Signed-off-by: Ben Pfaff
---
lib/flow.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/flow.h b/lib/flow.h
index 2271418..6d8615b 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -120,7 +120,7 @@ struct flow_metadata {
struct flow_tnl tunnel; /* Encapsulating tunnel parameters. */
ovs_be64 metadata; /* OpenFlow 1.1+ metadata field. */
uint32_t regs[FLOW_N_REGS]; /* Registers. */
-uint16_t in_port;/* OpenFlow port or zero. */
+uint32_t in_port;/* OpenFlow port or zero. */
};
void flow_extract(struct ofpbuf *, uint32_t priority, uint32_t mark,
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [flow_metadata 3/5] flow: Add skb_priority and skb_mark to struct flow_metadata.
With these additions, flow_metadata actually contains all of the metadata
fields in a struct flow.
Signed-off-by: Ben Pfaff
---
lib/flow.c |2 ++
lib/flow.h |2 ++
lib/ofp-util.c | 10 ++
3 files changed, 14 insertions(+)
diff --git a/lib/flow.c b/lib/flow.c
index 680b3230..ac3c32b 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -497,6 +497,8 @@ flow_get_metadata(const struct flow *flow, struct
flow_metadata *fmd)
fmd->metadata = flow->metadata;
memcpy(fmd->regs, flow->regs, sizeof fmd->regs);
fmd->in_port = flow->in_port;
+fmd->skb_priority = flow->skb_priority;
+fmd->skb_mark = flow->skb_mark;
}
char *
diff --git a/lib/flow.h b/lib/flow.h
index 6d8615b..40f5968 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -121,6 +121,8 @@ struct flow_metadata {
ovs_be64 metadata; /* OpenFlow 1.1+ metadata field. */
uint32_t regs[FLOW_N_REGS]; /* Registers. */
uint32_t in_port;/* OpenFlow port or zero. */
+uint32_t skb_priority; /* Packet priority for QoS. */
+uint32_t skb_mark; /* Packet mark. */
};
void flow_extract(struct ofpbuf *, uint32_t priority, uint32_t mark,
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 658b541..da9af72 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -2445,6 +2445,8 @@ ofputil_decode_packet_in_finish(struct ofputil_packet_in
*pin,
pin->fmd.tunnel = match->flow.tunnel;
pin->fmd.metadata = match->flow.metadata;
memcpy(pin->fmd.regs, match->flow.regs, sizeof pin->fmd.regs);
+pin->fmd.skb_priority = match->flow.skb_priority;
+pin->fmd.skb_mark = match->flow.skb_mark;
}
enum ofperr
@@ -2560,6 +2562,14 @@ ofputil_packet_in_to_match(const struct
ofputil_packet_in *pin,
}
match_set_in_port(match, pin->fmd.in_port);
+
+if (pin->fmd.skb_priority) {
+match_set_skb_priority(match, pin->fmd.skb_priority);
+}
+
+if (pin->fmd.skb_mark) {
+match_set_skb_priority(match, pin->fmd.skb_mark);
+}
}
/* Converts abstract ofputil_packet_in 'pin' into a PACKET_IN message
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [flow_metadata 5/5] flow: Make flow_extract()'s caller responsible for metadata.
We are getting to have a lot of metadata in struct flow. Until now, we
have tried to pass all of it as parameters to flow_extract(). Now that
the data and metadata members of struct flow are clearly separated, it
seems more reasonable to just make the caller initialize them itself.
Signed-off-by: Ben Pfaff
---
lib/dpif-netdev.c |5 +++--
lib/flow.c | 16 +++-
lib/flow.h |3 +--
lib/learning-switch.c |3 ++-
lib/ofp-print.c|2 +-
ofproto/ofproto-dpif.c | 12
ofproto/ofproto.c |6 --
tests/test-flows.c |5 +++--
8 files changed, 25 insertions(+), 27 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 8984f7d..f9a845d 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -941,7 +941,7 @@ dpif_netdev_execute(struct dpif *dpif, const struct
dpif_execute *execute)
ofpbuf_reserve(©, DP_NETDEV_HEADROOM);
ofpbuf_put(©, execute->packet->data, execute->packet->size);
-flow_extract(©, 0, 0, NULL, -1, &key);
+flow_extract(©, &key);
error = dpif_netdev_flow_from_nlattrs(execute->key, execute->key_len,
&key);
if (!error) {
@@ -1039,7 +1039,8 @@ dp_netdev_port_input(struct dp_netdev *dp, struct
dp_netdev_port *port,
if (packet->size < ETH_HEADER_LEN) {
return;
}
-flow_extract(packet, 0, 0, NULL, port->port_no, &key);
+flow_extract(packet, &key);
+key.md.in_port = port->port_no;
flow = dp_netdev_lookup_flow(dp, &key);
if (flow) {
dp_netdev_flow_used(flow, packet);
diff --git a/lib/flow.c b/lib/flow.c
index de4d171..2eb87e6 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -336,8 +336,8 @@ invalid:
}
-/* Initializes 'flow' members from 'packet', 'skb_priority', 'tnl', and
- * 'ofp_in_port'.
+/* Initializes 'flow' data members from 'packet'. Zeroes 'flow->md', which the
+ * caller must initialize appropriately afterward.
*
* Initializes 'packet' header pointers as follows:
*
@@ -356,9 +356,7 @@ invalid:
* present and has a correct length, and otherwise NULL.
*/
void
-flow_extract(struct ofpbuf *packet, uint32_t skb_priority, uint32_t skb_mark,
- const struct flow_tnl *tnl, uint16_t ofp_in_port,
- struct flow *flow)
+flow_extract(struct ofpbuf *packet, struct flow *flow)
{
struct ofpbuf b = *packet;
struct eth_header *eth;
@@ -367,14 +365,6 @@ flow_extract(struct ofpbuf *packet, uint32_t skb_priority,
uint32_t skb_mark,
memset(flow, 0, sizeof *flow);
-if (tnl) {
-ovs_assert(tnl != &flow->md.tunnel);
-flow->md.tunnel = *tnl;
-}
-flow->md.in_port = ofp_in_port;
-flow->md.skb_priority = skb_priority;
-flow->md.skb_mark = skb_mark;
-
packet->l2 = b.data;
packet->l2_5 = NULL;
packet->l3 = NULL;
diff --git a/lib/flow.h b/lib/flow.h
index 1610871..d593882 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -116,8 +116,7 @@ BUILD_ASSERT_DECL(sizeof(struct flow) % 4 == 0);
BUILD_ASSERT_DECL(sizeof(struct flow) == sizeof(struct flow_metadata) + 104 &&
FLOW_WC_SEQ == 20);
-void flow_extract(struct ofpbuf *, uint32_t priority, uint32_t mark,
- const struct flow_tnl *, uint16_t in_port, struct flow *);
+void flow_extract(struct ofpbuf *, struct flow *);
void flow_zero_wildcards(struct flow *, const struct flow_wildcards *);
diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index ec308fe..c55b102 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -558,7 +558,8 @@ process_packet_in(struct lswitch *sw, const struct
ofp_header *oh)
/* Extract flow data from 'opi' into 'flow'. */
ofpbuf_use_const(&pkt, pi.packet, pi.packet_len);
-flow_extract(&pkt, 0, 0, &pi.fmd.tunnel, pi.fmd.in_port, &flow);
+flow_extract(&pkt, &flow);
+flow.md = pi.fmd;
/* Choose output port. */
out_port = lswitch_choose_destination(sw, &flow);
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 1b89d3d..af2ca4a 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -62,7 +62,7 @@ ofp_packet_to_string(const void *data, size_t len)
struct flow flow;
ofpbuf_use_const(&buf, data, len);
-flow_extract(&buf, 0, 0, NULL, 0, &flow);
+flow_extract(&buf, &flow);
flow_format(&ds, &flow);
if (buf.l7) {
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 5c37509..a9c1ad3 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4030,8 +4030,8 @@ handle_miss_upcalls(struct dpif_backer *backer, struct
dpif_upcall *upcalls,
}
ofproto->n_missed++;
-flow_extract(upcall->packet, flow.md.skb_priority, flow.md.skb_mark,
- &flow.md.tunnel, flow.md.in_port, &miss->flow);
+flow_extract(upcall->packet, &miss->flow);
+miss->flow.md = flow.md;
/* Add other packets to a to-do list. */
hash = flow_has
[ovs-dev] [flow_metadata 0/5] improve and use flow_metadata more widely
The "struct flow_metadata" is only used in a few specific places currently, but it can be generalized and used more broadly. This series does that. The first three patches seem quite reasonable and low-impact to me: flow: Use struct flow_tnl in struct flow_metadata. flow: Extend flow_metadata member 'in_port' from 16 to 32 bits. flow: Add skb_priority and skb_mark to struct flow_metadata. The remaining two patches are more likely to prompt a reaction. I am curious to see what that reaction is: flow: Use struct flow_metadata inside struct flow. flow: Make flow_extract()'s caller responsible for metadata. lib/cfm.c |4 +- lib/dpif-netdev.c | 15 +++--- lib/flow.c | 34 ++--- lib/flow.h | 51 --- lib/learning-switch.c | 14 +++--- lib/match.c | 122 ++-- lib/meta-flow.c | 72 +- lib/nx-match.c | 17 --- lib/odp-util.c | 44 lib/ofp-parse.c |6 +-- lib/ofp-print.c | 17 --- lib/ofp-util.c | 64 +--- ofproto/netflow.c |4 +- ofproto/ofproto-dpif.c | 128 +-- ofproto/ofproto.c |6 ++- ofproto/tunnel.c| 32 ++-- ofproto/tunnel.h|2 +- tests/test-bundle.c |8 +-- tests/test-classifier.c | 58 ++--- tests/test-flows.c |5 +- tests/test-multipath.c |6 +-- tests/test-odp.c|2 +- 22 files changed, 350 insertions(+), 361 deletions(-) -- 1.7.10.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] tests/ofproto-dpif.at: Add tests for the ofproto/trace command
Two testcases are added to the testsuite, which test the new
command syntax and the corresponding corner cases.
Signed-off-by: Alex Wang
---
tests/ofproto-dpif.at | 303 +
1 file changed, 303 insertions(+)
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 6a46ddb..dcc2405 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -1044,6 +1044,309 @@ AT_CHECK([ovs-dpctl normalize-actions "$flow"
"$actual"], [0], [expout])
OVS_VSWITCHD_STOP
AT_CLEANUP
+# Two testcases below for the ofproto/trace command
+# The first one tests all correct syntax:
+# ofproto/trace [dp_name] odp_flow [-generate|packet]
+# ofproto/trace br_name br_flow [-generate|packet]
+AT_SETUP([ofproto-dpif - ofproto/trace command 1])
+OVS_VSWITCHD_START([set bridge br0 fail-mode=standalone])
+ADD_OF_PORTS([br0], 1, 2, 3)
+
+AT_DATA([flows.txt], [dnl
+in_port=1 actions=output:2
+in_port=2 actions=output:1
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+odp_flow="in_port(1)"
+br_flow="in_port=1"
+# Test command: ofproto/trace odp_flow
+AT_CHECK([ovs-appctl ofproto/trace "$odp_flow"], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0], [dnl
+Datapath actions: 2
+])
+
+# Test command: ofproto/trace dp_name odp_flow
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "$odp_flow"], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0], [dnl
+Datapath actions: 2
+])
+# Test commmand: ofproto/trace br_name br_flow
+AT_CHECK([ovs-appctl ofproto/trace br0 "$br_flow"], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0], [dnl
+Datapath actions: 2
+])
+
+# Delete the inserted flows
+AT_CHECK([ovs-ofctl del-flows br0 "in_port=1"], [0], [stdout])
+AT_CHECK([ovs-ofctl del-flows br0 "in_port=2"], [0], [stdout])
+
+# This section beflow tests the [-generate] option
+odp_flow="in_port(3),eth(src=50:54:00:00:00:05,dst=ff:ff:ff:ff:ff:ff)"
+br_flow="arp,metadata=0,in_port=3,vlan_tci=0x,dl_src=50:54:00:00:00:05,dl_dst=ff:ff:ff:ff:ff:ff"
+
+# Test command: ofproto/trace odp_flow
+AT_CHECK([ovs-appctl ofproto/trace "$odp_flow"], [0], [stdout])
+# Check for no MAC learning entry.
+AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], [0],
[dnl
+ port VLAN MACAge
+])
+
+# Test command: ofproto/trace br_name br_flow
+AT_CHECK([ovs-appctl ofproto/trace br0 "$br_flow"], [0], [stdout])
+# Check for no MAC learning entry.
+AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], [0],
[dnl
+ port VLAN MACAge
+])
+
+# Test command: ofproto/trace odp_flow -generate
+AT_CHECK([ovs-appctl ofproto/trace "$odp_flow" -generate], [0], [stdout])
+# Check for the MAC learning entry.
+AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], [0],
[dnl
+ port VLAN MACAge
+3 0 50:54:00:00:00:05?
+])
+
+# Test command: ofproto/trace dp_name odp_flow -generate
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy \
+ "in_port(1),eth(src=50:54:00:00:00:06,dst=50:54:00:00:00:05)" \
+ -generate], [0], [stdout])
+# Check for both MAC learning entries.
+AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], [0],
[dnl
+ port VLAN MACAge
+3 0 50:54:00:00:00:05?
+1 0 50:54:00:00:00:06?
+])
+
+# Test command: ofproto/trace br_name br_flow -generate
+AT_CHECK([ovs-appctl ofproto/trace br0 \
+ "in_port=2,dl_src=50:54:00:00:00:07,dl_dst=50:54:00:00:00:06" \
+ -generate], [0], [stdout])
+# Check for both MAC learning entries.
+AT_CHECK_UNQUOTED([ovs-appctl fdb/show br0 | sed 's/[[0-9]]\{1,\}$/?/'], [0],
[dnl
+ port VLAN MACAge
+3 0 50:54:00:00:00:05?
+1 0 50:54:00:00:00:06?
+2 0 50:54:00:00:00:07?
+])
+
+# This section beflow tests the [packet] option
+# The ovs-tcpundump of packets between port1 and port2
+pkt1to2="50540002505400010806451C00014001F98CC0A80001C0A800020800F7FF"
+pkt2to1="50540001505400020806451C00014001F98CC0A80002C0A800010800F7FF"
+
+# Construct the MAC learning table
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy \
+ "in_port(1),eth(src=50:54:00:00:00:01,dst=ff:ff:ff:ff:ff:ff)" \
+ -generate], [0], [stdout])
+
+# Construct the MAC learning table
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy \
+ "in_port(2),eth(src=50:54:00:00:00:02,dst=ff:ff:ff:ff:ff:ff)" \
+ -generate], [0], [stdout])
+
+# Test command: ofproto/trace odp_flow packet
+AT_CHECK([ovs-appctl ofproto/trace \
+ "in_port(1),skb_priority(1),skb_mark(2)" "$pkt1to2"], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0], [dnl
+Datapath actions: 2
+])
+AT_CHECK([head -n 3 stdout], [0], [dnl
+Bridge: br0
+Packet:
arp,metadata=0,in_port=0,vlan_tci=0x,dl_src=50:54:00:00:00:01,dl_dst=50:54:00:00:00:02,arp_spa=0.0.0.0,arp_tpa=0.0.0.0,arp_sha=00:00:00:00:00:00,arp_tha=00:00:00:00:00:00
+Flow:
skb_mark=0x2,skb_priority=0x1,arp,metadata=0,in_port=1,vlan_tci=0x,dl_src=50:54:00:0
