[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