Thanks for the patch It looks good to me, a few minor comments inline
On 15/07/2016 04:54, "Ilya Maximets" <i.maxim...@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.maxim...@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 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev