[ovs-dev] [PATCH v3 1/3] bridge: Pass interface's configuration to datapath.

Daniele Di Proietto diproiettod at vmware.com
Tue Jul 26 01:45:51 UTC 2016


Thanks for the patch

It looks good to me, a few minor comments inline


On 15/07/2016 04:54, "Ilya Maximets" <i.maximets at samsung.com> wrote:

>This commit adds functionality to pass value of 'other_config' column
>of 'Interface' table to datapath.
>
>This may be used to pass not directly connected with netdev options and
>configure behaviour of the datapath for different ports.
>For example: pinning of rx queues to polling threads in dpif-netdev.
>
>Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
>---
> lib/dpif-netdev.c          |  1 +
> lib/dpif-netlink.c         |  1 +
> lib/dpif-provider.h        |  5 +++++
> lib/dpif.c                 | 17 +++++++++++++++++
> lib/dpif.h                 |  1 +
> ofproto/ofproto-dpif.c     | 16 ++++++++++++++++
> ofproto/ofproto-provider.h |  5 +++++
> ofproto/ofproto.c          | 29 +++++++++++++++++++++++++++++
> ofproto/ofproto.h          |  2 ++
> vswitchd/bridge.c          |  2 ++
> 10 files changed, 79 insertions(+)
>
>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>index 6345944..4643cce 100644
>--- a/lib/dpif-netdev.c
>+++ b/lib/dpif-netdev.c
>@@ -4295,6 +4295,7 @@ const struct dpif_class dpif_netdev_class = {
>     dpif_netdev_get_stats,
>     dpif_netdev_port_add,
>     dpif_netdev_port_del,
>+    NULL,                       /* port_set_config */
>     dpif_netdev_port_query_by_number,
>     dpif_netdev_port_query_by_name,
>     NULL,                       /* port_get_pid */
>diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
>index e2bea23..2f939ae 100644
>--- a/lib/dpif-netlink.c
>+++ b/lib/dpif-netlink.c
>@@ -2348,6 +2348,7 @@ const struct dpif_class dpif_netlink_class = {
>     dpif_netlink_get_stats,
>     dpif_netlink_port_add,
>     dpif_netlink_port_del,
>+    NULL,                       /* port_set_config */
>     dpif_netlink_port_query_by_number,
>     dpif_netlink_port_query_by_name,
>     dpif_netlink_port_get_pid,
>diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
>index 25f4280..21fb0ba 100644
>--- a/lib/dpif-provider.h
>+++ b/lib/dpif-provider.h
>@@ -167,6 +167,11 @@ struct dpif_class {
>     /* Removes port numbered 'port_no' from 'dpif'. */
>     int (*port_del)(struct dpif *dpif, odp_port_t port_no);
> 
>+    /* Refreshes configuration of 'dpif's port. The implementation might
>+     * postpone applying the changes until run() is called. */
>+    int (*port_set_config)(struct dpif *dpif, odp_port_t port_no,
>+                           const struct smap *cfg);
>+
>     /* Queries 'dpif' for a port with the given 'port_no' or 'devname'.
>      * If 'port' is not null, stores information about the port into
>      * '*port' if successful.
>diff --git a/lib/dpif.c b/lib/dpif.c
>index 5f1be41..f6e5338 100644
>--- a/lib/dpif.c
>+++ b/lib/dpif.c
>@@ -610,6 +610,23 @@ dpif_port_exists(const struct dpif *dpif, const char *devname)
>     return !error;
> }
> 
>+/* Refreshes configuration of 'dpif's port. */
>+int
>+dpif_port_set_config(struct dpif *dpif, odp_port_t port_no,
>+                     const struct smap *cfg)
>+{
>+    int error = 0;
>+
>+    if (dpif->dpif_class->port_set_config) {
>+        error = dpif->dpif_class->port_set_config(dpif, port_no, cfg);
>+        if (error) {
>+            log_operation(dpif, "port_set_config", error);
>+        }
>+    }
>+
>+    return error;
>+}
>+
> /* Looks up port number 'port_no' in 'dpif'.  On success, returns 0 and
>  * initializes '*port' appropriately; on failure, returns a positive errno
>  * value.
>diff --git a/lib/dpif.h b/lib/dpif.h
>index 981868c..a7c5097 100644
>--- a/lib/dpif.h
>+++ b/lib/dpif.h
>@@ -839,6 +839,7 @@ void dpif_register_upcall_cb(struct dpif *, upcall_callback *, void *aux);
> int dpif_recv_set(struct dpif *, bool enable);
> int dpif_handlers_set(struct dpif *, uint32_t n_handlers);
> int dpif_poll_threads_set(struct dpif *, const char *cmask);
>+int dpif_port_set_config(struct dpif *, odp_port_t, const struct smap *cfg);
> int dpif_recv(struct dpif *, uint32_t handler_id, struct dpif_upcall *,
>               struct ofpbuf *);
> void dpif_recv_purge(struct dpif *);
>diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>index ce9383a..97510a9 100644
>--- a/ofproto/ofproto-dpif.c
>+++ b/ofproto/ofproto-dpif.c
>@@ -3542,6 +3542,21 @@ port_del(struct ofproto *ofproto_, ofp_port_t ofp_port)
> }
> 
> static int
>+port_set_config(struct ofproto *ofproto_, ofp_port_t ofp_port,
>+                const struct smap *cfg)

Can we change this to directly take struct ofport_dpif *ofport instead of ofp_port_t?

>+{
>+    struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
>+    struct ofport_dpif *ofport = ofp_port_to_ofport(ofproto, ofp_port);
>+
>+    if (!ofport || sset_contains(&ofproto->ghost_ports,
>+                                 netdev_get_name(ofport->up.netdev))) {
>+        return 0;
>+    }
>+
>+    return dpif_port_set_config(ofproto->backer->dpif, ofport->odp_port, cfg);
>+}
>+
>+static int
> port_get_stats(const struct ofport *ofport_, struct netdev_stats *stats)
> {
>     struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
>@@ -5609,6 +5624,7 @@ const struct ofproto_class ofproto_dpif_class = {
>     port_query_by_name,
>     port_add,
>     port_del,
>+    port_set_config,
>     port_get_stats,
>     port_dump_start,
>     port_dump_next,
>diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
>index ae6c08d..883641d 100644
>--- a/ofproto/ofproto-provider.h
>+++ b/ofproto/ofproto-provider.h
>@@ -972,6 +972,11 @@ struct ofproto_class {
>      * convenient. */
>     int (*port_del)(struct ofproto *ofproto, ofp_port_t ofp_port);
> 
>+    /* Refreshes dtapath configuration of port number 'ofp_port' in 'ofproto'.

s/dtapath/datapath/

>+     * Returns 0 if successful, otherwise a positive errno value. */
>+    int (*port_set_config)(struct ofproto *ofproto, ofp_port_t ofp_port,
>+                           const struct smap *cfg);
>+
>     /* Get port stats */
>     int (*port_get_stats)(const struct ofport *port,
>                           struct netdev_stats *stats);
>diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
>index 5f84aa1..ee9b689 100644
>--- a/ofproto/ofproto.c
>+++ b/ofproto/ofproto.c
>@@ -2078,6 +2078,35 @@ ofproto_port_del(struct ofproto *ofproto, ofp_port_t ofp_port)
>     return error;
> }
> 
>+/* Refreshes dtapath configuration of port number 'ofp_port' in 'ofproto'.

s/dtapath/datapath/


>+ *
>+ * This function has no effect if 'ofproto' does not have a port 'ofp_port'. */
>+void
>+ofproto_port_set_config(struct ofproto *ofproto, ofp_port_t ofp_port,
>+                        const struct smap *cfg)
>+{
>+    struct ofport *ofport;
>+    int error;
>+
>+    ofport = ofproto_get_port(ofproto, ofp_port);
>+    if (!ofport) {
>+        VLOG_WARN("%s: cannot configure datapath on nonexistent port %"PRIu16,
>+                  ofproto->name, ofp_port);
>+        return;
>+    }
>+
>+    error = (ofproto->ofproto_class->port_set_config
>+             ? ofproto->ofproto_class->port_set_config(ofproto, ofp_port, cfg)
>+             : EOPNOTSUPP);
>+    if (error) {
>+        VLOG_WARN("%s: dtatapath configuration on port %"PRIu16

s/dtatapath/datapath/

>+                  " (%s) failed (%s)",
>+                  ofproto->name, ofp_port, netdev_get_name(ofport->netdev),
>+                  ovs_strerror(error));
>+    }
>+}
>+
>+
> static void
> flow_mod_init(struct ofputil_flow_mod *fm,
>               const struct match *match, int priority,
>diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
>index a60327a..22d94c7 100644
>--- a/ofproto/ofproto.h
>+++ b/ofproto/ofproto.h
>@@ -293,6 +293,8 @@ const char *ofproto_port_open_type(const char *datapath_type,
>                                    const char *port_type);
> int ofproto_port_add(struct ofproto *, struct netdev *, ofp_port_t *ofp_portp);
> int ofproto_port_del(struct ofproto *, ofp_port_t ofp_port);
>+void ofproto_port_set_config(struct ofproto *, ofp_port_t ofp_port,
>+                             const struct smap *cfg);
> int ofproto_port_get_stats(const struct ofport *, struct netdev_stats *stats);
> 
> int ofproto_port_query_by_name(const struct ofproto *, const char *devname,
>diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
>index a5de84e..6c271a3 100644
>--- a/vswitchd/bridge.c
>+++ b/vswitchd/bridge.c
>@@ -655,6 +655,8 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
>                                      &iface->cfg->bfd);
>                 ofproto_port_set_lldp(br->ofproto, iface->ofp_port,
>                                       &iface->cfg->lldp);
>+                ofproto_port_set_config(br->ofproto, iface->ofp_port,
>+                                        &iface->cfg->other_config);
>             }
>         }
>         bridge_configure_mirrors(br);
>-- 
>2.7.4


More information about the dev mailing list