>From: O Mahony, Billy
>Sent: Thursday, August 17, 2017 3:24 PM
>To: [email protected]
>Cc: Kavanagh, Mark B <[email protected]>; O Mahony, Billy
><[email protected]>
>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 <[email protected]>
>---
> 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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
