> -----Original Message----- > From: [email protected] [mailto:ovs-dev- > [email protected]] On Behalf Of Róbert Mulik > Sent: Friday, January 19, 2018 12:44 PM > To: [email protected] > Subject: [ovs-dev] [PATCH v2] netdev-dpdk: Configurable Link State Change > (LSC) detection mode >
Thanks for working on this Robert, This isn't a full review, just some comment on first observation for a v3. I should have some more time next week to review. > It is possible to change LSC detection mode to polling or interrupt mode > for DPDK interfaces. The default is polling mode. To set interrupt mode, > option dpdk-lsc-interrupt has to be set to true. For the commit message it might be good to explain why someone would like to use interrupt over pmd mode. > > Global settings > Service restart is necessary for the global settings to take effect. > > Command to set interrupt mode for all interfaces: > ovs-vsctl set Open_vSwitch . other_config:dpdk-lsc-interrupt=true > > Command to set polling mode for all interfaces: > ovs-vsctl set Open_vSwitch . other_config:dpdk-lsc-interrupt=false > or: > ovs-vsctl remove Open_vSwitch . other_config dpdk-lsc-interrupt > > Interface specific settings (override global settings) Service restart is > not necessary to take effect. > > Command to set interrupt mode for a specific interface: > ovs-vsctl set interface <interface_name> options:dpdk-lsc-interrupt=true > > Command to set polling mode for a specific interface: > ovs-vsctl set interface <interface_name> options:dpdk-lsc-interrupt=false > > Command to reset to globally defined mode for a specific interface: > ovs-vsctl remove interface <interface_name> options dpdk-lsc-interrupt I would expect the above commands to be included in the OVS DPDK documentation rather than the commit message. Also an explanation of why LSC would be preferred over PMD preceding the usage above would be useful in the docs so users are aware of the pros/cons. > > Signed-off-by: Robert Mulik <[email protected]> > --- > lib/dpdk.c | 8 ++++++++ > lib/netdev-dpdk.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++- > lib/netdev-dpdk.h | 9 +++++++++ > vswitchd/vswitch.xml | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 109 insertions(+), 1 deletion(-) > > diff --git a/lib/dpdk.c b/lib/dpdk.c > index 6710d10..28687eb 100644 > --- a/lib/dpdk.c > +++ b/lib/dpdk.c > @@ -351,6 +351,14 @@ dpdk_init__(const struct smap *ovs_other_config) > VLOG_INFO("IOMMU support for vhost-user-client %s.", > vhost_iommu_enabled ? "enabled" : "disabled"); > > + if (smap_get_bool(ovs_other_config, "dpdk-lsc-interrupt", false)) { > + netdev_dpdk_set_default_lsc_detect_mode( > + NETDEV_DPDK_LSC_DETECT_INTERRUPT_MODE); > + } else { > + netdev_dpdk_set_default_lsc_detect_mode( > + NETDEV_DPDK_LSC_DETECT_POLL_MODE); > + } > + > argv = grow_argv(&argv, 0, 1); > argc = 1; > argv[0] = xstrdup(ovs_get_program_name()); diff --git a/lib/netdev- > dpdk.c b/lib/netdev-dpdk.c index e32c7f6..cb52222 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -148,7 +148,7 @@ typedef uint16_t dpdk_port_t; #define > VHOST_ENQ_RETRY_NUM 8 #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX > : IFNAMSIZ) > > -static const struct rte_eth_conf port_conf = { > +static struct rte_eth_conf port_conf = { > .rxmode = { > .mq_mode = ETH_MQ_RX_RSS, > .split_hdr_size = 0, > @@ -167,6 +167,10 @@ static const struct rte_eth_conf port_conf = { > .txmode = { > .mq_mode = ETH_MQ_TX_NONE, > }, > + .intr_conf = { > + /* LSC interrupt mode disabled, polling mode used. */ > + .lsc = (uint16_t)NETDEV_DPDK_LSC_DETECT_POLL_MODE, > + }, > }; > > /* > @@ -416,6 +420,9 @@ struct netdev_dpdk { > int requested_n_rxq; > int requested_rxq_size; > int requested_txq_size; > + enum netdev_dpdk_lsc_detect_mode requested_lsc_detect_mode; > + White space here. Don't think it's needed. > + enum netdev_dpdk_lsc_detect_mode lsc_detect_mode; > > /* Number of rx/tx descriptors for physical devices */ > int rxq_size; > @@ -457,6 +464,19 @@ int netdev_dpdk_get_vid(const struct netdev_dpdk > *dev); struct ingress_policer * netdev_dpdk_get_ingress_policer(const > struct netdev_dpdk *dev); > > +void > +netdev_dpdk_set_default_lsc_detect_mode( > + enum netdev_dpdk_lsc_detect_mode default_lsc) { > + port_conf.intr_conf.lsc = (uint16_t)default_lsc; } > + > +enum netdev_dpdk_lsc_detect_mode > +netdev_dpdk_get_default_lsc_detect_mode(void) > +{ > + return port_conf.intr_conf.lsc; > +} > + > static bool > is_dpdk_class(const struct netdev_class *class) { @@ -690,6 +710,8 @@ > dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int n_txq) > int i; > struct rte_eth_conf conf = port_conf; > > + conf.intr_conf.lsc = dev->lsc_detect_mode; > + > /* For some NICs (e.g. Niantic), scatter_rx mode needs to be > explicitly > * enabled. */ > if (dev->mtu > ETHER_MTU) { > @@ -895,6 +917,7 @@ common_construct(struct netdev *netdev, dpdk_port_t > port_no, > dev->flags = 0; > dev->requested_mtu = ETHER_MTU; > dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu); > + dev->requested_lsc_detect_mode = > + netdev_dpdk_get_default_lsc_detect_mode(); > ovsrcu_index_init(&dev->vid, -1); > dev->vhost_reconfigured = false; > dev->attached = false; > @@ -1469,6 +1492,25 @@ netdev_dpdk_set_config(struct netdev *netdev, const > struct smap *args, > "The old 'dpdk<port_id>' names are not > supported", > netdev_get_name(netdev)); > err = EINVAL; > + goto out; > + } > + > + struct smap_node *node = smap_get_node(args, "dpdk-lsc-interrupt"); > + uint16_t lsc; > + > + if (node) { > + lsc = smap_get_bool(args, "dpdk-lsc-interrupt", false); > + } else { > + lsc = netdev_dpdk_get_default_lsc_detect_mode(); > + } > + if (lsc > NETDEV_DPDK_LSC_DETECT_MAX) { > + VLOG_WARN("%s: invalid LSC value %d\n", dev->up.name, lsc); > + err = EINVAL; > + goto out; > + } else if (dev->requested_lsc_detect_mode != > + (enum netdev_dpdk_lsc_detect_mode)lsc) { > + dev->requested_lsc_detect_mode = (enum > netdev_dpdk_lsc_detect_mode)lsc; > + netdev_request_reconfigure(netdev); > } > > if (err) { > @@ -3423,6 +3465,7 @@ netdev_dpdk_reconfigure(struct netdev *netdev) > if (netdev->n_txq == dev->requested_n_txq > && netdev->n_rxq == dev->requested_n_rxq > && dev->mtu == dev->requested_mtu > + && dev->lsc_detect_mode == dev->requested_lsc_detect_mode > && dev->rxq_size == dev->requested_rxq_size > && dev->txq_size == dev->requested_txq_size > && dev->socket_id == dev->requested_socket_id) { @@ -3438,6 > +3481,8 @@ netdev_dpdk_reconfigure(struct netdev *netdev) > goto out; > } > > + dev->lsc_detect_mode = dev->requested_lsc_detect_mode; > + > netdev->n_txq = dev->requested_n_txq; > netdev->n_rxq = dev->requested_n_rxq; > > diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h index b7d02a7..93fdd1d > 100644 > --- a/lib/netdev-dpdk.h > +++ b/lib/netdev-dpdk.h > @@ -25,8 +25,17 @@ struct dp_packet; > > #ifdef DPDK_NETDEV > > +enum netdev_dpdk_lsc_detect_mode { > + NETDEV_DPDK_LSC_DETECT_POLL_MODE, > + NETDEV_DPDK_LSC_DETECT_INTERRUPT_MODE, > + NETDEV_DPDK_LSC_DETECT_MAX = NETDEV_DPDK_LSC_DETECT_INTERRUPT_MODE, > +}; > + > void netdev_dpdk_register(void); > void free_dpdk_buf(struct dp_packet *); > +void netdev_dpdk_set_default_lsc_detect_mode( > + enum netdev_dpdk_lsc_detect_mode default_lsc); enum > +netdev_dpdk_lsc_detect_mode > +netdev_dpdk_get_default_lsc_detect_mode(void); > > #else > > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index > 58c0ebd..a4a0150 100644 > --- a/vswitchd/vswitch.xml > +++ b/vswitchd/vswitch.xml > @@ -320,6 +320,30 @@ > </p> > </column> > > + <column name="other_config" key="dpdk-lsc-interrupt" > + type='{"type": "boolean"}'> > + <p> > + Set this value to <code>true</code> to set interrupt mode for > Link > + State Change (LSC) detection instead of poll mode for DPDK > + interfaces. > + </p> > + <p> > + The default value is <code>false</code>. Changing this value > requires > + restarting the daemon > + </p> > + <p> > + If this value is <code>false</code> at startup, poll mode is > used for > + all netdev dpdk interfaces. > + </p> > + <p> > + This value can be overridden for a single interface in the > options > + section of that interface. > + </p> > + <p> > + This parameter has any effect only on netdev dpdk interfaces. > + </p> > + </column> > + > <column name="other_config" key="dpdk-extra" > type='{"type": "string"}'> > <p> > @@ -3593,6 +3617,28 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 > type=patch options:peer=p1 \ > </column> > </group> > > + <group title="Link State Change detection mode"> > + <column name="options" key="dpdk-lsc-interrupt" > + type='{"type": "boolean"}'> > + <p> > + Set this value to <code>true</code> to set interrupt mode for > Link > + State Change (LSC) detection instead of poll mode for the DPDK > + interface. > + </p> > + <p> > + If this value is not set, the value is taken from the global > + settings. > + </p> > + <p> > + If this value is set, the global LSC interrupt settings are > + overridden. > + </p> > + <p> > + This parameter has any effect only on netdev dpdk interfaces. > + </p> > + </column> > + </group> > + > <group title="Common Columns"> > The overall purpose of these columns is described under > <code>Common > Columns</code> at the beginning of this document. > -- > 1.9.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
