| Message ID | 1502979847-22898-2-git-send-email-billy.o.mahony@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
>From: O Mahony, Billy >Sent: Thursday, August 17, 2017 3:24 PM >To: dev@openvswitch.org >Cc: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; O Mahony, Billy ><billy.o.mahony@intel.com> >Subject: [PATCH v2 1/4] netdev: Add set_ingress_sched to netdev api > >Passes ingress_sched config item from other_config column of Interface >table to the netdev. > >Signed-off-by: Billy O'Mahony <billy.o.mahony@intel.com> >--- > lib/netdev-bsd.c | 1 + > lib/netdev-dpdk.c | 21 +++++++++++++++++++++ > lib/netdev-dummy.c | 1 + > lib/netdev-linux.c | 1 + > lib/netdev-provider.h | 10 ++++++++++ > lib/netdev-vport.c | 1 + > lib/netdev.c | 22 ++++++++++++++++++++++ > lib/netdev.h | 1 + > vswitchd/bridge.c | 2 ++ > 9 files changed, 60 insertions(+) > >diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c >index 8a4cdb3..3943b7b 100644 >--- a/lib/netdev-bsd.c >+++ b/lib/netdev-bsd.c >@@ -1506,6 +1506,7 @@ netdev_bsd_update_flags(struct netdev *netdev_, enum >netdev_flags off, > netdev_bsd_get_etheraddr, \ > netdev_bsd_get_mtu, \ > NULL, /* set_mtu */ \ >+ NULL, /* set_ingress_sched */ \ > netdev_bsd_get_ifindex, \ > netdev_bsd_get_carrier, \ > NULL, /* get_carrier_resets */ \ >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >index 1aaf6f7..1ffedd4 100644 >--- a/lib/netdev-dpdk.c >+++ b/lib/netdev-dpdk.c >@@ -390,6 +390,9 @@ struct netdev_dpdk { > /* If true, device was attached by rte_eth_dev_attach(). */ > bool attached; > >+ /* Ingress Scheduling config */ >+ char *ingress_sched_str; >+ > /* In dpdk_list. */ > struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex); > >@@ -1081,6 +1084,9 @@ netdev_dpdk_destruct(struct netdev *netdev) > } > > free(dev->devargs); >+ if (dev->ingress_sched_str) { >+ free(dev->ingress_sched_str); >+ } > common_destruct(dev); > > ovs_mutex_unlock(&dpdk_mutex); >@@ -2004,6 +2010,20 @@ netdev_dpdk_set_mtu(struct netdev *netdev, int mtu) > } > > static int >+netdev_dpdk_set_ingress_sched(struct netdev *netdev, >+ const char *ingress_sched_str) >+{ >+ struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); >+ >+ free(dev->ingress_sched_str); Is the intended behavior here that the previous scheduling policy is always freed, even if the new ingress_sched_str is, for whatever reason, NULL (the possibility of which is implied by the check on the next line)? Should the behavior instead be that the old value is maintained until a new valid one is obtained, or is setting ingress_sched_str to NULL simply a way of disabling ingress scheduling? If the latter, a comment to that effect would be useful. >+ if (ingress_sched_str) { >+ dev->ingress_sched_str = xstrdup(ingress_sched_str); >+ } >+ >+ return 0; >+} >+ >+static int > netdev_dpdk_get_carrier(const struct netdev *netdev, bool *carrier); > > static int >@@ -3342,6 +3362,7 @@ unlock: > netdev_dpdk_get_etheraddr, \ > netdev_dpdk_get_mtu, \ > netdev_dpdk_set_mtu, \ >+ netdev_dpdk_set_ingress_sched, \ Shouldn't this function be localized to 'dpdk' ports, since it's not applicable in its current implementation to vhost ports? > netdev_dpdk_get_ifindex, \ > GET_CARRIER, \ > netdev_dpdk_get_carrier_resets, \ >diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c >index f731af1..0c36d2d 100644 >--- a/lib/netdev-dummy.c >+++ b/lib/netdev-dummy.c >@@ -1378,6 +1378,7 @@ netdev_dummy_update_flags(struct netdev *netdev_, > netdev_dummy_get_etheraddr, \ > netdev_dummy_get_mtu, \ > netdev_dummy_set_mtu, \ >+ NULL, /* set_ingress_sched */ \ > netdev_dummy_get_ifindex, \ > NULL, /* get_carrier */ \ > NULL, /* get_carrier_resets */ \ >diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c >index 2ff3e2b..00cbe17 100644 >--- a/lib/netdev-linux.c >+++ b/lib/netdev-linux.c >@@ -2847,6 +2847,7 @@ netdev_linux_update_flags(struct netdev *netdev_, enum >netdev_flags off, > netdev_linux_get_etheraddr, \ > netdev_linux_get_mtu, \ > netdev_linux_set_mtu, \ >+ NULL, /* set_ingress_sched */ \ > netdev_linux_get_ifindex, \ > netdev_linux_get_carrier, \ > netdev_linux_get_carrier_resets, \ >diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h >index b3c57d5..6cbd066 100644 >--- a/lib/netdev-provider.h >+++ b/lib/netdev-provider.h >@@ -34,6 +34,7 @@ extern "C" { > > struct netdev_tnl_build_header_params; > #define NETDEV_NUMA_UNSPEC OVS_NUMA_UNSPEC >+#define NETDEV_PRIO_RXQ_UNSPEC (-1) > > /* A network device (e.g. an Ethernet device). > * >@@ -76,6 +77,7 @@ struct netdev { > * modify them. */ > int n_txq; > int n_rxq; >+ int priority_rxq; /* id of prioritized rxq. -1 = None <micro-nit> s/id/ID/ </micro-nit> >*/ > int ref_cnt; /* Times this devices was opened. */ > struct shash_node *node; /* Pointer to element in global map. >*/ > struct ovs_list saved_flags_list; /* Contains "struct >netdev_saved_flags". */ >@@ -412,6 +414,14 @@ struct netdev_class { > * null if it would always return EOPNOTSUPP. */ > int (*set_mtu)(struct netdev *netdev, int mtu); > >+ /* Sets 'netdev''s ingress scheduling policy. >+ * >+ * If 'netdev' does not support the specified policy then >+ * this function should return EOPNOTSUPP. This function may be set to >+ * null if it would always return EOPNOTSUPP. */ >+ int (*set_ingress_sched)(struct netdev *netdev, >+ const char *ingress_sched_str); >+ > /* Returns the ifindex of 'netdev', if successful, as a positive number. > * On failure, returns a negative errno value. > * >diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c >index d11c5cc..57a9b01 100644 >--- a/lib/netdev-vport.c >+++ b/lib/netdev-vport.c >@@ -901,6 +901,7 @@ netdev_vport_get_ifindex(const struct netdev *netdev_) > netdev_vport_get_etheraddr, \ > NULL, /* get_mtu */ \ > NULL, /* set_mtu */ \ >+ NULL, /* set_ingress_sched */ \ > GET_IFINDEX, \ > NULL, /* get_carrier */ \ > NULL, /* get_carrier_resets */ \ >diff --git a/lib/netdev.c b/lib/netdev.c >index b4e570b..4562fdf 100644 >--- a/lib/netdev.c >+++ b/lib/netdev.c >@@ -418,6 +418,7 @@ netdev_open(const char *name, const char *type, struct >netdev **netdevp) > /* By default enable one tx and rx queue per netdev. */ > netdev->n_txq = netdev->netdev_class->send ? 1 : 0; > netdev->n_rxq = netdev->netdev_class->rxq_alloc ? 1 : 0; >+ netdev->priority_rxq = NETDEV_PRIO_RXQ_UNSPEC; > > ovs_list_init(&netdev->saved_flags_list); > >@@ -978,6 +979,27 @@ netdev_mtu_is_user_config(struct netdev *netdev) > return netdev->mtu_user_config; > } > >+/* Sets the Ingress Scheduling policy of 'netdev'. >+ * >+ * If successful, returns 0. Returns EOPNOTSUPP if 'netdev' does not support >+ * the specified policy */ >+int >+netdev_set_ingress_sched(struct netdev *netdev, const char >*ingress_sched_str) >+{ >+ const struct netdev_class *class = netdev->netdev_class; >+ int error; >+ >+ error = class->set_ingress_sched ? >+ class->set_ingress_sched(netdev, ingress_sched_str) : EOPNOTSUPP; >+ if (error && error != EOPNOTSUPP) { >+ VLOG_DBG_RL(&rl, "failed to set ingress scheduling for network " \ >+ "device %s: %s", >+ netdev_get_name(netdev), ovs_strerror(error)); >+ } >+ >+ return error; >+} >+ > /* Returns the ifindex of 'netdev', if successful, as a positive number. On > * failure, returns a negative errno value. > * >diff --git a/lib/netdev.h b/lib/netdev.h >index f8482f7..b65085c 100644 >--- a/lib/netdev.h >+++ b/lib/netdev.h >@@ -164,6 +164,7 @@ int netdev_get_mtu(const struct netdev *, int *mtup); > int netdev_set_mtu(struct netdev *, int mtu); > void netdev_mtu_user_config(struct netdev *, bool); > bool netdev_mtu_is_user_config(struct netdev *); >+int netdev_set_ingress_sched(struct netdev *, const char *ingress_sched_str); > int netdev_get_ifindex(const struct netdev *); > int netdev_set_tx_multiq(struct netdev *, unsigned int n_txq); > enum netdev_pt_mode netdev_get_pt_mode(const struct netdev *); >diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c >index a8cbae7..93c91f6 100644 >--- a/vswitchd/bridge.c >+++ b/vswitchd/bridge.c >@@ -1794,6 +1794,8 @@ iface_do_create(const struct bridge *br, > } > > iface_set_netdev_mtu(iface_cfg, netdev); >+ netdev_set_ingress_sched(netdev, >+ smap_get(&iface_cfg->other_config, "ingress_sched")); The convention in this function is to wrap netdev calls within an 'iface_*' function. It may be cleaner to stick with same, i.e.: iface_set_ingress_sched(iface_cfg, netdev); static int iface_set_ingress_sched(const struct ovsrec_interface *iface_cfg, struct netdev *netdev) { netdev_set_ingress_sched(netdev, smap_get(&iface_cfg->other_config, "ingress_sched")); return 0; } > > *ofp_portp = iface_pick_ofport(iface_cfg); > error = ofproto_port_add(br->ofproto, netdev, ofp_portp); >-- >2.7.4
diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c index 8a4cdb3..3943b7b 100644 --- a/lib/netdev-bsd.c +++ b/lib/netdev-bsd.c @@ -1506,6 +1506,7 @@ netdev_bsd_update_flags(struct netdev *netdev_, enum netdev_flags off, netdev_bsd_get_etheraddr, \ netdev_bsd_get_mtu, \ NULL, /* set_mtu */ \ + NULL, /* set_ingress_sched */ \ netdev_bsd_get_ifindex, \ netdev_bsd_get_carrier, \ NULL, /* get_carrier_resets */ \ diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 1aaf6f7..1ffedd4 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -390,6 +390,9 @@ struct netdev_dpdk { /* If true, device was attached by rte_eth_dev_attach(). */ bool attached; + /* Ingress Scheduling config */ + char *ingress_sched_str; + /* In dpdk_list. */ struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex); @@ -1081,6 +1084,9 @@ netdev_dpdk_destruct(struct netdev *netdev) } free(dev->devargs); + if (dev->ingress_sched_str) { + free(dev->ingress_sched_str); + } common_destruct(dev); ovs_mutex_unlock(&dpdk_mutex); @@ -2004,6 +2010,20 @@ netdev_dpdk_set_mtu(struct netdev *netdev, int mtu) } static int +netdev_dpdk_set_ingress_sched(struct netdev *netdev, + const char *ingress_sched_str) +{ + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); + + free(dev->ingress_sched_str); + if (ingress_sched_str) { + dev->ingress_sched_str = xstrdup(ingress_sched_str); + } + + return 0; +} + +static int netdev_dpdk_get_carrier(const struct netdev *netdev, bool *carrier); static int @@ -3342,6 +3362,7 @@ unlock: netdev_dpdk_get_etheraddr, \ netdev_dpdk_get_mtu, \ netdev_dpdk_set_mtu, \ + netdev_dpdk_set_ingress_sched, \ netdev_dpdk_get_ifindex, \ GET_CARRIER, \ netdev_dpdk_get_carrier_resets, \ diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c index f731af1..0c36d2d 100644 --- a/lib/netdev-dummy.c +++ b/lib/netdev-dummy.c @@ -1378,6 +1378,7 @@ netdev_dummy_update_flags(struct netdev *netdev_, netdev_dummy_get_etheraddr, \ netdev_dummy_get_mtu, \ netdev_dummy_set_mtu, \ + NULL, /* set_ingress_sched */ \ netdev_dummy_get_ifindex, \ NULL, /* get_carrier */ \ NULL, /* get_carrier_resets */ \ diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 2ff3e2b..00cbe17 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -2847,6 +2847,7 @@ netdev_linux_update_flags(struct netdev *netdev_, enum netdev_flags off, netdev_linux_get_etheraddr, \ netdev_linux_get_mtu, \ netdev_linux_set_mtu, \ + NULL, /* set_ingress_sched */ \ netdev_linux_get_ifindex, \ netdev_linux_get_carrier, \ netdev_linux_get_carrier_resets, \ diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h index b3c57d5..6cbd066 100644 --- a/lib/netdev-provider.h +++ b/lib/netdev-provider.h @@ -34,6 +34,7 @@ extern "C" { struct netdev_tnl_build_header_params; #define NETDEV_NUMA_UNSPEC OVS_NUMA_UNSPEC +#define NETDEV_PRIO_RXQ_UNSPEC (-1) /* A network device (e.g. an Ethernet device). * @@ -76,6 +77,7 @@ struct netdev { * modify them. */ int n_txq; int n_rxq; + int priority_rxq; /* id of prioritized rxq. -1 = None */ int ref_cnt; /* Times this devices was opened. */ struct shash_node *node; /* Pointer to element in global map. */ struct ovs_list saved_flags_list; /* Contains "struct netdev_saved_flags". */ @@ -412,6 +414,14 @@ struct netdev_class { * null if it would always return EOPNOTSUPP. */ int (*set_mtu)(struct netdev *netdev, int mtu); + /* Sets 'netdev''s ingress scheduling policy. + * + * If 'netdev' does not support the specified policy then + * this function should return EOPNOTSUPP. This function may be set to + * null if it would always return EOPNOTSUPP. */ + int (*set_ingress_sched)(struct netdev *netdev, + const char *ingress_sched_str); + /* Returns the ifindex of 'netdev', if successful, as a positive number. * On failure, returns a negative errno value. * diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index d11c5cc..57a9b01 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -901,6 +901,7 @@ netdev_vport_get_ifindex(const struct netdev *netdev_) netdev_vport_get_etheraddr, \ NULL, /* get_mtu */ \ NULL, /* set_mtu */ \ + NULL, /* set_ingress_sched */ \ GET_IFINDEX, \ NULL, /* get_carrier */ \ NULL, /* get_carrier_resets */ \ diff --git a/lib/netdev.c b/lib/netdev.c index b4e570b..4562fdf 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -418,6 +418,7 @@ netdev_open(const char *name, const char *type, struct netdev **netdevp) /* By default enable one tx and rx queue per netdev. */ netdev->n_txq = netdev->netdev_class->send ? 1 : 0; netdev->n_rxq = netdev->netdev_class->rxq_alloc ? 1 : 0; + netdev->priority_rxq = NETDEV_PRIO_RXQ_UNSPEC; ovs_list_init(&netdev->saved_flags_list); @@ -978,6 +979,27 @@ netdev_mtu_is_user_config(struct netdev *netdev) return netdev->mtu_user_config; } +/* Sets the Ingress Scheduling policy of 'netdev'. + * + * If successful, returns 0. Returns EOPNOTSUPP if 'netdev' does not support + * the specified policy */ +int +netdev_set_ingress_sched(struct netdev *netdev, const char *ingress_sched_str) +{ + const struct netdev_class *class = netdev->netdev_class; + int error; + + error = class->set_ingress_sched ? + class->set_ingress_sched(netdev, ingress_sched_str) : EOPNOTSUPP; + if (error && error != EOPNOTSUPP) { + VLOG_DBG_RL(&rl, "failed to set ingress scheduling for network " \ + "device %s: %s", + netdev_get_name(netdev), ovs_strerror(error)); + } + + return error; +} + /* Returns the ifindex of 'netdev', if successful, as a positive number. On * failure, returns a negative errno value. * diff --git a/lib/netdev.h b/lib/netdev.h index f8482f7..b65085c 100644 --- a/lib/netdev.h +++ b/lib/netdev.h @@ -164,6 +164,7 @@ int netdev_get_mtu(const struct netdev *, int *mtup); int netdev_set_mtu(struct netdev *, int mtu); void netdev_mtu_user_config(struct netdev *, bool); bool netdev_mtu_is_user_config(struct netdev *); +int netdev_set_ingress_sched(struct netdev *, const char *ingress_sched_str); int netdev_get_ifindex(const struct netdev *); int netdev_set_tx_multiq(struct netdev *, unsigned int n_txq); enum netdev_pt_mode netdev_get_pt_mode(const struct netdev *); diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index a8cbae7..93c91f6 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -1794,6 +1794,8 @@ iface_do_create(const struct bridge *br, } iface_set_netdev_mtu(iface_cfg, netdev); + netdev_set_ingress_sched(netdev, + smap_get(&iface_cfg->other_config, "ingress_sched")); *ofp_portp = iface_pick_ofport(iface_cfg); error = ofproto_port_add(br->ofproto, netdev, ofp_portp);
Passes ingress_sched config item from other_config column of Interface table to the netdev. Signed-off-by: Billy O'Mahony <billy.o.mahony@intel.com> --- lib/netdev-bsd.c | 1 + lib/netdev-dpdk.c | 21 +++++++++++++++++++++ lib/netdev-dummy.c | 1 + lib/netdev-linux.c | 1 + lib/netdev-provider.h | 10 ++++++++++ lib/netdev-vport.c | 1 + lib/netdev.c | 22 ++++++++++++++++++++++ lib/netdev.h | 1 + vswitchd/bridge.c | 2 ++ 9 files changed, 60 insertions(+)