Discussion:
[dpdk-dev] [PATCH v4 0/3] net/i40e: vf port reset
Wei Zhao
2017-03-30 09:21:43 UTC
Permalink
The patches mainly finish following functions:
1) get pf reset vf comamand falg from interrupt server function.
2) reset vf port from testpmd app using a command.
3) sore and restore vf related parameters.

v2:
-change the order of patch in the set.
-add more details in patch comment to clarify that
the rte and tespmd patch can also used in pf port reset.
-add rte_free for memory free after restore.
-change varible name of reset_number to reset_falg.
-fix patch check warning.

v3:
-fix error of author mail address

v4:
-fix typo error
-add rte_wmb() after change the reset_ports.

zhao wei (3):
app/testpmd: add port reset command into testpmd
lib/librte_ether: add support for port reset
net/i40e: implement device reset on port

app/test-pmd/cmdline.c | 17 ++-
app/test-pmd/testpmd.c | 65 ++++++++++++
app/test-pmd/testpmd.h | 1 +
drivers/net/i40e/i40e_ethdev.c | 2 +-
drivers/net/i40e/i40e_ethdev.h | 16 +++
drivers/net/i40e/i40e_ethdev_vf.c | 185 +++++++++++++++++++++++++++++++++
drivers/net/i40e/i40e_rxtx.c | 10 ++
drivers/net/i40e/i40e_rxtx.h | 4 +
lib/librte_ether/rte_ethdev.c | 17 +++
lib/librte_ether/rte_ethdev.h | 25 +++++
lib/librte_ether/rte_ether_version.map | 6 ++
11 files changed, 343 insertions(+), 5 deletions(-)
--
2.9.3
Wei Zhao
2017-03-30 09:21:44 UTC
Permalink
Add support for port reset in rte layer.This reset
feature can not only used in vf port reset in later code
develop, but alsopf port.But in this patch set, we only
limit the discussion scope to vf reset.
This patch Add an API to restart the device.
It's for VF device in this scenario, kernel PF + DPDK VF.
When the PF port down->up, APP should call this API to
restart VF port. Most likely, APP should call it in its
management thread and guarantee the thread safe. It means
APP should stop the rx/tx and the device, then restart
the device, then recover the device and rx/tx.
Also, it's APP's responsibilty to release the occupied
memory.

Signed-off-by: Wenzhuo Lu <***@intel.com>
Signed-off-by: Wei Zhao <***@intel.com>
---
lib/librte_ether/rte_ethdev.c | 17 +++++++++++++++++
lib/librte_ether/rte_ethdev.h | 25 +++++++++++++++++++++++++
lib/librte_ether/rte_ether_version.map | 6 ++++++
3 files changed, 48 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index eb0a94a..2e06dca 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3273,3 +3273,20 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
-ENOTSUP);
return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel, mask, en);
}
+
+int
+rte_eth_dev_reset(uint8_t port_id)
+{
+ struct rte_eth_dev *dev;
+ int diag;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+ dev = &rte_eth_devices[port_id];
+
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -ENOTSUP);
+
+ diag = (*dev->dev_ops->dev_reset)(dev);
+
+ return diag;
+}
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 4be217c..34412c0 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1367,6 +1367,9 @@ typedef int (*eth_l2_tunnel_offload_set_t)
uint8_t en);
/**< @internal enable/disable the l2 tunnel offload functions */

+typedef int (*eth_dev_reset_t)(struct rte_eth_dev *dev);
+/**< @internal Function used to reset a configured Ethernet device. */
+
#ifdef RTE_NIC_BYPASS

enum {
@@ -1509,6 +1512,9 @@ struct eth_dev_ops {
eth_l2_tunnel_offload_set_t l2_tunnel_offload_set;
/** Enable/disable l2 tunnel offload functions. */

+ /** Reset device. */
+ eth_dev_reset_t dev_reset;
+
eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue rate limit. */

rss_hash_update_t rss_hash_update; /** Configure RSS hash protocols. */
@@ -4413,6 +4419,25 @@ int
rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);

/**
+ * Reset an ethernet device when it's not working. One scenario is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup the rx/tx
+ * queues, restart the port.
+ * Before calling this API, APP should stop the rx/tx. When tx is being stopped,
+ * APP can drop the packets and release the buffer instead of sending them.
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ *
+ * @return
+ * - (0) if successful.
+ * - (-ENODEV) if port identifier is invalid.
+ * - (-ENOTSUP) if hardware doesn't support this function.
+ */
+int
+rte_eth_dev_reset(uint8_t port_id);
+
+/**
* @internal
* Wrapper for use by pci drivers as a .probe function to attach to a ethdev
* interface.
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index c6c9d0d..529b27f 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -154,3 +154,9 @@ DPDK_17.02 {
rte_flow_validate;

} DPDK_16.11;
+
+DPDK_17.05 {
+ global:
+
+ rte_eth_dev_reset;
+} DPDK_17.02;
\ No newline at end of file
--
2.9.3
Wei Zhao
2017-03-30 09:21:45 UTC
Permalink
This patch implement the device reset function on vf port.
This restart function will detach device then
attach device, reconfigure dev, re-setup
the Rx/Tx queues.

Signed-off-by: Wei Zhao <***@intel.com>
Signed-off-by: Wenzhuo Lu <***@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 2 +-
drivers/net/i40e/i40e_ethdev.h | 16 ++++
drivers/net/i40e/i40e_ethdev_vf.c | 185 ++++++++++++++++++++++++++++++++++++++
drivers/net/i40e/i40e_rxtx.c | 10 +++
drivers/net/i40e/i40e_rxtx.h | 4 +
5 files changed, 216 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 3702214..f2fdb2f 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -6020,7 +6020,7 @@ i40e_find_vlan_filter(struct i40e_vsi *vsi,
return 0;
}

-static void
+void
i40e_store_vlan_filter(struct i40e_vsi *vsi,
uint16_t vlan_id, bool on)
{
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index aebb097..515a288 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -652,6 +652,16 @@ struct i40e_vf_tx_queues {
uint32_t tx_ring_len;
};

+struct i40e_vf_reset_store {
+ struct ether_addr *mac_addrs; /* Device Ethernet Link address. */
+ bool promisc_unicast_enabled;
+ bool promisc_multicast_enabled;
+ uint16_t vlan_num; /* Total VLAN number */
+ uint32_t vfta[I40E_VFTA_SIZE]; /* VLAN bitmap */
+ uint16_t mac_num; /* Total mac number */
+};
+
+
/*
* Structure to store private data specific for VF instance.
*/
@@ -709,6 +719,10 @@ struct i40e_adapter {
struct rte_timecounter systime_tc;
struct rte_timecounter rx_tstamp_tc;
struct rte_timecounter tx_tstamp_tc;
+
+ /* For port reset */
+ volatile uint8_t reset_flag;
+ void *reset_store_data;
};

extern const struct rte_flow_ops i40e_flow_ops;
@@ -749,6 +763,8 @@ int i40e_dev_link_update(struct rte_eth_dev *dev,
__rte_unused int wait_to_complete);
void i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi);
void i40e_vsi_queues_unbind_intr(struct i40e_vsi *vsi);
+void i40e_store_vlan_filter(struct i40e_vsi *vsi,
+ uint16_t vlan_id, bool on);
int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
struct i40e_vsi_vlan_pvid_info *info);
int i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 55fd344..9ab035c 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -161,6 +161,9 @@ i40evf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id);
static void i40evf_handle_pf_event(__rte_unused struct rte_eth_dev *dev,
uint8_t *msg,
uint16_t msglen);
+static int i40evf_dev_uninit(struct rte_eth_dev *eth_dev);
+static int i40evf_dev_init(struct rte_eth_dev *eth_dev);
+static int i40evf_reset_dev(struct rte_eth_dev *dev);

/* Default hash key buffer for RSS */
static uint32_t rss_key_default[I40E_VFQF_HKEY_MAX_INDEX + 1];
@@ -230,6 +233,7 @@ static const struct eth_dev_ops i40evf_eth_dev_ops = {
.rss_hash_conf_get = i40evf_dev_rss_hash_conf_get,
.mtu_set = i40evf_dev_mtu_set,
.mac_addr_set = i40evf_set_default_mac_addr,
+ .dev_reset = i40evf_reset_dev,
};

/*
@@ -889,6 +893,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
PMD_DRV_LOG(ERR, "fail to execute command "
"OP_ADD_ETHER_ADDRESS");

+ vf->vsi.mac_num++;
return;
}

@@ -926,6 +931,7 @@ i40evf_del_mac_addr_by_addr(struct rte_eth_dev *dev,
if (err)
PMD_DRV_LOG(ERR, "fail to execute command "
"OP_DEL_ETHER_ADDRESS");
+ vf->vsi.mac_num--;
return;
}

@@ -1047,6 +1053,7 @@ static int
i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
{
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+ struct i40e_vsi *vsi = &vf->vsi;
struct i40e_virtchnl_vlan_filter_list *vlan_list;
uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
sizeof(uint16_t)];
@@ -1066,6 +1073,8 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
err = i40evf_execute_vf_cmd(dev, &args);
if (err)
PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+ i40e_store_vlan_filter(vsi, vlanid, 1);
+ vsi->vlan_num++;

return err;
}
@@ -1074,6 +1083,7 @@ static int
i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
{
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+ struct i40e_vsi *vsi = &vf->vsi;
struct i40e_virtchnl_vlan_filter_list *vlan_list;
uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
sizeof(uint16_t)];
@@ -1093,6 +1103,8 @@ i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
err = i40evf_execute_vf_cmd(dev, &args);
if (err)
PMD_DRV_LOG(ERR, "fail to execute command OP_DEL_VLAN");
+ i40e_store_vlan_filter(vsi, vlanid, 0);
+ vsi->vlan_num--;

return err;
}
@@ -2716,3 +2728,176 @@ i40evf_set_default_mac_addr(struct rte_eth_dev *dev,

i40evf_add_mac_addr(dev, mac_addr, 0, 0);
}
+
+static void
+i40evf_restore_vlan_filter(struct rte_eth_dev *dev,
+ uint32_t *vfta)
+{
+ uint32_t vid_idx, vid_bit;
+ uint16_t vlan_id;
+
+ for (vid_idx = 0; vid_idx < I40E_VFTA_SIZE; vid_idx++) {
+ for (vid_bit = 0; vid_bit < I40E_UINT32_BIT_SIZE; vid_bit++) {
+ if (vfta[vid_idx] & (1 << vid_bit)) {
+ vlan_id = (vid_idx << 5) + vid_bit;
+ i40evf_add_vlan(dev, vlan_id);
+ }
+ }
+ }
+}
+
+static void
+i40evf_restore_macaddr(struct rte_eth_dev *dev,
+ struct ether_addr *mac_addrs)
+{
+ struct ether_addr *addr;
+ uint16_t i;
+
+ /* replay MAC address configuration including default MAC */
+ addr = &mac_addrs[0];
+
+ i40evf_set_default_mac_addr(dev, addr);
+ memcpy(dev->data->mac_addrs, mac_addrs,
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+ for (i = 1; i < I40E_NUM_MACADDR_MAX; i++) {
+ addr = &mac_addrs[i];
+
+ /* skip zero address */
+ if (is_zero_ether_addr(addr))
+ continue;
+
+ i40evf_add_mac_addr(dev, addr, 0, 0);
+ }
+}
+
+
+static void
+i40e_vf_queue_reset(struct rte_eth_dev *dev)
+{
+ uint16_t i;
+
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ struct i40e_rx_queue *rxq = dev->data->rx_queues[i];
+
+ if (rxq->q_set) {
+ i40e_dev_rx_queue_setup(dev,
+ rxq->queue_id,
+ rxq->nb_rx_desc,
+ rxq->socket_id,
+ &rxq->rxconf,
+ rxq->mp);
+ }
+ }
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ struct i40e_tx_queue *txq = dev->data->tx_queues[i];
+
+ if (txq->q_set) {
+ i40e_dev_tx_queue_setup(dev,
+ txq->queue_id,
+ txq->nb_tx_desc,
+ txq->socket_id,
+ &txq->txconf);
+ }
+ }
+}
+
+static int
+i40evf_store_before_reset(struct rte_eth_dev *dev)
+{
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+ struct i40e_vf_reset_store *store_data;
+ struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+
+ adapter->reset_store_data = rte_zmalloc("i40evf_store_reset",
+ sizeof(struct i40e_vf_reset_store), 0);
+ if (adapter->reset_store_data == NULL) {
+ PMD_INIT_LOG(ERR, "Failed to allocate %ld bytes needed to"
+ " to store data when reset vf",
+ sizeof(struct i40e_vf_reset_store));
+ return -ENOMEM;
+ }
+ store_data =
+ (struct i40e_vf_reset_store *)adapter->reset_store_data;
+ store_data->mac_addrs = rte_zmalloc("i40evf_mac_store_reset",
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX, 0);
+ if (store_data->mac_addrs == NULL) {
+ PMD_INIT_LOG(ERR, "Failed to allocate %d bytes needed to"
+ " to store MAC addresses when reset vf",
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+ }
+
+ memcpy(store_data->mac_addrs, dev->data->mac_addrs,
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+ store_data->promisc_unicast_enabled = vf->promisc_unicast_enabled;
+ store_data->promisc_multicast_enabled = vf->promisc_multicast_enabled;
+
+ store_data->vlan_num = vf->vsi.vlan_num;
+ memcpy(store_data->vfta, vf->vsi.vfta,
+ sizeof(uint32_t) * I40E_VFTA_SIZE);
+
+ store_data->mac_num = vf->vsi.mac_num;
+
+ return 0;
+}
+
+static void
+i40evf_restore_after_reset(struct rte_eth_dev *dev)
+{
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+ struct i40e_vf_reset_store *store_data =
+ (struct i40e_vf_reset_store *)adapter->reset_store_data;
+
+ if (store_data->promisc_unicast_enabled)
+ i40evf_dev_promiscuous_enable(dev);
+ if (store_data->promisc_multicast_enabled)
+ i40evf_dev_allmulticast_enable(dev);
+
+ if (store_data->vlan_num)
+ i40evf_restore_vlan_filter(dev, store_data->vfta);
+
+ if (store_data->mac_num)
+ i40evf_restore_macaddr(dev, store_data->mac_addrs);
+
+ rte_free(store_data->mac_addrs);
+ rte_free(adapter->reset_store_data);
+}
+
+static int
+i40evf_reset_dev(struct rte_eth_dev *dev)
+{
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+
+ adapter->reset_flag = 1;
+ i40evf_store_before_reset(dev);
+
+ i40evf_dev_close(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev close complete");
+
+ i40evf_dev_uninit(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev detached");
+
+ memset(dev->data->dev_private, 0,
+ (uint64_t)&adapter->reset_flag - (uint64_t)adapter);
+
+ i40evf_dev_init(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev attached");
+
+ i40evf_dev_configure(dev);
+
+ i40e_vf_queue_reset(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf queue reset");
+
+ i40evf_restore_after_reset(dev);
+
+ i40evf_dev_start(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev restart");
+
+ adapter->reset_flag = 0;
+
+ return 0;
+}
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 02367b7..d891a54 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1709,6 +1709,7 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
uint16_t len, i;
uint16_t base, bsf, tc_mapping;
int use_def_burst_func = 1;
+ struct rte_eth_rxconf conf = *rx_conf;

if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
struct i40e_vf *vf =
@@ -1748,6 +1749,8 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
}
rxq->mp = mp;
rxq->nb_rx_desc = nb_desc;
+ rxq->socket_id = socket_id;
+ rxq->rxconf = conf;
rxq->rx_free_thresh = rx_conf->rx_free_thresh;
rxq->queue_id = queue_idx;
if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF)
@@ -1932,6 +1935,7 @@ i40e_dev_tx_queue_setup(struct rte_eth_dev *dev,
uint32_t ring_size;
uint16_t tx_rs_thresh, tx_free_thresh;
uint16_t i, base, bsf, tc_mapping;
+ struct rte_eth_txconf conf = *tx_conf;

if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
struct i40e_vf *vf =
@@ -2054,6 +2058,8 @@ i40e_dev_tx_queue_setup(struct rte_eth_dev *dev,
}

txq->nb_tx_desc = nb_desc;
+ txq->socket_id = socket_id;
+ txq->txconf = conf;
txq->tx_rs_thresh = tx_rs_thresh;
txq->tx_free_thresh = tx_free_thresh;
txq->pthresh = tx_conf->tx_thresh.pthresh;
@@ -2520,8 +2526,12 @@ void
i40e_dev_free_queues(struct rte_eth_dev *dev)
{
uint16_t i;
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);

PMD_INIT_FUNC_TRACE();
+ if (adapter->reset_flag)
+ return;

for (i = 0; i < dev->data->nb_rx_queues; i++) {
if (!dev->data->rx_queues[i])
diff --git a/drivers/net/i40e/i40e_rxtx.h b/drivers/net/i40e/i40e_rxtx.h
index a87bdb0..0e3cc19 100644
--- a/drivers/net/i40e/i40e_rxtx.h
+++ b/drivers/net/i40e/i40e_rxtx.h
@@ -136,6 +136,8 @@ struct i40e_rx_queue {
bool rx_deferred_start; /**< don't start this queue in dev start */
uint16_t rx_using_sse; /**<flag indicate the usage of vPMD for rx */
uint8_t dcb_tc; /**< Traffic class of rx queue */
+ uint8_t socket_id;
+ struct rte_eth_rxconf rxconf;
};

struct i40e_tx_entry {
@@ -177,6 +179,8 @@ struct i40e_tx_queue {
bool q_set; /**< indicate if tx queue has been configured */
bool tx_deferred_start; /**< don't start this queue in dev start */
uint8_t dcb_tc; /**< Traffic class of tx queue */
+ uint8_t socket_id;
+ struct rte_eth_txconf txconf;
};

/** Offload features */
--
2.9.3
Wei Zhao
2017-03-30 09:21:46 UTC
Permalink
Add port reset command into testpmd project,
it is the interface for user to reset port.
And also it is not limit to be used only in vf reset,
but also for pf port reset.But this patch set only
realted to vf reset feature.

Signed-off-by: Wei Zhao <***@intel.com>
Signed-off-by: Wenzhuo Lu <***@intel.com>
---
app/test-pmd/cmdline.c | 17 +++++++++----
app/test-pmd/testpmd.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
app/test-pmd/testpmd.h | 1 +
3 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 47f935d..6cbdcc8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -599,6 +599,9 @@ static void cmd_help_long_parsed(void *parsed_result,
"port close (port_id|all)\n"
" Close all ports or port_id.\n\n"

+ "port reset (port_id|all)\n"
+ " Reset all ports or port_id.\n\n"
+
"port attach (ident)\n"
" Attach physical or virtual dev by pci address or virtual device name\n\n"

@@ -909,6 +912,8 @@ static void cmd_operate_port_parsed(void *parsed_result,
stop_port(RTE_PORT_ALL);
else if (!strcmp(res->name, "close"))
close_port(RTE_PORT_ALL);
+ else if (!strcmp(res->name, "reset"))
+ reset_port(RTE_PORT_ALL);
else
printf("Unknown parameter\n");
}
@@ -918,14 +923,15 @@ cmdline_parse_token_string_t cmd_operate_port_all_cmd =
"port");
cmdline_parse_token_string_t cmd_operate_port_all_port =
TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, name,
- "start#stop#close");
+ "start#stop#close#reset");
cmdline_parse_token_string_t cmd_operate_port_all_all =
TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, value, "all");

cmdline_parse_inst_t cmd_operate_port = {
.f = cmd_operate_port_parsed,
.data = NULL,
- .help_str = "port start|stop|close all: Start/Stop/Close all ports",
+ .help_str = "port start|stop|close|reset all: Start/Stop/Close/"
+ "Reset all ports",
.tokens = {
(void *)&cmd_operate_port_all_cmd,
(void *)&cmd_operate_port_all_port,
@@ -953,6 +959,8 @@ static void cmd_operate_specific_port_parsed(void *parsed_result,
stop_port(res->value);
else if (!strcmp(res->name, "close"))
close_port(res->value);
+ else if (!strcmp(res->name, "reset"))
+ reset_port(res->value);
else
printf("Unknown parameter\n");
}
@@ -962,7 +970,7 @@ cmdline_parse_token_string_t cmd_operate_specific_port_cmd =
keyword, "port");
cmdline_parse_token_string_t cmd_operate_specific_port_port =
TOKEN_STRING_INITIALIZER(struct cmd_operate_specific_port_result,
- name, "start#stop#close");
+ name, "start#stop#close#reset");
cmdline_parse_token_num_t cmd_operate_specific_port_id =
TOKEN_NUM_INITIALIZER(struct cmd_operate_specific_port_result,
value, UINT8);
@@ -970,7 +978,8 @@ cmdline_parse_token_num_t cmd_operate_specific_port_id =
cmdline_parse_inst_t cmd_operate_specific_port = {
.f = cmd_operate_specific_port_parsed,
.data = NULL,
- .help_str = "port start|stop|close <port_id>: Start/Stop/Close port_id",
+ .help_str = "port start|stop|close|reset <port_id>: Start/Stop/Close/"
+ "Reset port_id",
.tokens = {
(void *)&cmd_operate_specific_port_cmd,
(void *)&cmd_operate_specific_port_port,
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 484c19b..a4f5330 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -137,6 +137,7 @@ portid_t nb_fwd_ports; /**< Number of forwarding ports. */

unsigned int fwd_lcores_cpuids[RTE_MAX_LCORE]; /**< CPU ids configuration. */
portid_t fwd_ports_ids[RTE_MAX_ETHPORTS]; /**< Port ids configuration. */
+volatile char reset_ports[RTE_MAX_ETHPORTS] = {0}; /**< Port reset flag. */

struct fwd_stream **fwd_streams; /**< For each RX queue of each port. */
streamid_t nb_fwd_streams; /**< Is equal to (nb_ports * nb_rxq). */
@@ -1305,6 +1306,18 @@ port_is_closed(portid_t port_id)
return 1;
}

+static void
+reset_event_callback(uint8_t port_id, enum rte_eth_event_type type, void *param)
+{
+ RTE_SET_USED(param);
+
+ printf("Event type: %s on port %d\n",
+ type == RTE_ETH_EVENT_INTR_RESET ? "RESET interrupt" :
+ "unknown event", port_id);
+ reset_ports[port_id] = 1;
+ rte_wmb();
+}
+
int
start_port(portid_t pid)
{
@@ -1350,6 +1363,10 @@ start_port(portid_t pid)
return -1;
}
}
+
+ /* register reset interrupt callback */
+ rte_eth_dev_callback_register(pi, RTE_ETH_EVENT_INTR_RESET,
+ reset_event_callback, NULL);
if (port->need_reconfig_queues > 0) {
port->need_reconfig_queues = 0;
/* setup tx queues */
@@ -1559,6 +1576,54 @@ close_port(portid_t pid)
}

void
+reset_port(portid_t pid)
+{
+ portid_t pi;
+ struct rte_port *port;
+
+ if (port_id_is_invalid(pid, ENABLED_WARN))
+ return;
+
+ printf("Reset ports...\n");
+
+ FOREACH_PORT(pi, ports) {
+ if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
+ continue;
+
+ if (port_is_forwarding(pi) != 0 && test_done == 0) {
+ printf("Please remove port %d from forwarding "
+ "configuration.\n", pi);
+ continue;
+ }
+
+ if (port_is_bonding_slave(pi)) {
+ printf("Please remove port %d from "
+ "bonded device.\n", pi);
+ continue;
+ }
+
+ if (!reset_ports[pi]) {
+ printf("vf must get reset port %d info from "
+ "pf before reset.\n", pi);
+ continue;
+ }
+
+ port = &ports[pi];
+ if (rte_atomic16_cmpset(&(port->port_status),
+ RTE_PORT_STOPPED, RTE_PORT_HANDLING) == 1) {
+ printf("Port %d is not stopped\n", pi);
+ continue;
+ }
+
+ rte_eth_dev_reset(pi);
+ reset_ports[pi] = 0;
+ rte_wmb();
+ }
+
+ printf("Done\n");
+}
+
+void
attach_port(char *identifier)
{
portid_t pi = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 8cf2860..0c7e44c 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -586,6 +586,7 @@ int init_port_dcb_config(portid_t pid, enum dcb_mode_enable dcb_mode,
int start_port(portid_t pid);
void stop_port(portid_t pid);
void close_port(portid_t pid);
+void reset_port(portid_t pid);
void attach_port(char *identifier);
void detach_port(uint8_t port_id);
int all_ports_stopped(void);
--
2.9.3
Wei Zhao
2017-03-30 09:34:13 UTC
Permalink
The patches mainly finish following functions:
1) get pf reset vf comamand falg from interrupt server function.
2) reset vf port from testpmd app using a command.
3) sore and restore vf related parameters.

v2:
-change the order of patch in the set.
-add more details in patch comment to clarify that
the rte and tespmd patch can also used in pf port reset.
-add rte_free for memory free after restore.
-change varible name of reset_number to reset_falg.
-fix patch check warning.

v3:
-fix error of author mail address

v4:
-fix typo error
-add rte_wmb() after change the reset_ports.

zhao wei (3):
app/testpmd: add port reset command into testpmd
lib/librte_ether: add support for port reset
net/i40e: implement device reset on port

app/test-pmd/cmdline.c | 17 ++-
app/test-pmd/testpmd.c | 65 ++++++++++++
app/test-pmd/testpmd.h | 1 +
drivers/net/i40e/i40e_ethdev.c | 2 +-
drivers/net/i40e/i40e_ethdev.h | 16 +++
drivers/net/i40e/i40e_ethdev_vf.c | 185 +++++++++++++++++++++++++++++++++
drivers/net/i40e/i40e_rxtx.c | 10 ++
drivers/net/i40e/i40e_rxtx.h | 4 +
lib/librte_ether/rte_ethdev.c | 17 +++
lib/librte_ether/rte_ethdev.h | 25 +++++
lib/librte_ether/rte_ether_version.map | 6 ++
11 files changed, 343 insertions(+), 5 deletions(-)
--
2.9.3
Wei Zhao
2017-03-30 09:34:14 UTC
Permalink
Add support for port reset in rte layer.This reset
feature can not only used in vf port reset in later code
develop, but alsopf port.But in this patch set, we only
limit the discussion scope to vf reset.
This patch Add an API to restart the device.
It's for VF device in this scenario, kernel PF + DPDK VF.
When the PF port down->up, APP should call this API to
restart VF port. Most likely, APP should call it in its
management thread and guarantee the thread safe. It means
APP should stop the rx/tx and the device, then restart
the device, then recover the device and rx/tx.
Also, it's APP's responsibilty to release the occupied
memory.

Signed-off-by: Wenzhuo Lu <***@intel.com>
Signed-off-by: Wei Zhao <***@intel.com>
---
lib/librte_ether/rte_ethdev.c | 17 +++++++++++++++++
lib/librte_ether/rte_ethdev.h | 25 +++++++++++++++++++++++++
lib/librte_ether/rte_ether_version.map | 6 ++++++
3 files changed, 48 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index eb0a94a..2e06dca 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3273,3 +3273,20 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
-ENOTSUP);
return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel, mask, en);
}
+
+int
+rte_eth_dev_reset(uint8_t port_id)
+{
+ struct rte_eth_dev *dev;
+ int diag;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+ dev = &rte_eth_devices[port_id];
+
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -ENOTSUP);
+
+ diag = (*dev->dev_ops->dev_reset)(dev);
+
+ return diag;
+}
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 4be217c..34412c0 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1367,6 +1367,9 @@ typedef int (*eth_l2_tunnel_offload_set_t)
uint8_t en);
/**< @internal enable/disable the l2 tunnel offload functions */

+typedef int (*eth_dev_reset_t)(struct rte_eth_dev *dev);
+/**< @internal Function used to reset a configured Ethernet device. */
+
#ifdef RTE_NIC_BYPASS

enum {
@@ -1509,6 +1512,9 @@ struct eth_dev_ops {
eth_l2_tunnel_offload_set_t l2_tunnel_offload_set;
/** Enable/disable l2 tunnel offload functions. */

+ /** Reset device. */
+ eth_dev_reset_t dev_reset;
+
eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue rate limit. */

rss_hash_update_t rss_hash_update; /** Configure RSS hash protocols. */
@@ -4413,6 +4419,25 @@ int
rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);

/**
+ * Reset an ethernet device when it's not working. One scenario is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup the rx/tx
+ * queues, restart the port.
+ * Before calling this API, APP should stop the rx/tx. When tx is being stopped,
+ * APP can drop the packets and release the buffer instead of sending them.
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ *
+ * @return
+ * - (0) if successful.
+ * - (-ENODEV) if port identifier is invalid.
+ * - (-ENOTSUP) if hardware doesn't support this function.
+ */
+int
+rte_eth_dev_reset(uint8_t port_id);
+
+/**
* @internal
* Wrapper for use by pci drivers as a .probe function to attach to a ethdev
* interface.
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index c6c9d0d..529b27f 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -154,3 +154,9 @@ DPDK_17.02 {
rte_flow_validate;

} DPDK_16.11;
+
+DPDK_17.05 {
+ global:
+
+ rte_eth_dev_reset;
+} DPDK_17.02;
\ No newline at end of file
--
2.9.3
Thomas Monjalon
2017-03-30 19:55:19 UTC
Permalink
Hi,

Please help reviewers, use --in-reply-to to keep patches threaded.
Post by Wei Zhao
Add support for port reset in rte layer.This reset
feature can not only used in vf port reset in later code
develop, but alsopf port.But in this patch set, we only
limit the discussion scope to vf reset.
This patch Add an API to restart the device.
It's for VF device in this scenario, kernel PF + DPDK VF.
When the PF port down->up, APP should call this API to
restart VF port. Most likely, APP should call it in its
management thread and guarantee the thread safe. It means
APP should stop the rx/tx and the device, then restart
the device, then recover the device and rx/tx.
Also, it's APP's responsibilty to release the occupied
memory.
Which memory should be released?

[...]
/**
Post by Wei Zhao
+ * Reset an ethernet device when it's not working. One scenario is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup the rx/tx
+ * queues, restart the port.
s/The API/This function/

Please explain exactly the responsibility of this function,
and how it is different from calling stop/configure/start.
Post by Wei Zhao
+ * Before calling this API, APP should stop the rx/tx. When tx is being stopped,
+ * APP can drop the packets and release the buffer instead of sending them.
+ *
+ * The port identifier of the Ethernet device.
+ *
+ * - (0) if successful.
+ * - (-ENODEV) if port identifier is invalid.
+ * - (-ENOTSUP) if hardware doesn't support this function.
+ */
+int
+rte_eth_dev_reset(uint8_t port_id);
Please John, could you help with the API documentation here?
Zhao1, Wei
2017-04-06 02:57:29 UTC
Permalink
Hi, Thomas
-----Original Message-----
Sent: Friday, March 31, 2017 3:55 AM
Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
reset
Hi,
Please help reviewers, use --in-reply-to to keep patches threaded.
Ok, I will use that use --in-reply-to in later when commit patch.
Add support for port reset in rte layer.This reset feature can not
only used in vf port reset in later code develop, but alsopf port.But
in this patch set, we only limit the discussion scope to vf reset.
This patch Add an API to restart the device.
It's for VF device in this scenario, kernel PF + DPDK VF.
When the PF port down->up, APP should call this API to restart VF
port. Most likely, APP should call it in its management thread and
guarantee the thread safe. It means APP should stop the rx/tx and the
device, then restart the device, then recover the device and rx/tx.
Also, it's APP's responsibilty to release the occupied memory.
Which memory should be released?
That is a redundancy now, because the older version need to allocation some memory for this feature,
But it do not need now. So I will delete it in next version.
[...]
/**
+ * Reset an ethernet device when it's not working. One scenario is,
+ after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup the
+ rx/tx
+ * queues, restart the port.
s/The API/This function/
Please explain exactly the responsibility of this function, and how it is
different from calling stop/configure/start.
In this reset feature, reset function can do the calling stop/configure/start process, but also
It can also do some restore work for the port, for example, it can restore the added parameters
of vlan, mac_addrs, promisc_unicast_enabled falg and promisc_multicast_enabled flag.
Maybe , I should add this explanation in the patch comments or function comments?
+ * Before calling this API, APP should stop the rx/tx. When tx is
+being stopped,
+ * APP can drop the packets and release the buffer instead of sending
them.
+ *
+ * The port identifier of the Ethernet device.
+ *
+ * - (0) if successful.
+ * - (-ENODEV) if port identifier is invalid.
+ * - (-ENOTSUP) if hardware doesn't support this function.
+ */
+int
+rte_eth_dev_reset(uint8_t port_id);
Please John, could you help with the API documentation here?
Thank you for John, ple
Thomas Monjalon
2017-04-06 07:11:22 UTC
Permalink
Post by Zhao1, Wei
Post by Thomas Monjalon
/**
Post by Wei Zhao
+ * Reset an ethernet device when it's not working. One scenario is,
+ after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup the
+ rx/tx
+ * queues, restart the port.
s/The API/This function/
Please explain exactly the responsibility of this function, and how it is
different from calling stop/configure/start.
In this reset feature, reset function can do the calling stop/configure/start process, but also
It can also do some restore work for the port, for example, it can restore the added parameters
of vlan, mac_addrs, promisc_unicast_enabled falg and promisc_multicast_enabled flag.
Maybe , I should add this explanation in the patch comments or function comments?
Yes it must be explain in the doxygen part of the function.
Zhao1, Wei
2017-04-06 08:53:13 UTC
Permalink
Hi, Thomas
-----Original Message-----
Sent: Thursday, April 6, 2017 3:11 PM
Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
reset
Post by Zhao1, Wei
Post by Thomas Monjalon
/**
Post by Wei Zhao
+ * Reset an ethernet device when it's not working. One scenario
+ is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup
+ the rx/tx
+ * queues, restart the port.
s/The API/This function/
Please explain exactly the responsibility of this function, and how
it is different from calling stop/configure/start.
In this reset feature, reset function can do the calling
stop/configure/start process, but also It can also do some restore
work for the port, for example, it can restore the added parameters of
vlan, mac_addrs, promisc_unicast_enabled falg and
promisc_multicast_enabled flag.
Post by Zhao1, Wei
Maybe , I should add this explanation in the patch comments or function
comments?
Yes it must be explain in the doxygen part of the function.
Yes, I have add that explanation in v5 which has been commit to dpdk.org.
Ananyev, Konstantin
2017-04-06 09:02:39 UTC
Permalink
-----Original Message-----
Sent: Thursday, April 6, 2017 9:53 AM
Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset
Hi, Thomas
-----Original Message-----
Sent: Thursday, April 6, 2017 3:11 PM
Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
reset
Post by Zhao1, Wei
Post by Thomas Monjalon
/**
Post by Wei Zhao
+ * Reset an ethernet device when it's not working. One scenario
+ is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup
+ the rx/tx
+ * queues, restart the port.
s/The API/This function/
Please explain exactly the responsibility of this function, and how
it is different from calling stop/configure/start.
In this reset feature, reset function can do the calling
stop/configure/start process, but also It can also do some restore
work for the port, for example, it can restore the added parameters of
vlan, mac_addrs, promisc_unicast_enabled falg and
promisc_multicast_enabled flag.
Ok, but why start/stop can't do these things?
Konstantin
Post by Zhao1, Wei
Maybe , I should add this explanation in the patch comments or function
comments?
Yes it must be explain in the doxygen part of the function.
Yes, I have add that explanation in v5 which has been commit to dpdk.org.
Thomas Monjalon
2017-04-10 20:58:25 UTC
Permalink
Post by Ananyev, Konstantin
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Thomas Monjalon
/**
Post by Wei Zhao
+ * Reset an ethernet device when it's not working. One scenario
+ is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup
+ the rx/tx
+ * queues, restart the port.
s/The API/This function/
Please explain exactly the responsibility of this function, and how
it is different from calling stop/configure/start.
In this reset feature, reset function can do the calling
stop/configure/start process, but also It can also do some restore
work for the port, for example, it can restore the added parameters of
vlan, mac_addrs, promisc_unicast_enabled falg and
promisc_multicast_enabled flag.
Ok, but why start/stop can't do these things?
Please could you try to answer this question?

We cannot accept v7 if there are some doubts remaining.
Zhao1, Wei
2017-04-13 08:55:22 UTC
Permalink
Hi, Konstantin
-----Original Message-----
From: Ananyev, Konstantin
Sent: Thursday, April 6, 2017 5:03 PM
Subject: RE: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
reset
-----Original Message-----
Sent: Thursday, April 6, 2017 9:53 AM
Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset
Hi, Thomas
-----Original Message-----
Sent: Thursday, April 6, 2017 3:11 PM
Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset
Post by Zhao1, Wei
Post by Thomas Monjalon
/**
Post by Wei Zhao
+ * Reset an ethernet device when it's not working. One
+ scenario is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues,
+ re-setup the rx/tx
+ * queues, restart the port.
s/The API/This function/
Please explain exactly the responsibility of this function, and
how it is different from calling stop/configure/start.
In this reset feature, reset function can do the calling
stop/configure/start process, but also It can also do some restore
work for the port, for example, it can restore the added
parameters of
vlan, mac_addrs, promisc_unicast_enabled falg and
promisc_multicast_enabled flag.
Ok, but why start/stop can't do these things?
Konstantin
This is because in i40e PMD code, start and stop process do not have the process of store and restore
the added key parameters. Not only i40e but also other PMD code. So, in the function pointed to by dev_reset,
we add specific function do store and restore of some of the important parameters listed above.
Post by Zhao1, Wei
Maybe , I should add this explanation in the patch comments or function
comments?
Yes it must be explain in the doxygen part of the function.
Yes, I have add that explanation in v5 which has been commit to dpdk.org.
Thomas Monjalon
2017-04-13 10:06:32 UTC
Permalink
From: Ananyev, Konstantin
From: Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Thomas Monjalon
/**
Post by Wei Zhao
+ * Reset an ethernet device when it's not working. One
+ scenario is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues,
+ re-setup the rx/tx
+ * queues, restart the port.
s/The API/This function/
Please explain exactly the responsibility of this function, and
how it is different from calling stop/configure/start.
In this reset feature, reset function can do the calling
stop/configure/start process, but also It can also do some restore
work for the port, for example, it can restore the added parameters of
vlan, mac_addrs, promisc_unicast_enabled falg and
promisc_multicast_enabled flag.
Ok, but why start/stop can't do these things?
Konstantin
This is because in i40e PMD code, start and stop process do not have the process of store and restore
the added key parameters. Not only i40e but also other PMD code. So, in the function pointed to by dev_reset,
we add specific function do store and restore of some of the important parameters listed above.
Why store and restore cannot be implemented in start/stop functions?
Zhao1, Wei
2017-04-14 01:29:16 UTC
Permalink
Hi, Thomas Monjalon
-----Original Message-----
Sent: Thursday, April 13, 2017 6:07 PM
Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset
From: Ananyev, Konstantin
From: Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Thomas Monjalon
/**
Post by Wei Zhao
+ * Reset an ethernet device when it's not working. One
+ scenario is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues,
+ re-setup the rx/tx
+ * queues, restart the port.
s/The API/This function/
Please explain exactly the responsibility of this function,
and how it is different from calling stop/configure/start.
In this reset feature, reset function can do the calling
stop/configure/start process, but also It can also do some
restore work for the port, for example, it can restore the
added parameters of
vlan, mac_addrs, promisc_unicast_enabled falg and
promisc_multicast_enabled flag.
Ok, but why start/stop can't do these things?
Konstantin
This is because in i40e PMD code, start and stop process do not have
the process of store and restore the added key parameters. Not only
i40e but also other PMD code. So, in the function pointed to by dev_reset,
we add specific function do store and restore of some of the important
parameters listed above.
Why store and restore cannot be implemented in start/stop functions?
Because reset and start/stop are used for two purposes, for example:
Some user maybe just start/stop the port and he do not care what key parameters
has been configuration last time, and even worse when he want to clear all the configuration last time ,
if we add specific function do store and restore in that two function, it is useless for them,
and may cause a result that user do not expect.
Thomas Monjalon
2017-04-14 06:31:26 UTC
Permalink
Post by Zhao1, Wei
Post by Zhao1, Wei
From: Ananyev, Konstantin
From: Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Thomas Monjalon
/**
Post by Wei Zhao
+ * Reset an ethernet device when it's not working. One
+ scenario is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues,
+ re-setup the rx/tx
+ * queues, restart the port.
s/The API/This function/
Please explain exactly the responsibility of this function,
and how it is different from calling stop/configure/start.
In this reset feature, reset function can do the calling
stop/configure/start process, but also It can also do some
restore work for the port, for example, it can restore the
added parameters of
vlan, mac_addrs, promisc_unicast_enabled falg and
promisc_multicast_enabled flag.
Ok, but why start/stop can't do these things?
Konstantin
This is because in i40e PMD code, start and stop process do not have
the process of store and restore the added key parameters. Not only
i40e but also other PMD code. So, in the function pointed to by dev_reset,
we add specific function do store and restore of some of the important
parameters listed above.
Why store and restore cannot be implemented in start/stop functions?
Some user maybe just start/stop the port and he do not care what key parameters
has been configuration last time, and even worse when he want to clear all the configuration last time ,
if we add specific function do store and restore in that two function, it is useless for them,
and may cause a result that user do not expect.
Is it said somewhere in the doc that the configuration is lost when
stopping a device?
Can we say which configuration parameter is kept and which one is lost?

rte_eth_dev_config_restore() is called in rte_eth_dev_start().
Zhao1, Wei
2017-04-14 08:03:04 UTC
Permalink
Hi, Thomas
-----Original Message-----
Sent: Friday, April 14, 2017 2:31 PM
Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset
Post by Zhao1, Wei
Post by Zhao1, Wei
From: Ananyev, Konstantin
From: Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Thomas Monjalon
/**
Post by Wei Zhao
+ * Reset an ethernet device when it's not working.
+ One scenario is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx
+ queues, re-setup the rx/tx
+ * queues, restart the port.
s/The API/This function/
Please explain exactly the responsibility of this
function, and how it is different from calling
stop/configure/start.
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
In this reset feature, reset function can do the calling
stop/configure/start process, but also It can also do some
restore work for the port, for example, it can restore the
added parameters of
vlan, mac_addrs, promisc_unicast_enabled falg and
promisc_multicast_enabled flag.
Ok, but why start/stop can't do these things?
Konstantin
This is because in i40e PMD code, start and stop process do not
have the process of store and restore the added key parameters.
Not only i40e but also other PMD code. So, in the function
pointed to by dev_reset,
we add specific function do store and restore of some of the
important parameters listed above.
Why store and restore cannot be implemented in start/stop functions?
Some user maybe just start/stop the port and he do not care what key
parameters has been configuration last time, and even worse when he
want to clear all the configuration last time , if we add specific
function do store and restore in that two function, it is useless for them,
and may cause a result that user do not expect.
Is it said somewhere in the doc that the configuration is lost when stopping a
device?
Can we say which configuration parameter is kept and which one is lost?
rte_eth_dev_config_restore() is called in rte_eth_dev_start().
Port reset process not only involve the rte_eth_dev_start() and rte_eth_dev_stop().
It also involve eth_dev_uninit() and eth_dev_init() process,
in which PMD device uninit and init. In this case, for example, data->mac_addrs buffer is freed so it need to add store and restore function.
BUT, if you only call stop/configure/start process, that is not strictly
Zhao1, Wei
2017-04-17 02:08:45 UTC
Permalink
Hi, Thomas

All questions about this patch set has been answered, is it clear or not?And is there anything that I should do for it?
Or it is OK for merge into 17.02-RC2?

Thank you.
-----Original Message-----
From: Zhao1, Wei
Sent: Friday, April 14, 2017 4:03 PM
Subject: RE: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
reset
Hi, Thomas
-----Original Message-----
Sent: Friday, April 14, 2017 2:31 PM
John
Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset
Post by Zhao1, Wei
Post by Zhao1, Wei
From: Ananyev, Konstantin
From: Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Thomas Monjalon
/**
Post by Wei Zhao
+ * Reset an ethernet device when it's not working.
+ One scenario is, after PF
+ * port is down and up, the related VF port should be
reset.
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Thomas Monjalon
Post by Wei Zhao
+ * The API will stop the port, clear the rx/tx
+ queues, re-setup the rx/tx
+ * queues, restart the port.
s/The API/This function/
Please explain exactly the responsibility of this
function, and how it is different from calling
stop/configure/start.
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
In this reset feature, reset function can do the calling
stop/configure/start process, but also It can also do
some restore work for the port, for example, it can
restore the added parameters of
vlan, mac_addrs, promisc_unicast_enabled falg and
promisc_multicast_enabled flag.
Ok, but why start/stop can't do these things?
Konstantin
This is because in i40e PMD code, start and stop process do not
have the process of store and restore the added key parameters.
Not only i40e but also other PMD code. So, in the function
pointed to by dev_reset,
we add specific function do store and restore of some of the
important parameters listed above.
Why store and restore cannot be implemented in start/stop functions?
Some user maybe just start/stop the port and he do not care what
key parameters has been configuration last time, and even worse when
he want to clear all the configuration last time , if we add
specific function do store and restore in that two function, it is
useless for them,
and may cause a result that user do not expect.
Is it said somewhere in the doc that the configuration is lost when
stopping a device?
Can we say which configuration parameter is kept and which one is lost?
rte_eth_dev_config_restore() is called in rte_eth_dev_start().
Port reset process not only involve the rte_eth_dev_start() and rte_eth_dev_stop().
It also involve eth_dev_uninit() and eth_dev_init() process,
in which PMD device uninit and init. In this case, for example, data-
mac_addrs buffer is freed so it need to add store and restore function.
BUT, if you only call stop/configure/start process, that is not strictly
Zhao1, Wei
2017-04-17 05:02:16 UTC
Permalink
Add Thomas new mail address for this mail.
-----Original Message-----
Sent: Monday, April 17, 2017 10:09 AM
Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset
Hi, Thomas
All questions about this patch set has been answered, is it clear or not?And
is there anything that I should do for it?
Or it is OK for merge into 17.02-RC2?
Thank you.
-----Original Message-----
From: Zhao1, Wei
Sent: Friday, April 14, 2017 4:03 PM
John
Subject: RE: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support
for port reset
Hi, Thomas
-----Original Message-----
Sent: Friday, April 14, 2017 2:31 PM
John
Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset
Post by Zhao1, Wei
Post by Zhao1, Wei
From: Ananyev, Konstantin
From: Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Thomas Monjalon
/**
Post by Wei Zhao
+ * Reset an ethernet device when it's not working.
+ One scenario is, after PF
+ * port is down and up, the related VF port
+ should be
reset.
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Thomas Monjalon
Post by Wei Zhao
+ * The API will stop the port, clear the rx/tx
+ queues, re-setup the rx/tx
+ * queues, restart the port.
s/The API/This function/
Please explain exactly the responsibility of this
function, and how it is different from calling
stop/configure/start.
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
In this reset feature, reset function can do the
calling stop/configure/start process, but also It can
also do some restore work for the port, for example,
it can restore the added parameters of
vlan, mac_addrs, promisc_unicast_enabled falg and
promisc_multicast_enabled flag.
Ok, but why start/stop can't do these things?
Konstantin
This is because in i40e PMD code, start and stop process do
not have the process of store and restore the added key
parameters.
Post by Zhao1, Wei
Post by Zhao1, Wei
Not only i40e but also other PMD code. So, in the function
pointed to by dev_reset,
we add specific function do store and restore of some of the
important parameters listed above.
Why store and restore cannot be implemented in start/stop functions?
Some user maybe just start/stop the port and he do not care what
key parameters has been configuration last time, and even worse
when he want to clear all the configuration last time , if we add
specific function do store and restore in that two function, it
is useless for them,
and may cause a result that user do not expect.
Is it said somewhere in the doc that the configuration is lost when
stopping a device?
Can we say which configuration parameter is kept and which one is lost?
rte_eth_dev_config_restore() is called in rte_eth_dev_start().
Port reset process not only involve the rte_eth_dev_start() and rte_eth_dev_stop().
It also involve eth_dev_uninit() and eth_dev_init() process,
in which PMD device uninit and init. In this case, for example, data-
mac_addrs buffer is freed so it need to add store and restore function.
BUT, if you only call stop/configure/start process, that is not
strictly what named "devic
Yuanhan Liu
2017-04-20 06:07:58 UTC
Permalink
This post might be inappropriate. Click to display it.
Zhao1, Wei
2017-04-20 09:17:24 UTC
Permalink
Hi, Yuanhan
-----Original Message-----
Sent: Thursday, April 20, 2017 2:08 PM
Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
reset
Post by Zhao1, Wei
Post by Thomas Monjalon
Post by Wei Zhao
+ * Reset an ethernet device when it's not working. One scenario
+ is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup
+ the rx/tx
+ * queues, restart the port.
s/The API/This function/
Please explain exactly the responsibility of this function, and how
it is different from calling stop/configure/start.
In this reset feature, reset function can do the calling
stop/configure/start process, but also It can also do some restore
work for the port, for example, it can restore the added parameters of
vlan, mac_addrs, promisc_unicast_enabled falg and
promisc_multicast_enabled flag.
Post by Zhao1, Wei
Maybe , I should add this explanation in the patch comments or function
comments?
I'm curious why we have to do save & restore for a reset operation.
Why some configures have to be saved and restored? Doesn't "reset"
literally means reset everything?
Users maybe do not want to do a second configuration operation to waste time after reset which lost all previous configuration.
But he still want these configuration valid after reset.
So, save & restore can help them to save this process time and effort.
Even though, how do you tell what kind of configures need be restored and
what should not? Again, even though, will all PMDs supports restoring the
same set of configurations?
Yes, this is hard to say what may be need and what may be not for user.
Now, the kinds of supported is list in patch set comment. And only i40e NIC support this feature.
While looking at your reset implementation for i40e, it looks more complex
than necessary: just thinking we have to call "xxx_queue_setup"
for all PMDs.
I'm thinking a simple hardware reset might be enough?
/* literally reset the hardware: reset everything */
rte_eth_reset(port)
{
eth_dev->ops->reset();
}
You mean just do a reset and do not restore any configuration?
That may not meet the need for this feature from customer?
Assume the application already has a function (say, port_init()) to initiate a
specific port, it then just needs do something like following to handle the
rte_eth_reset(port);
port_init(port);
You mean "rte_eth_reset" is the function of "rte_eth_dev_reset"?
I do not find any function named rte_eth_reset.
Makes sense? Sorry it's completely wrong; I've limited knowledge on NIC
pmd drivers after all :/
--yliu
Yuanhan Liu
2017-04-21 02:27:47 UTC
Permalink
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
Please explain exactly the responsibility of this function, and how
it is different from calling stop/configure/start.
In this reset feature, reset function can do the calling
stop/configure/start process, but also It can also do some restore
work for the port, for example, it can restore the added parameters of
vlan, mac_addrs, promisc_unicast_enabled falg and
promisc_multicast_enabled flag.
Post by Zhao1, Wei
Maybe , I should add this explanation in the patch comments or function
comments?
I'm curious why we have to do save & restore for a reset operation.
Why some configures have to be saved and restored? Doesn't "reset"
literally means reset everything?
Users maybe do not want to do a second configuration operation to waste time after reset which lost all previous configuration.
But he still want these configuration valid after reset.
So, save & restore can help them to save this process time and effort.
Post by Zhao1, Wei
Even though, how do you tell what kind of configures need be restored and
what should not? Again, even though, will all PMDs supports restoring the
same set of configurations?
Yes, this is hard to say what may be need and what may be not for user.
Now, the kinds of supported is list in patch set comment. And only i40e NIC support this feature.
Why it's the configurations listed in patch 2? Because they are requested
by customers?

Is that all could be saved? If not, are you going to save & restore all
possible configurations?

Assuming the configurations saved & restored may differ from different
PMD drivers, how could you keep the consistency then? And judging that the
application has no idea about the difference (yet it knows nothing about
what kind of configurations will be saved), how could the application know
that some configurations are not saved & restored by the driver that it
has to do re-configuration by itself?
Post by Zhao1, Wei
Post by Zhao1, Wei
While looking at your reset implementation for i40e, it looks more complex
than necessary: just thinking we have to call "xxx_queue_setup"
for all PMDs.
I'm thinking a simple hardware reset might be enough?
/* literally reset the hardware: reset everything */
rte_eth_reset(port)
{
eth_dev->ops->reset();
}
You mean just do a reset and do not restore any configuration?
That may not meet the need for this feature from customer?
Right, I'm just aware of the configuration might be done by PF (but not
only by the application), that the VF port may be not aware of those
configurations. So the save & restore is needed. I don't quite like
how it is done in this patch set though. I also don't think the API is
well named: as said, reset should literally reset everything.

We may need think how to do it properly.

Thomas, Konstantin, what do you guys think of it?

--yliu
Thomas Monjalon
2017-04-21 08:27:49 UTC
Permalink
Post by Yuanhan Liu
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
Please explain exactly the responsibility of this function, and how
it is different from calling stop/configure/start.
In this reset feature, reset function can do the calling
stop/configure/start process, but also It can also do some restore
work for the port, for example, it can restore the added parameters
of
vlan, mac_addrs, promisc_unicast_enabled falg and
promisc_multicast_enabled flag.
Post by Zhao1, Wei
Maybe , I should add this explanation in the patch comments or function
comments?
I'm curious why we have to do save & restore for a reset operation.
Why some configures have to be saved and restored? Doesn't "reset"
literally means reset everything?
Users maybe do not want to do a second configuration operation to waste
time after reset which lost all previous configuration. But he still want
these configuration valid after reset.
So, save & restore can help them to save this process time and effort.
Post by Zhao1, Wei
Even though, how do you tell what kind of configures need be restored and
what should not? Again, even though, will all PMDs supports restoring the
same set of configurations?
Yes, this is hard to say what may be need and what may be not for user.
Now, the kinds of supported is list in patch set comment. And only i40e
NIC support this feature.
Why it's the configurations listed in patch 2? Because they are requested
by customers?
Is that all could be saved? If not, are you going to save & restore all
possible configurations?
Assuming the configurations saved & restored may differ from different
PMD drivers, how could you keep the consistency then? And judging that the
application has no idea about the difference (yet it knows nothing about
what kind of configurations will be saved), how could the application know
that some configurations are not saved & restored by the driver that it
has to do re-configuration by itself?
Post by Zhao1, Wei
Post by Zhao1, Wei
While looking at your reset implementation for i40e, it looks more complex
than necessary: just thinking we have to call "xxx_queue_setup"
for all PMDs.
I'm thinking a simple hardware reset might be enough?
/* literally reset the hardware: reset everything */
rte_eth_reset(port)
{
eth_dev->ops->reset();
}
You mean just do a reset and do not restore any configuration?
That may not meet the need for this feature from customer?
Right, I'm just aware of the configuration might be done by PF (but not
only by the application), that the VF port may be not aware of those
configurations. So the save & restore is needed. I don't quite like
how it is done in this patch set though. I also don't think the API is
well named: as said, reset should literally reset everything.
We may need think how to do it properly.
Thomas, Konstantin, what do you guys think of it?
I have the same concerns.
I think we should better document the current status of start/stop
and which configuration parameters are lost or saved.
Zhao1, Wei
2017-04-21 08:59:17 UTC
Permalink
This post might be inappropriate. Click to display it.
Thomas Monjalon
2017-04-21 09:28:37 UTC
Permalink
Post by Zhao1, Wei
Hi, thomas
Post by Thomas Monjalon
Post by Yuanhan Liu
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Thomas Monjalon
Please explain exactly the responsibility of this function,
and how it is different from calling stop/configure/start.
In this reset feature, reset function can do the calling
stop/configure/start process, but also It can also do some
restore work for the port, for example, it can restore the added
parameters of
vlan, mac_addrs, promisc_unicast_enabled falg and
promisc_multicast_enabled flag.
Post by Zhao1, Wei
Maybe , I should add this explanation in the patch comments or function
comments?
I'm curious why we have to do save & restore for a reset operation.
Why some configures have to be saved and restored? Doesn't "reset"
literally means reset everything?
Users maybe do not want to do a second configuration operation to
waste time after reset which lost all previous configuration. But he
still want these configuration valid after reset.
So, save & restore can help them to save this process time and effort.
Post by Zhao1, Wei
Even though, how do you tell what kind of configures need be
restored and what should not? Again, even though, will all PMDs
supports restoring the same set of configurations?
Yes, this is hard to say what may be need and what may be not for user.
Now, the kinds of supported is list in patch set comment. And only
i40e NIC support this feature.
Why it's the configurations listed in patch 2? Because they are
requested by customers?
Is that all could be saved? If not, are you going to save & restore
all possible configurations?
Assuming the configurations saved & restored may differ from different
PMD drivers, how could you keep the consistency then? And judging that
the application has no idea about the difference (yet it knows nothing
about what kind of configurations will be saved), how could the
application know that some configurations are not saved & restored by
the driver that it has to do re-configuration by itself?
Post by Zhao1, Wei
Post by Zhao1, Wei
While looking at your reset implementation for i40e, it looks more
complex than necessary: just thinking we have to call
"xxx_queue_setup"
for all PMDs.
I'm thinking a simple hardware reset might be enough?
/* literally reset the hardware: reset everything */
rte_eth_reset(port)
{
eth_dev->ops->reset();
}
You mean just do a reset and do not restore any configuration?
That may not meet the need for this feature from customer?
Right, I'm just aware of the configuration might be done by PF (but
not only by the application), that the VF port may be not aware of
those configurations. So the save & restore is needed. I don't quite
like how it is done in this patch set though. I also don't think the
API is well named: as said, reset should literally reset everything.
We may need think how to do it properly.
Thomas, Konstantin, what do you guys think of it?
I have the same concerns.
I think we should better document the current status of start/stop and
which configuration parameters are lost or saved.
Maybe I should add some words in doc\guides\nics\i40e.rst to Record which
configurations are saved and restored by the PMD driver in reset
function. Which not list in that are recognized as not saved and restored
by default. OTHER NIC for this feature can add similar record in their
xxx.rst.
No, when defining a generic API in ethdev, we must define the same
behaviour for every drivers.
Please check how to make the behaviour consistent and documented
in ethdev. We may need to document your new function and start/stop also.
Yuanhan Liu
2017-04-24 02:01:20 UTC
Permalink
Post by Thomas Monjalon
Post by Zhao1, Wei
Maybe I should add some words in doc\guides\nics\i40e.rst to Record which
configurations are saved and restored by the PMD driver in reset
function. Which not list in that are recognized as not saved and restored
by default. OTHER NIC for this feature can add similar record in their
xxx.rst.
No, when defining a generic API in ethdev, we must define the same
behaviour for every drivers.
Agreed. That was my point.

--yliu
Post by Thomas Monjalon
Please check how to make the behaviour consistent and documented
in ethdev. We may need to document your new function and start/stop also.
Zhao1, Wei
2017-04-24 03:15:50 UTC
Permalink
Hi, yuanhan & thomas
-----Original Message-----
Sent: Monday, April 24, 2017 10:01 AM
Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
reset
Post by Thomas Monjalon
Post by Zhao1, Wei
Maybe I should add some words in doc\guides\nics\i40e.rst to Record
which configurations are saved and restored by the PMD driver in
reset function. Which not list in that are recognized as not saved
and restored by default. OTHER NIC for this feature can add similar
record in their xxx.rst.
No, when defining a generic API in ethdev, we must define the same
behaviour for every drivers.
Agreed. That was my point.
--yliu
Post by Thomas Monjalon
Please check how to make the behaviour consistent and documented in
ethdev. We may need to document your new function and start/stop also.
Maybe I should record these reset and restore info in some doc in rte layer.
And we have only implement this feature in i40e NIC by now.
Zhao1, Wei
2017-04-24 03:39:54 UTC
Permalink
Hi, yuanhan & thomas
-----Original Message-----
Sent: Monday, April 24, 2017 10:01 AM
Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
reset
Post by Thomas Monjalon
Post by Zhao1, Wei
Maybe I should add some words in doc\guides\nics\i40e.rst to Record
which configurations are saved and restored by the PMD driver in
reset function. Which not list in that are recognized as not saved
and restored by default. OTHER NIC for this feature can add similar
record in their xxx.rst.
No, when defining a generic API in ethdev, we must define the same
behaviour for every drivers.
Agreed. That was my point.
--yliu
Post by Thomas Monjalon
Please check how to make the behaviour consistent and documented in
ethdev. We may need to document your new function and start/stop also.
Do you have any suggestion on which document in rte layer to record store and restore info by me?
Thomas Monjalon
2017-04-24 08:04:40 UTC
Permalink
Post by Zhao1, Wei
Hi, yuanhan & thomas
Post by Yuanhan Liu
Post by Thomas Monjalon
Post by Zhao1, Wei
Maybe I should add some words in doc\guides\nics\i40e.rst to Record
which configurations are saved and restored by the PMD driver in
reset function. Which not list in that are recognized as not saved
and restored by default. OTHER NIC for this feature can add similar
record in their xxx.rst.
No, when defining a generic API in ethdev, we must define the same
behaviour for every drivers.
Agreed. That was my point.
--yliu
Post by Thomas Monjalon
Please check how to make the behaviour consistent and documented in
ethdev. We may need to document your new function and start/stop also.
Do you have any suggestion on which document in rte layer to record store
and restore info by me?
It should be documented in the doxygen comment of the functions.
Either we explain which configuration is restored on start and reset,
or we state everything (or nothing) is restored except the configurations
commented in the related configuration functions.
Zhao1, Wei
2017-04-25 03:14:49 UTC
Permalink
Ok.
-----Original Message-----
Sent: Monday, April 24, 2017 4:05 PM
Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
reset
Post by Zhao1, Wei
Hi, yuanhan & thomas
Post by Yuanhan Liu
Post by Thomas Monjalon
Post by Zhao1, Wei
Maybe I should add some words in doc\guides\nics\i40e.rst to
Record which configurations are saved and restored by the PMD
driver in reset function. Which not list in that are recognized
as not saved and restored by default. OTHER NIC for this
feature can add similar record in their xxx.rst.
No, when defining a generic API in ethdev, we must define the same
behaviour for every drivers.
Agreed. That was my point.
--yliu
Post by Thomas Monjalon
Please check how to make the behaviour consistent and documented
in ethdev. We may need to document your new function and start/stop
also.
Post by Zhao1, Wei
Do you have any suggestion on which document in rte layer to record
store and restore info by me?
It should be documented in the doxygen comment of the functions.
Either we explain which configuration is restored on start and reset, or we
state everything (or nothing) is restored except the configurations
commented in the related configuration functions.
Zhao1, Wei
2017-04-21 08:55:38 UTC
Permalink
-----Original Message-----
Sent: Friday, April 21, 2017 10:28 AM
Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
reset
Post by Zhao1, Wei
Post by Zhao1, Wei
Post by Zhao1, Wei
Please explain exactly the responsibility of this function, and
how it is different from calling stop/configure/start.
In this reset feature, reset function can do the calling
stop/configure/start process, but also It can also do some restore
work for the port, for example, it can restore the added
parameters of
vlan, mac_addrs, promisc_unicast_enabled falg and
promisc_multicast_enabled flag.
Post by Zhao1, Wei
Maybe , I should add this explanation in the patch comments or function
comments?
I'm curious why we have to do save & restore for a reset operation.
Why some configures have to be saved and restored? Doesn't "reset"
literally means reset everything?
Users maybe do not want to do a second configuration operation to waste
time after reset which lost all previous configuration.
Post by Zhao1, Wei
But he still want these configuration valid after reset.
So, save & restore can help them to save this process time and effort.
Post by Zhao1, Wei
Even though, how do you tell what kind of configures need be
restored and what should not? Again, even though, will all PMDs
supports restoring the same set of configurations?
Yes, this is hard to say what may be need and what may be not for user.
Now, the kinds of supported is list in patch set comment. And only i40e NIC
support this feature.
Why it's the configurations listed in patch 2? Because they are requested by
customers?
Is that all could be saved? If not, are you going to save & restore all possible
configurations?
Assuming the configurations saved & restored may differ from different
PMD drivers, how could you keep the consistency then? And judging that the
application has no idea about the difference (yet it knows nothing about
what kind of configurations will be saved), how could the application know
that some configurations are not saved & restored by the driver that it has to
do re-configuration by itself?
Good idea, so maybe I should add some words in doc\guides\nics\i40e.rst to
Record which configurations are saved and restored by the PMD driver in reset function.
Which not list in that are recognized as not saved and restored by default.
Is that ok ?
Post by Zhao1, Wei
Post by Zhao1, Wei
While looking at your reset implementation for i40e, it looks more
complex than necessary: just thinking we have to call
"xxx_queue_setup"
Post by Zhao1, Wei
Post by Zhao1, Wei
for all PMDs.
I'm thinking a simple hardware reset might be enough?
/* literally reset the hardware: reset everything */
rte_eth_reset(port)
{
eth_dev->ops->reset();
}
You mean just do a reset and do not restore any configuration?
That may not meet the need for this feature from customer?
Right, I'm just aware of the configuration might be done by PF (but not only
by the application), that the VF port may be not aware of those
configurations. So the save & restore is needed. I don't quite like how it is
done in this patch set though. I also don't think the API is well named: as said,
reset should literally reset everything.
We may need think how to do it properly.
Thomas, Konstantin, what do you guys think of it?
So, your opinion is if it is named "reset", we had better do not do any restore work?
If we keep this restore feature, we had better change function name?
May be use the name "reset_and_restore" as function and feature name ?
--yliu
Wei Zhao
2017-03-30 09:34:15 UTC
Permalink
This patch implement the device reset function on vf port.
This restart function will detach device then
attach device, reconfigure dev, re-setup
the Rx/Tx queues.

Signed-off-by: Wei Zhao <***@intel.com>
Signed-off-by: Wenzhuo Lu <***@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 2 +-
drivers/net/i40e/i40e_ethdev.h | 16 ++++
drivers/net/i40e/i40e_ethdev_vf.c | 185 ++++++++++++++++++++++++++++++++++++++
drivers/net/i40e/i40e_rxtx.c | 10 +++
drivers/net/i40e/i40e_rxtx.h | 4 +
5 files changed, 216 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 3702214..f2fdb2f 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -6020,7 +6020,7 @@ i40e_find_vlan_filter(struct i40e_vsi *vsi,
return 0;
}

-static void
+void
i40e_store_vlan_filter(struct i40e_vsi *vsi,
uint16_t vlan_id, bool on)
{
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index aebb097..515a288 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -652,6 +652,16 @@ struct i40e_vf_tx_queues {
uint32_t tx_ring_len;
};

+struct i40e_vf_reset_store {
+ struct ether_addr *mac_addrs; /* Device Ethernet Link address. */
+ bool promisc_unicast_enabled;
+ bool promisc_multicast_enabled;
+ uint16_t vlan_num; /* Total VLAN number */
+ uint32_t vfta[I40E_VFTA_SIZE]; /* VLAN bitmap */
+ uint16_t mac_num; /* Total mac number */
+};
+
+
/*
* Structure to store private data specific for VF instance.
*/
@@ -709,6 +719,10 @@ struct i40e_adapter {
struct rte_timecounter systime_tc;
struct rte_timecounter rx_tstamp_tc;
struct rte_timecounter tx_tstamp_tc;
+
+ /* For port reset */
+ volatile uint8_t reset_flag;
+ void *reset_store_data;
};

extern const struct rte_flow_ops i40e_flow_ops;
@@ -749,6 +763,8 @@ int i40e_dev_link_update(struct rte_eth_dev *dev,
__rte_unused int wait_to_complete);
void i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi);
void i40e_vsi_queues_unbind_intr(struct i40e_vsi *vsi);
+void i40e_store_vlan_filter(struct i40e_vsi *vsi,
+ uint16_t vlan_id, bool on);
int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
struct i40e_vsi_vlan_pvid_info *info);
int i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 55fd344..9ab035c 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -161,6 +161,9 @@ i40evf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id);
static void i40evf_handle_pf_event(__rte_unused struct rte_eth_dev *dev,
uint8_t *msg,
uint16_t msglen);
+static int i40evf_dev_uninit(struct rte_eth_dev *eth_dev);
+static int i40evf_dev_init(struct rte_eth_dev *eth_dev);
+static int i40evf_reset_dev(struct rte_eth_dev *dev);

/* Default hash key buffer for RSS */
static uint32_t rss_key_default[I40E_VFQF_HKEY_MAX_INDEX + 1];
@@ -230,6 +233,7 @@ static const struct eth_dev_ops i40evf_eth_dev_ops = {
.rss_hash_conf_get = i40evf_dev_rss_hash_conf_get,
.mtu_set = i40evf_dev_mtu_set,
.mac_addr_set = i40evf_set_default_mac_addr,
+ .dev_reset = i40evf_reset_dev,
};

/*
@@ -889,6 +893,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
PMD_DRV_LOG(ERR, "fail to execute command "
"OP_ADD_ETHER_ADDRESS");

+ vf->vsi.mac_num++;
return;
}

@@ -926,6 +931,7 @@ i40evf_del_mac_addr_by_addr(struct rte_eth_dev *dev,
if (err)
PMD_DRV_LOG(ERR, "fail to execute command "
"OP_DEL_ETHER_ADDRESS");
+ vf->vsi.mac_num--;
return;
}

@@ -1047,6 +1053,7 @@ static int
i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
{
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+ struct i40e_vsi *vsi = &vf->vsi;
struct i40e_virtchnl_vlan_filter_list *vlan_list;
uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
sizeof(uint16_t)];
@@ -1066,6 +1073,8 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
err = i40evf_execute_vf_cmd(dev, &args);
if (err)
PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+ i40e_store_vlan_filter(vsi, vlanid, 1);
+ vsi->vlan_num++;

return err;
}
@@ -1074,6 +1083,7 @@ static int
i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
{
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+ struct i40e_vsi *vsi = &vf->vsi;
struct i40e_virtchnl_vlan_filter_list *vlan_list;
uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
sizeof(uint16_t)];
@@ -1093,6 +1103,8 @@ i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
err = i40evf_execute_vf_cmd(dev, &args);
if (err)
PMD_DRV_LOG(ERR, "fail to execute command OP_DEL_VLAN");
+ i40e_store_vlan_filter(vsi, vlanid, 0);
+ vsi->vlan_num--;

return err;
}
@@ -2716,3 +2728,176 @@ i40evf_set_default_mac_addr(struct rte_eth_dev *dev,

i40evf_add_mac_addr(dev, mac_addr, 0, 0);
}
+
+static void
+i40evf_restore_vlan_filter(struct rte_eth_dev *dev,
+ uint32_t *vfta)
+{
+ uint32_t vid_idx, vid_bit;
+ uint16_t vlan_id;
+
+ for (vid_idx = 0; vid_idx < I40E_VFTA_SIZE; vid_idx++) {
+ for (vid_bit = 0; vid_bit < I40E_UINT32_BIT_SIZE; vid_bit++) {
+ if (vfta[vid_idx] & (1 << vid_bit)) {
+ vlan_id = (vid_idx << 5) + vid_bit;
+ i40evf_add_vlan(dev, vlan_id);
+ }
+ }
+ }
+}
+
+static void
+i40evf_restore_macaddr(struct rte_eth_dev *dev,
+ struct ether_addr *mac_addrs)
+{
+ struct ether_addr *addr;
+ uint16_t i;
+
+ /* replay MAC address configuration including default MAC */
+ addr = &mac_addrs[0];
+
+ i40evf_set_default_mac_addr(dev, addr);
+ memcpy(dev->data->mac_addrs, mac_addrs,
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+ for (i = 1; i < I40E_NUM_MACADDR_MAX; i++) {
+ addr = &mac_addrs[i];
+
+ /* skip zero address */
+ if (is_zero_ether_addr(addr))
+ continue;
+
+ i40evf_add_mac_addr(dev, addr, 0, 0);
+ }
+}
+
+
+static void
+i40e_vf_queue_reset(struct rte_eth_dev *dev)
+{
+ uint16_t i;
+
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ struct i40e_rx_queue *rxq = dev->data->rx_queues[i];
+
+ if (rxq->q_set) {
+ i40e_dev_rx_queue_setup(dev,
+ rxq->queue_id,
+ rxq->nb_rx_desc,
+ rxq->socket_id,
+ &rxq->rxconf,
+ rxq->mp);
+ }
+ }
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ struct i40e_tx_queue *txq = dev->data->tx_queues[i];
+
+ if (txq->q_set) {
+ i40e_dev_tx_queue_setup(dev,
+ txq->queue_id,
+ txq->nb_tx_desc,
+ txq->socket_id,
+ &txq->txconf);
+ }
+ }
+}
+
+static int
+i40evf_store_before_reset(struct rte_eth_dev *dev)
+{
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+ struct i40e_vf_reset_store *store_data;
+ struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+
+ adapter->reset_store_data = rte_zmalloc("i40evf_store_reset",
+ sizeof(struct i40e_vf_reset_store), 0);
+ if (adapter->reset_store_data == NULL) {
+ PMD_INIT_LOG(ERR, "Failed to allocate %ld bytes needed to"
+ " to store data when reset vf",
+ sizeof(struct i40e_vf_reset_store));
+ return -ENOMEM;
+ }
+ store_data =
+ (struct i40e_vf_reset_store *)adapter->reset_store_data;
+ store_data->mac_addrs = rte_zmalloc("i40evf_mac_store_reset",
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX, 0);
+ if (store_data->mac_addrs == NULL) {
+ PMD_INIT_LOG(ERR, "Failed to allocate %d bytes needed to"
+ " to store MAC addresses when reset vf",
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+ }
+
+ memcpy(store_data->mac_addrs, dev->data->mac_addrs,
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+ store_data->promisc_unicast_enabled = vf->promisc_unicast_enabled;
+ store_data->promisc_multicast_enabled = vf->promisc_multicast_enabled;
+
+ store_data->vlan_num = vf->vsi.vlan_num;
+ memcpy(store_data->vfta, vf->vsi.vfta,
+ sizeof(uint32_t) * I40E_VFTA_SIZE);
+
+ store_data->mac_num = vf->vsi.mac_num;
+
+ return 0;
+}
+
+static void
+i40evf_restore_after_reset(struct rte_eth_dev *dev)
+{
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+ struct i40e_vf_reset_store *store_data =
+ (struct i40e_vf_reset_store *)adapter->reset_store_data;
+
+ if (store_data->promisc_unicast_enabled)
+ i40evf_dev_promiscuous_enable(dev);
+ if (store_data->promisc_multicast_enabled)
+ i40evf_dev_allmulticast_enable(dev);
+
+ if (store_data->vlan_num)
+ i40evf_restore_vlan_filter(dev, store_data->vfta);
+
+ if (store_data->mac_num)
+ i40evf_restore_macaddr(dev, store_data->mac_addrs);
+
+ rte_free(store_data->mac_addrs);
+ rte_free(adapter->reset_store_data);
+}
+
+static int
+i40evf_reset_dev(struct rte_eth_dev *dev)
+{
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+
+ adapter->reset_flag = 1;
+ i40evf_store_before_reset(dev);
+
+ i40evf_dev_close(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev close complete");
+
+ i40evf_dev_uninit(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev detached");
+
+ memset(dev->data->dev_private, 0,
+ (uint64_t)&adapter->reset_flag - (uint64_t)adapter);
+
+ i40evf_dev_init(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev attached");
+
+ i40evf_dev_configure(dev);
+
+ i40e_vf_queue_reset(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf queue reset");
+
+ i40evf_restore_after_reset(dev);
+
+ i40evf_dev_start(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev restart");
+
+ adapter->reset_flag = 0;
+
+ return 0;
+}
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 02367b7..d891a54 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1709,6 +1709,7 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
uint16_t len, i;
uint16_t base, bsf, tc_mapping;
int use_def_burst_func = 1;
+ struct rte_eth_rxconf conf = *rx_conf;

if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
struct i40e_vf *vf =
@@ -1748,6 +1749,8 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
}
rxq->mp = mp;
rxq->nb_rx_desc = nb_desc;
+ rxq->socket_id = socket_id;
+ rxq->rxconf = conf;
rxq->rx_free_thresh = rx_conf->rx_free_thresh;
rxq->queue_id = queue_idx;
if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF)
@@ -1932,6 +1935,7 @@ i40e_dev_tx_queue_setup(struct rte_eth_dev *dev,
uint32_t ring_size;
uint16_t tx_rs_thresh, tx_free_thresh;
uint16_t i, base, bsf, tc_mapping;
+ struct rte_eth_txconf conf = *tx_conf;

if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
struct i40e_vf *vf =
@@ -2054,6 +2058,8 @@ i40e_dev_tx_queue_setup(struct rte_eth_dev *dev,
}

txq->nb_tx_desc = nb_desc;
+ txq->socket_id = socket_id;
+ txq->txconf = conf;
txq->tx_rs_thresh = tx_rs_thresh;
txq->tx_free_thresh = tx_free_thresh;
txq->pthresh = tx_conf->tx_thresh.pthresh;
@@ -2520,8 +2526,12 @@ void
i40e_dev_free_queues(struct rte_eth_dev *dev)
{
uint16_t i;
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);

PMD_INIT_FUNC_TRACE();
+ if (adapter->reset_flag)
+ return;

for (i = 0; i < dev->data->nb_rx_queues; i++) {
if (!dev->data->rx_queues[i])
diff --git a/drivers/net/i40e/i40e_rxtx.h b/drivers/net/i40e/i40e_rxtx.h
index a87bdb0..0e3cc19 100644
--- a/drivers/net/i40e/i40e_rxtx.h
+++ b/drivers/net/i40e/i40e_rxtx.h
@@ -136,6 +136,8 @@ struct i40e_rx_queue {
bool rx_deferred_start; /**< don't start this queue in dev start */
uint16_t rx_using_sse; /**<flag indicate the usage of vPMD for rx */
uint8_t dcb_tc; /**< Traffic class of rx queue */
+ uint8_t socket_id;
+ struct rte_eth_rxconf rxconf;
};

struct i40e_tx_entry {
@@ -177,6 +179,8 @@ struct i40e_tx_queue {
bool q_set; /**< indicate if tx queue has been configured */
bool tx_deferred_start; /**< don't start this queue in dev start */
uint8_t dcb_tc; /**< Traffic class of tx queue */
+ uint8_t socket_id;
+ struct rte_eth_txconf txconf;
};

/** Offload features */
--
2.9.3
Wei Zhao
2017-03-30 09:34:16 UTC
Permalink
Add port reset command into testpmd project,
it is the interface for user to reset port.
And also it is not limit to be used only in vf reset,
but also for pf port reset.But this patch set only
realted to vf reset feature.

Signed-off-by: Wei Zhao <***@intel.com>
Signed-off-by: Wenzhuo Lu <***@intel.com>
---
app/test-pmd/cmdline.c | 17 +++++++++----
app/test-pmd/testpmd.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
app/test-pmd/testpmd.h | 1 +
3 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 47f935d..6cbdcc8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -599,6 +599,9 @@ static void cmd_help_long_parsed(void *parsed_result,
"port close (port_id|all)\n"
" Close all ports or port_id.\n\n"

+ "port reset (port_id|all)\n"
+ " Reset all ports or port_id.\n\n"
+
"port attach (ident)\n"
" Attach physical or virtual dev by pci address or virtual device name\n\n"

@@ -909,6 +912,8 @@ static void cmd_operate_port_parsed(void *parsed_result,
stop_port(RTE_PORT_ALL);
else if (!strcmp(res->name, "close"))
close_port(RTE_PORT_ALL);
+ else if (!strcmp(res->name, "reset"))
+ reset_port(RTE_PORT_ALL);
else
printf("Unknown parameter\n");
}
@@ -918,14 +923,15 @@ cmdline_parse_token_string_t cmd_operate_port_all_cmd =
"port");
cmdline_parse_token_string_t cmd_operate_port_all_port =
TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, name,
- "start#stop#close");
+ "start#stop#close#reset");
cmdline_parse_token_string_t cmd_operate_port_all_all =
TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, value, "all");

cmdline_parse_inst_t cmd_operate_port = {
.f = cmd_operate_port_parsed,
.data = NULL,
- .help_str = "port start|stop|close all: Start/Stop/Close all ports",
+ .help_str = "port start|stop|close|reset all: Start/Stop/Close/"
+ "Reset all ports",
.tokens = {
(void *)&cmd_operate_port_all_cmd,
(void *)&cmd_operate_port_all_port,
@@ -953,6 +959,8 @@ static void cmd_operate_specific_port_parsed(void *parsed_result,
stop_port(res->value);
else if (!strcmp(res->name, "close"))
close_port(res->value);
+ else if (!strcmp(res->name, "reset"))
+ reset_port(res->value);
else
printf("Unknown parameter\n");
}
@@ -962,7 +970,7 @@ cmdline_parse_token_string_t cmd_operate_specific_port_cmd =
keyword, "port");
cmdline_parse_token_string_t cmd_operate_specific_port_port =
TOKEN_STRING_INITIALIZER(struct cmd_operate_specific_port_result,
- name, "start#stop#close");
+ name, "start#stop#close#reset");
cmdline_parse_token_num_t cmd_operate_specific_port_id =
TOKEN_NUM_INITIALIZER(struct cmd_operate_specific_port_result,
value, UINT8);
@@ -970,7 +978,8 @@ cmdline_parse_token_num_t cmd_operate_specific_port_id =
cmdline_parse_inst_t cmd_operate_specific_port = {
.f = cmd_operate_specific_port_parsed,
.data = NULL,
- .help_str = "port start|stop|close <port_id>: Start/Stop/Close port_id",
+ .help_str = "port start|stop|close|reset <port_id>: Start/Stop/Close/"
+ "Reset port_id",
.tokens = {
(void *)&cmd_operate_specific_port_cmd,
(void *)&cmd_operate_specific_port_port,
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 484c19b..a4f5330 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -137,6 +137,7 @@ portid_t nb_fwd_ports; /**< Number of forwarding ports. */

unsigned int fwd_lcores_cpuids[RTE_MAX_LCORE]; /**< CPU ids configuration. */
portid_t fwd_ports_ids[RTE_MAX_ETHPORTS]; /**< Port ids configuration. */
+volatile char reset_ports[RTE_MAX_ETHPORTS] = {0}; /**< Port reset flag. */

struct fwd_stream **fwd_streams; /**< For each RX queue of each port. */
streamid_t nb_fwd_streams; /**< Is equal to (nb_ports * nb_rxq). */
@@ -1305,6 +1306,18 @@ port_is_closed(portid_t port_id)
return 1;
}

+static void
+reset_event_callback(uint8_t port_id, enum rte_eth_event_type type, void *param)
+{
+ RTE_SET_USED(param);
+
+ printf("Event type: %s on port %d\n",
+ type == RTE_ETH_EVENT_INTR_RESET ? "RESET interrupt" :
+ "unknown event", port_id);
+ reset_ports[port_id] = 1;
+ rte_wmb();
+}
+
int
start_port(portid_t pid)
{
@@ -1350,6 +1363,10 @@ start_port(portid_t pid)
return -1;
}
}
+
+ /* register reset interrupt callback */
+ rte_eth_dev_callback_register(pi, RTE_ETH_EVENT_INTR_RESET,
+ reset_event_callback, NULL);
if (port->need_reconfig_queues > 0) {
port->need_reconfig_queues = 0;
/* setup tx queues */
@@ -1559,6 +1576,54 @@ close_port(portid_t pid)
}

void
+reset_port(portid_t pid)
+{
+ portid_t pi;
+ struct rte_port *port;
+
+ if (port_id_is_invalid(pid, ENABLED_WARN))
+ return;
+
+ printf("Reset ports...\n");
+
+ FOREACH_PORT(pi, ports) {
+ if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
+ continue;
+
+ if (port_is_forwarding(pi) != 0 && test_done == 0) {
+ printf("Please remove port %d from forwarding "
+ "configuration.\n", pi);
+ continue;
+ }
+
+ if (port_is_bonding_slave(pi)) {
+ printf("Please remove port %d from "
+ "bonded device.\n", pi);
+ continue;
+ }
+
+ if (!reset_ports[pi]) {
+ printf("vf must get reset port %d info from "
+ "pf before reset.\n", pi);
+ continue;
+ }
+
+ port = &ports[pi];
+ if (rte_atomic16_cmpset(&(port->port_status),
+ RTE_PORT_STOPPED, RTE_PORT_HANDLING) == 1) {
+ printf("Port %d is not stopped\n", pi);
+ continue;
+ }
+
+ rte_eth_dev_reset(pi);
+ reset_ports[pi] = 0;
+ rte_wmb();
+ }
+
+ printf("Done\n");
+}
+
+void
attach_port(char *identifier)
{
portid_t pi = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 8cf2860..0c7e44c 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -586,6 +586,7 @@ int init_port_dcb_config(portid_t pid, enum dcb_mode_enable dcb_mode,
int start_port(portid_t pid);
void stop_port(portid_t pid);
void close_port(portid_t pid);
+void reset_port(portid_t pid);
void attach_port(char *identifier);
void detach_port(uint8_t port_id);
int all_ports_stopped(void);
--
2.9.3
Wu, Jingjing
2017-03-30 12:32:57 UTC
Permalink
-----Original Message-----
Sent: Thursday, March 30, 2017 5:34 PM
Subject: [dpdk-dev] [PATCH v4 0/3] net/i40e: vf port reset
1) get pf reset vf comamand falg from interrupt server function.
2) reset vf port from testpmd app using a command.
3) sore and restore vf related parameters.
-change the order of patch in the set.
-add more details in patch comment to clarify that the rte and tespmd patch can
also used in pf port reset.
-add rte_free for memory free after restore.
-change varible name of reset_number to reset_falg.
-fix patch check warning.
-fix error of author mail address
-fix typo error
-add rte_wmb() after change the reset_ports.
app/testpmd: add port reset command into testpmd
lib/librte_ether: add support for port reset
net/i40e: implement device reset on port
Acked-by Jingjing Wu <***@intel.com>

It looks like you sent 2 V4? Could you suspend one of them in Patchwork?
Zhao1, Wei
2017-04-05 05:42:45 UTC
Permalink
Hi, jingjing
-----Original Message-----
From: Wu, Jingjing
Sent: Thursday, March 30, 2017 8:33 PM
Subject: RE: [dpdk-dev] [PATCH v4 0/3] net/i40e: vf port reset
-----Original Message-----
Sent: Thursday, March 30, 2017 5:34 PM
Subject: [dpdk-dev] [PATCH v4 0/3] net/i40e: vf port reset
1) get pf reset vf comamand falg from interrupt server function.
2) reset vf port from testpmd app using a command.
3) sore and restore vf related parameters.
-change the order of patch in the set.
-add more details in patch comment to clarify that the rte and tespmd
patch can also used in pf port reset.
-add rte_free for memory free after restore.
-change varible name of reset_number to reset_falg.
-fix patch check warning.
-fix error of author mail address
-fix typo error
-add rte_wmb() after change the reset_ports.
app/testpmd: add port reset command into testpmd
lib/librte_ether: add support for port reset
net/i40e: implement device reset on port
It looks like you sent 2 V4? Could you suspend one of them in Patchwork?
I have
Wei Zhao
2017-04-06 06:33:44 UTC
Permalink
The patches mainly finish following functions:
1) get pf reset vf comamand falg from interrupt server function.
2) reset vf port from testpmd app using a command.
3) sore and restore vf related parameters.

v2:
-change the order of patch in the set.
-add more details in patch comment to clarify that
the rte and tespmd patch can also used in pf port reset.
-add rte_free for memory free after restore.
-change varible name of reset_number to reset_falg.
-fix patch check warning.

v3:
-fix error of author mail address

v4:
-fix typo error
-add rte_wmb() after change the reset_ports.

v5:
-add more comments for rte reset function.

zhao wei (3):
app/testpmd: add port reset command into testpmd
lib/librte_ether: add support for port reset
net/i40e: implement device reset on port

app/test-pmd/cmdline.c | 17 ++-
app/test-pmd/testpmd.c | 65 ++++++++++++
app/test-pmd/testpmd.h | 1 +
drivers/net/i40e/i40e_ethdev.c | 2 +-
drivers/net/i40e/i40e_ethdev.h | 16 +++
drivers/net/i40e/i40e_ethdev_vf.c | 185 +++++++++++++++++++++++++++++++++
drivers/net/i40e/i40e_rxtx.c | 10 ++
drivers/net/i40e/i40e_rxtx.h | 4 +
lib/librte_ether/rte_ethdev.c | 17 +++
lib/librte_ether/rte_ethdev.h | 28 +++++
lib/librte_ether/rte_ether_version.map | 6 ++
11 files changed, 346 insertions(+), 5 deletions(-)
--
2.9.3

Acked-by: Jingjing Wu <***@intel.com>
Wei Zhao
2017-04-06 06:33:45 UTC
Permalink
Add support for port reset in rte layer.This reset
feature can not only used in vf port reset in later code
develop, but alsopf port.But in this patch set, we only
limit the discussion scope to vf reset.
This patch Add an API to restart the device.
It's for VF device in this scenario, kernel PF + DPDK VF.
When the PF port down->up, APP should call this API to
restart VF port. Most likely, APP should call it in its
management thread and guarantee the thread safe. It means
APP should stop the rx/tx and the device, then restart
the device, then recover the device and rx/tx.This API can
also do some restore work for the port.

Signed-off-by: Wenzhuo Lu <***@intel.com>
Signed-off-by: Wei Zhao <***@intel.com>
---
lib/librte_ether/rte_ethdev.c | 17 +++++++++++++++++
lib/librte_ether/rte_ethdev.h | 28 ++++++++++++++++++++++++++++
lib/librte_ether/rte_ether_version.map | 6 ++++++
3 files changed, 51 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index eb0a94a..2e06dca 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3273,3 +3273,20 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
-ENOTSUP);
return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel, mask, en);
}
+
+int
+rte_eth_dev_reset(uint8_t port_id)
+{
+ struct rte_eth_dev *dev;
+ int diag;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+ dev = &rte_eth_devices[port_id];
+
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -ENOTSUP);
+
+ diag = (*dev->dev_ops->dev_reset)(dev);
+
+ return diag;
+}
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 4be217c..48c0d0b 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1367,6 +1367,9 @@ typedef int (*eth_l2_tunnel_offload_set_t)
uint8_t en);
/**< @internal enable/disable the l2 tunnel offload functions */

+typedef int (*eth_dev_reset_t)(struct rte_eth_dev *dev);
+/**< @internal Function used to reset a configured Ethernet device. */
+
#ifdef RTE_NIC_BYPASS

enum {
@@ -1509,6 +1512,9 @@ struct eth_dev_ops {
eth_l2_tunnel_offload_set_t l2_tunnel_offload_set;
/** Enable/disable l2 tunnel offload functions. */

+ /** Reset device. */
+ eth_dev_reset_t dev_reset;
+
eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue rate limit. */

rss_hash_update_t rss_hash_update; /** Configure RSS hash protocols. */
@@ -4413,6 +4419,28 @@ int
rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);

/**
+ * Reset an ethernet device when it's not working. One scenario is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup the rx/tx
+ * queues, restart the port.
+ * Before calling this API, APP should stop the rx/tx. When tx is being stopped,
+ * APP can drop the packets and release the buffer instead of sending them.
+ * This function can also do some restore work for the port, for example, it can
+ * restore the added parameters of vlan, mac_addrs, promisc_unicast_enabled flag
+ * and promisc_multicast_enabled flag.
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ *
+ * @return
+ * - (0) if successful.
+ * - (-ENODEV) if port identifier is invalid.
+ * - (-ENOTSUP) if hardware doesn't support this function.
+ */
+int
+rte_eth_dev_reset(uint8_t port_id);
+
+/**
* @internal
* Wrapper for use by pci drivers as a .probe function to attach to a ethdev
* interface.
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index c6c9d0d..529b27f 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -154,3 +154,9 @@ DPDK_17.02 {
rte_flow_validate;

} DPDK_16.11;
+
+DPDK_17.05 {
+ global:
+
+ rte_eth_dev_reset;
+} DPDK_17.02;
\ No newline at end of file
--
2.9.3
Wei Zhao
2017-04-06 06:33:47 UTC
Permalink
Add port reset command into testpmd project,
it is the interface for user to reset port.
And also it is not limit to be used only in vf reset,
but also for pf port reset.But this patch set only
realted to vf reset feature.

Signed-off-by: Wei Zhao <***@intel.com>
Signed-off-by: Wenzhuo Lu <***@intel.com>
---
app/test-pmd/cmdline.c | 17 +++++++++----
app/test-pmd/testpmd.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
app/test-pmd/testpmd.h | 1 +
3 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 47f935d..6cbdcc8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -599,6 +599,9 @@ static void cmd_help_long_parsed(void *parsed_result,
"port close (port_id|all)\n"
" Close all ports or port_id.\n\n"

+ "port reset (port_id|all)\n"
+ " Reset all ports or port_id.\n\n"
+
"port attach (ident)\n"
" Attach physical or virtual dev by pci address or virtual device name\n\n"

@@ -909,6 +912,8 @@ static void cmd_operate_port_parsed(void *parsed_result,
stop_port(RTE_PORT_ALL);
else if (!strcmp(res->name, "close"))
close_port(RTE_PORT_ALL);
+ else if (!strcmp(res->name, "reset"))
+ reset_port(RTE_PORT_ALL);
else
printf("Unknown parameter\n");
}
@@ -918,14 +923,15 @@ cmdline_parse_token_string_t cmd_operate_port_all_cmd =
"port");
cmdline_parse_token_string_t cmd_operate_port_all_port =
TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, name,
- "start#stop#close");
+ "start#stop#close#reset");
cmdline_parse_token_string_t cmd_operate_port_all_all =
TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, value, "all");

cmdline_parse_inst_t cmd_operate_port = {
.f = cmd_operate_port_parsed,
.data = NULL,
- .help_str = "port start|stop|close all: Start/Stop/Close all ports",
+ .help_str = "port start|stop|close|reset all: Start/Stop/Close/"
+ "Reset all ports",
.tokens = {
(void *)&cmd_operate_port_all_cmd,
(void *)&cmd_operate_port_all_port,
@@ -953,6 +959,8 @@ static void cmd_operate_specific_port_parsed(void *parsed_result,
stop_port(res->value);
else if (!strcmp(res->name, "close"))
close_port(res->value);
+ else if (!strcmp(res->name, "reset"))
+ reset_port(res->value);
else
printf("Unknown parameter\n");
}
@@ -962,7 +970,7 @@ cmdline_parse_token_string_t cmd_operate_specific_port_cmd =
keyword, "port");
cmdline_parse_token_string_t cmd_operate_specific_port_port =
TOKEN_STRING_INITIALIZER(struct cmd_operate_specific_port_result,
- name, "start#stop#close");
+ name, "start#stop#close#reset");
cmdline_parse_token_num_t cmd_operate_specific_port_id =
TOKEN_NUM_INITIALIZER(struct cmd_operate_specific_port_result,
value, UINT8);
@@ -970,7 +978,8 @@ cmdline_parse_token_num_t cmd_operate_specific_port_id =
cmdline_parse_inst_t cmd_operate_specific_port = {
.f = cmd_operate_specific_port_parsed,
.data = NULL,
- .help_str = "port start|stop|close <port_id>: Start/Stop/Close port_id",
+ .help_str = "port start|stop|close|reset <port_id>: Start/Stop/Close/"
+ "Reset port_id",
.tokens = {
(void *)&cmd_operate_specific_port_cmd,
(void *)&cmd_operate_specific_port_port,
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 484c19b..a4f5330 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -137,6 +137,7 @@ portid_t nb_fwd_ports; /**< Number of forwarding ports. */

unsigned int fwd_lcores_cpuids[RTE_MAX_LCORE]; /**< CPU ids configuration. */
portid_t fwd_ports_ids[RTE_MAX_ETHPORTS]; /**< Port ids configuration. */
+volatile char reset_ports[RTE_MAX_ETHPORTS] = {0}; /**< Port reset flag. */

struct fwd_stream **fwd_streams; /**< For each RX queue of each port. */
streamid_t nb_fwd_streams; /**< Is equal to (nb_ports * nb_rxq). */
@@ -1305,6 +1306,18 @@ port_is_closed(portid_t port_id)
return 1;
}

+static void
+reset_event_callback(uint8_t port_id, enum rte_eth_event_type type, void *param)
+{
+ RTE_SET_USED(param);
+
+ printf("Event type: %s on port %d\n",
+ type == RTE_ETH_EVENT_INTR_RESET ? "RESET interrupt" :
+ "unknown event", port_id);
+ reset_ports[port_id] = 1;
+ rte_wmb();
+}
+
int
start_port(portid_t pid)
{
@@ -1350,6 +1363,10 @@ start_port(portid_t pid)
return -1;
}
}
+
+ /* register reset interrupt callback */
+ rte_eth_dev_callback_register(pi, RTE_ETH_EVENT_INTR_RESET,
+ reset_event_callback, NULL);
if (port->need_reconfig_queues > 0) {
port->need_reconfig_queues = 0;
/* setup tx queues */
@@ -1559,6 +1576,54 @@ close_port(portid_t pid)
}

void
+reset_port(portid_t pid)
+{
+ portid_t pi;
+ struct rte_port *port;
+
+ if (port_id_is_invalid(pid, ENABLED_WARN))
+ return;
+
+ printf("Reset ports...\n");
+
+ FOREACH_PORT(pi, ports) {
+ if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
+ continue;
+
+ if (port_is_forwarding(pi) != 0 && test_done == 0) {
+ printf("Please remove port %d from forwarding "
+ "configuration.\n", pi);
+ continue;
+ }
+
+ if (port_is_bonding_slave(pi)) {
+ printf("Please remove port %d from "
+ "bonded device.\n", pi);
+ continue;
+ }
+
+ if (!reset_ports[pi]) {
+ printf("vf must get reset port %d info from "
+ "pf before reset.\n", pi);
+ continue;
+ }
+
+ port = &ports[pi];
+ if (rte_atomic16_cmpset(&(port->port_status),
+ RTE_PORT_STOPPED, RTE_PORT_HANDLING) == 1) {
+ printf("Port %d is not stopped\n", pi);
+ continue;
+ }
+
+ rte_eth_dev_reset(pi);
+ reset_ports[pi] = 0;
+ rte_wmb();
+ }
+
+ printf("Done\n");
+}
+
+void
attach_port(char *identifier)
{
portid_t pi = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 8cf2860..0c7e44c 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -586,6 +586,7 @@ int init_port_dcb_config(portid_t pid, enum dcb_mode_enable dcb_mode,
int start_port(portid_t pid);
void stop_port(portid_t pid);
void close_port(portid_t pid);
+void reset_port(portid_t pid);
void attach_port(char *identifier);
void detach_port(uint8_t port_id);
int all_ports_stopped(void);
--
2.9.3
Wei Zhao
2017-04-06 06:33:46 UTC
Permalink
This patch implement the device reset function on vf port.
This restart function will detach device then
attach device, reconfigure dev, re-setup
the Rx/Tx queues.

Signed-off-by: Wei Zhao <***@intel.com>
Signed-off-by: Wenzhuo Lu <***@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 2 +-
drivers/net/i40e/i40e_ethdev.h | 16 ++++
drivers/net/i40e/i40e_ethdev_vf.c | 185 ++++++++++++++++++++++++++++++++++++++
drivers/net/i40e/i40e_rxtx.c | 10 +++
drivers/net/i40e/i40e_rxtx.h | 4 +
5 files changed, 216 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 3702214..f2fdb2f 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -6020,7 +6020,7 @@ i40e_find_vlan_filter(struct i40e_vsi *vsi,
return 0;
}

-static void
+void
i40e_store_vlan_filter(struct i40e_vsi *vsi,
uint16_t vlan_id, bool on)
{
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index aebb097..515a288 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -652,6 +652,16 @@ struct i40e_vf_tx_queues {
uint32_t tx_ring_len;
};

+struct i40e_vf_reset_store {
+ struct ether_addr *mac_addrs; /* Device Ethernet Link address. */
+ bool promisc_unicast_enabled;
+ bool promisc_multicast_enabled;
+ uint16_t vlan_num; /* Total VLAN number */
+ uint32_t vfta[I40E_VFTA_SIZE]; /* VLAN bitmap */
+ uint16_t mac_num; /* Total mac number */
+};
+
+
/*
* Structure to store private data specific for VF instance.
*/
@@ -709,6 +719,10 @@ struct i40e_adapter {
struct rte_timecounter systime_tc;
struct rte_timecounter rx_tstamp_tc;
struct rte_timecounter tx_tstamp_tc;
+
+ /* For port reset */
+ volatile uint8_t reset_flag;
+ void *reset_store_data;
};

extern const struct rte_flow_ops i40e_flow_ops;
@@ -749,6 +763,8 @@ int i40e_dev_link_update(struct rte_eth_dev *dev,
__rte_unused int wait_to_complete);
void i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi);
void i40e_vsi_queues_unbind_intr(struct i40e_vsi *vsi);
+void i40e_store_vlan_filter(struct i40e_vsi *vsi,
+ uint16_t vlan_id, bool on);
int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
struct i40e_vsi_vlan_pvid_info *info);
int i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 55fd344..9ab035c 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -161,6 +161,9 @@ i40evf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id);
static void i40evf_handle_pf_event(__rte_unused struct rte_eth_dev *dev,
uint8_t *msg,
uint16_t msglen);
+static int i40evf_dev_uninit(struct rte_eth_dev *eth_dev);
+static int i40evf_dev_init(struct rte_eth_dev *eth_dev);
+static int i40evf_reset_dev(struct rte_eth_dev *dev);

/* Default hash key buffer for RSS */
static uint32_t rss_key_default[I40E_VFQF_HKEY_MAX_INDEX + 1];
@@ -230,6 +233,7 @@ static const struct eth_dev_ops i40evf_eth_dev_ops = {
.rss_hash_conf_get = i40evf_dev_rss_hash_conf_get,
.mtu_set = i40evf_dev_mtu_set,
.mac_addr_set = i40evf_set_default_mac_addr,
+ .dev_reset = i40evf_reset_dev,
};

/*
@@ -889,6 +893,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
PMD_DRV_LOG(ERR, "fail to execute command "
"OP_ADD_ETHER_ADDRESS");

+ vf->vsi.mac_num++;
return;
}

@@ -926,6 +931,7 @@ i40evf_del_mac_addr_by_addr(struct rte_eth_dev *dev,
if (err)
PMD_DRV_LOG(ERR, "fail to execute command "
"OP_DEL_ETHER_ADDRESS");
+ vf->vsi.mac_num--;
return;
}

@@ -1047,6 +1053,7 @@ static int
i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
{
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+ struct i40e_vsi *vsi = &vf->vsi;
struct i40e_virtchnl_vlan_filter_list *vlan_list;
uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
sizeof(uint16_t)];
@@ -1066,6 +1073,8 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
err = i40evf_execute_vf_cmd(dev, &args);
if (err)
PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+ i40e_store_vlan_filter(vsi, vlanid, 1);
+ vsi->vlan_num++;

return err;
}
@@ -1074,6 +1083,7 @@ static int
i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
{
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+ struct i40e_vsi *vsi = &vf->vsi;
struct i40e_virtchnl_vlan_filter_list *vlan_list;
uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
sizeof(uint16_t)];
@@ -1093,6 +1103,8 @@ i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
err = i40evf_execute_vf_cmd(dev, &args);
if (err)
PMD_DRV_LOG(ERR, "fail to execute command OP_DEL_VLAN");
+ i40e_store_vlan_filter(vsi, vlanid, 0);
+ vsi->vlan_num--;

return err;
}
@@ -2716,3 +2728,176 @@ i40evf_set_default_mac_addr(struct rte_eth_dev *dev,

i40evf_add_mac_addr(dev, mac_addr, 0, 0);
}
+
+static void
+i40evf_restore_vlan_filter(struct rte_eth_dev *dev,
+ uint32_t *vfta)
+{
+ uint32_t vid_idx, vid_bit;
+ uint16_t vlan_id;
+
+ for (vid_idx = 0; vid_idx < I40E_VFTA_SIZE; vid_idx++) {
+ for (vid_bit = 0; vid_bit < I40E_UINT32_BIT_SIZE; vid_bit++) {
+ if (vfta[vid_idx] & (1 << vid_bit)) {
+ vlan_id = (vid_idx << 5) + vid_bit;
+ i40evf_add_vlan(dev, vlan_id);
+ }
+ }
+ }
+}
+
+static void
+i40evf_restore_macaddr(struct rte_eth_dev *dev,
+ struct ether_addr *mac_addrs)
+{
+ struct ether_addr *addr;
+ uint16_t i;
+
+ /* replay MAC address configuration including default MAC */
+ addr = &mac_addrs[0];
+
+ i40evf_set_default_mac_addr(dev, addr);
+ memcpy(dev->data->mac_addrs, mac_addrs,
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+ for (i = 1; i < I40E_NUM_MACADDR_MAX; i++) {
+ addr = &mac_addrs[i];
+
+ /* skip zero address */
+ if (is_zero_ether_addr(addr))
+ continue;
+
+ i40evf_add_mac_addr(dev, addr, 0, 0);
+ }
+}
+
+
+static void
+i40e_vf_queue_reset(struct rte_eth_dev *dev)
+{
+ uint16_t i;
+
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ struct i40e_rx_queue *rxq = dev->data->rx_queues[i];
+
+ if (rxq->q_set) {
+ i40e_dev_rx_queue_setup(dev,
+ rxq->queue_id,
+ rxq->nb_rx_desc,
+ rxq->socket_id,
+ &rxq->rxconf,
+ rxq->mp);
+ }
+ }
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ struct i40e_tx_queue *txq = dev->data->tx_queues[i];
+
+ if (txq->q_set) {
+ i40e_dev_tx_queue_setup(dev,
+ txq->queue_id,
+ txq->nb_tx_desc,
+ txq->socket_id,
+ &txq->txconf);
+ }
+ }
+}
+
+static int
+i40evf_store_before_reset(struct rte_eth_dev *dev)
+{
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+ struct i40e_vf_reset_store *store_data;
+ struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+
+ adapter->reset_store_data = rte_zmalloc("i40evf_store_reset",
+ sizeof(struct i40e_vf_reset_store), 0);
+ if (adapter->reset_store_data == NULL) {
+ PMD_INIT_LOG(ERR, "Failed to allocate %ld bytes needed to"
+ " to store data when reset vf",
+ sizeof(struct i40e_vf_reset_store));
+ return -ENOMEM;
+ }
+ store_data =
+ (struct i40e_vf_reset_store *)adapter->reset_store_data;
+ store_data->mac_addrs = rte_zmalloc("i40evf_mac_store_reset",
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX, 0);
+ if (store_data->mac_addrs == NULL) {
+ PMD_INIT_LOG(ERR, "Failed to allocate %d bytes needed to"
+ " to store MAC addresses when reset vf",
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+ }
+
+ memcpy(store_data->mac_addrs, dev->data->mac_addrs,
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+ store_data->promisc_unicast_enabled = vf->promisc_unicast_enabled;
+ store_data->promisc_multicast_enabled = vf->promisc_multicast_enabled;
+
+ store_data->vlan_num = vf->vsi.vlan_num;
+ memcpy(store_data->vfta, vf->vsi.vfta,
+ sizeof(uint32_t) * I40E_VFTA_SIZE);
+
+ store_data->mac_num = vf->vsi.mac_num;
+
+ return 0;
+}
+
+static void
+i40evf_restore_after_reset(struct rte_eth_dev *dev)
+{
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+ struct i40e_vf_reset_store *store_data =
+ (struct i40e_vf_reset_store *)adapter->reset_store_data;
+
+ if (store_data->promisc_unicast_enabled)
+ i40evf_dev_promiscuous_enable(dev);
+ if (store_data->promisc_multicast_enabled)
+ i40evf_dev_allmulticast_enable(dev);
+
+ if (store_data->vlan_num)
+ i40evf_restore_vlan_filter(dev, store_data->vfta);
+
+ if (store_data->mac_num)
+ i40evf_restore_macaddr(dev, store_data->mac_addrs);
+
+ rte_free(store_data->mac_addrs);
+ rte_free(adapter->reset_store_data);
+}
+
+static int
+i40evf_reset_dev(struct rte_eth_dev *dev)
+{
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+
+ adapter->reset_flag = 1;
+ i40evf_store_before_reset(dev);
+
+ i40evf_dev_close(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev close complete");
+
+ i40evf_dev_uninit(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev detached");
+
+ memset(dev->data->dev_private, 0,
+ (uint64_t)&adapter->reset_flag - (uint64_t)adapter);
+
+ i40evf_dev_init(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev attached");
+
+ i40evf_dev_configure(dev);
+
+ i40e_vf_queue_reset(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf queue reset");
+
+ i40evf_restore_after_reset(dev);
+
+ i40evf_dev_start(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev restart");
+
+ adapter->reset_flag = 0;
+
+ return 0;
+}
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 02367b7..d891a54 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1709,6 +1709,7 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
uint16_t len, i;
uint16_t base, bsf, tc_mapping;
int use_def_burst_func = 1;
+ struct rte_eth_rxconf conf = *rx_conf;

if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
struct i40e_vf *vf =
@@ -1748,6 +1749,8 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
}
rxq->mp = mp;
rxq->nb_rx_desc = nb_desc;
+ rxq->socket_id = socket_id;
+ rxq->rxconf = conf;
rxq->rx_free_thresh = rx_conf->rx_free_thresh;
rxq->queue_id = queue_idx;
if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF)
@@ -1932,6 +1935,7 @@ i40e_dev_tx_queue_setup(struct rte_eth_dev *dev,
uint32_t ring_size;
uint16_t tx_rs_thresh, tx_free_thresh;
uint16_t i, base, bsf, tc_mapping;
+ struct rte_eth_txconf conf = *tx_conf;

if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
struct i40e_vf *vf =
@@ -2054,6 +2058,8 @@ i40e_dev_tx_queue_setup(struct rte_eth_dev *dev,
}

txq->nb_tx_desc = nb_desc;
+ txq->socket_id = socket_id;
+ txq->txconf = conf;
txq->tx_rs_thresh = tx_rs_thresh;
txq->tx_free_thresh = tx_free_thresh;
txq->pthresh = tx_conf->tx_thresh.pthresh;
@@ -2520,8 +2526,12 @@ void
i40e_dev_free_queues(struct rte_eth_dev *dev)
{
uint16_t i;
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);

PMD_INIT_FUNC_TRACE();
+ if (adapter->reset_flag)
+ return;

for (i = 0; i < dev->data->nb_rx_queues; i++) {
if (!dev->data->rx_queues[i])
diff --git a/drivers/net/i40e/i40e_rxtx.h b/drivers/net/i40e/i40e_rxtx.h
index a87bdb0..0e3cc19 100644
--- a/drivers/net/i40e/i40e_rxtx.h
+++ b/drivers/net/i40e/i40e_rxtx.h
@@ -136,6 +136,8 @@ struct i40e_rx_queue {
bool rx_deferred_start; /**< don't start this queue in dev start */
uint16_t rx_using_sse; /**<flag indicate the usage of vPMD for rx */
uint8_t dcb_tc; /**< Traffic class of rx queue */
+ uint8_t socket_id;
+ struct rte_eth_rxconf rxconf;
};

struct i40e_tx_entry {
@@ -177,6 +179,8 @@ struct i40e_tx_queue {
bool q_set; /**< indicate if tx queue has been configured */
bool tx_deferred_start; /**< don't start this queue in dev start */
uint8_t dcb_tc; /**< Traffic class of tx queue */
+ uint8_t socket_id;
+ struct rte_eth_txconf txconf;
};

/** Offload features */
--
2.9.3
Wei Zhao
2017-04-06 06:51:20 UTC
Permalink
The patches mainly finish following functions:
1) get pf reset vf comamand falg from interrupt server function.
2) reset vf port from testpmd app using a command.
3) sore and restore vf related parameters.

v2:
-change the order of patch in the set.
-add more details in patch comment to clarify that
the rte and tespmd patch can also used in pf port reset.
-add rte_free for memory free after restore.
-change varible name of reset_number to reset_falg.
-fix patch check warning.

v3:
-fix error of author mail address

v4:
-fix typo error
-add rte_wmb() after change the reset_ports.

v5:
-add more comments for rte reset function.

v6:
-fix checkpatch warning.

zhao wei (3):
app/testpmd: add port reset command into testpmd
lib/librte_ether: add support for port reset
net/i40e: implement device reset on port

app/test-pmd/cmdline.c | 17 ++-
app/test-pmd/testpmd.c | 65 ++++++++++++
app/test-pmd/testpmd.h | 1 +
drivers/net/i40e/i40e_ethdev.c | 2 +-
drivers/net/i40e/i40e_ethdev.h | 16 +++
drivers/net/i40e/i40e_ethdev_vf.c | 185 +++++++++++++++++++++++++++++++++
drivers/net/i40e/i40e_rxtx.c | 10 ++
drivers/net/i40e/i40e_rxtx.h | 4 +
lib/librte_ether/rte_ethdev.c | 17 +++
lib/librte_ether/rte_ethdev.h | 28 +++++
lib/librte_ether/rte_ether_version.map | 6 ++
11 files changed, 346 insertions(+), 5 deletions(-)
--
2.9.3

Acked-by: Jingjing Wu <***@intel.com>
Wei Zhao
2017-04-06 06:51:21 UTC
Permalink
Add support for port reset in rte layer.This reset
feature can not only used in vf port reset in later code
develop, but alsopf port.But in this patch set, we only
limit the discussion scope to vf reset.
This patch Add an API to restart the device.
It's for VF device in this scenario, kernel PF + DPDK VF.
When the PF port down->up, APP should call this API to
restart VF port. Most likely, APP should call it in its
management thread and guarantee the thread safe. It means
APP should stop the rx/tx and the device, then restart
the device, then recover the device and rx/tx.This API can
also do some restore work for the port.

Signed-off-by: Wenzhuo Lu <***@intel.com>
Signed-off-by: Wei Zhao <***@intel.com>
---
lib/librte_ether/rte_ethdev.c | 17 +++++++++++++++++
lib/librte_ether/rte_ethdev.h | 28 ++++++++++++++++++++++++++++
lib/librte_ether/rte_ether_version.map | 6 ++++++
3 files changed, 51 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index eb0a94a..2e06dca 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3273,3 +3273,20 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
-ENOTSUP);
return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel, mask, en);
}
+
+int
+rte_eth_dev_reset(uint8_t port_id)
+{
+ struct rte_eth_dev *dev;
+ int diag;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+ dev = &rte_eth_devices[port_id];
+
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -ENOTSUP);
+
+ diag = (*dev->dev_ops->dev_reset)(dev);
+
+ return diag;
+}
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 4be217c..8287c50 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1367,6 +1367,9 @@ typedef int (*eth_l2_tunnel_offload_set_t)
uint8_t en);
/**< @internal enable/disable the l2 tunnel offload functions */

+typedef int (*eth_dev_reset_t)(struct rte_eth_dev *dev);
+/**< @internal Function used to reset a configured Ethernet device. */
+
#ifdef RTE_NIC_BYPASS

enum {
@@ -1509,6 +1512,9 @@ struct eth_dev_ops {
eth_l2_tunnel_offload_set_t l2_tunnel_offload_set;
/** Enable/disable l2 tunnel offload functions. */

+ /** Reset device. */
+ eth_dev_reset_t dev_reset;
+
eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue rate limit. */

rss_hash_update_t rss_hash_update; /** Configure RSS hash protocols. */
@@ -4413,6 +4419,28 @@ int
rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);

/**
+ * Reset an ethernet device when it's not working. One scenario is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup the rx/tx
+ * queues, restart the port.
+ * Before calling this API, APP should stop the rx/tx. When tx is being stopped,
+ * APP can drop the packets and release the buffer instead of sending them.
+ * This function can also do some restore work for the port, for example, it can
+ * restore the added parameters of vlan, mac_addrs, promisc_unicast_enabled
+ * flag and promisc_multicast_enabled flag.
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ *
+ * @return
+ * - (0) if successful.
+ * - (-ENODEV) if port identifier is invalid.
+ * - (-ENOTSUP) if hardware doesn't support this function.
+ */
+int
+rte_eth_dev_reset(uint8_t port_id);
+
+/**
* @internal
* Wrapper for use by pci drivers as a .probe function to attach to a ethdev
* interface.
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index c6c9d0d..529b27f 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -154,3 +154,9 @@ DPDK_17.02 {
rte_flow_validate;

} DPDK_16.11;
+
+DPDK_17.05 {
+ global:
+
+ rte_eth_dev_reset;
+} DPDK_17.02;
\ No newline at end of file
--
2.9.3
Yang, Qiming
2017-04-07 06:58:48 UTC
Permalink
Hi, Wei
-----Original Message-----
Sent: Thursday, April 6, 2017 2:51 PM
Subject: [dpdk-dev] [PATCH v6 1/3] lib/librte_ether: add support for port
reset
Add support for port reset in rte layer.This reset feature can not only used in
vf port reset in later code develop, but alsopf port.But in this patch set, we
only limit the discussion scope to vf reset.
This patch Add an API to restart the device.
' alsopf' should add space. 'Add' should be lowercase.
It's for VF device in this scenario, kernel PF + DPDK VF.
When the PF port down->up, APP should call this API to restart VF port. Most
likely, APP should call it in its management thread and guarantee the thread
safe. It means APP should stop the rx/tx and the device, then restart the
device, then recover the device and rx/tx.This API can also do some restore
work for the port.
Please check the grammar problems in this paragraph.
---
lib/librte_ether/rte_ethdev.c | 17 +++++++++++++++++
lib/librte_ether/rte_ethdev.h | 28 ++++++++++++++++++++++++++++
lib/librte_ether/rte_ether_version.map | 6 ++++++
3 files changed, 51 insertions(+)
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index eb0a94a..2e06dca 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3273,3 +3273,20 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
-ENOTSUP);
return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel,
mask, en); }
+
+int
+rte_eth_dev_reset(uint8_t port_id)
+{
+ struct rte_eth_dev *dev;
+ int diag;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+ dev = &rte_eth_devices[port_id];
+
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -
ENOTSUP);
+
+ diag = (*dev->dev_ops->dev_reset)(dev);
+
+ return diag;
+}
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 4be217c..8287c50 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1367,6 +1367,9 @@ typedef int (*eth_l2_tunnel_offload_set_t)
uint8_t en);
+typedef int (*eth_dev_reset_t)(struct rte_eth_dev *dev); /**<
+
#ifdef RTE_NIC_BYPASS
enum {
@@ -1509,6 +1512,9 @@ struct eth_dev_ops {
eth_l2_tunnel_offload_set_t l2_tunnel_offload_set;
/** Enable/disable l2 tunnel offload functions. */
+ /** Reset device. */
+ eth_dev_reset_t dev_reset;
+
eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue rate limit. */
rss_hash_update_t rss_hash_update; /** Configure RSS hash protocols. */
@@ -4413,6 +4419,28 @@ int
rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);
/**
+ * Reset an ethernet device when it's not working. One scenario is,
+after PF
+ * port is down and up, the related VF port should be reset.
Is there have other scenario? I don't know what is 'down and up' means? Can you put it another way?
+ * The API will stop the port, clear the rx/tx queues, re-setup the
+rx/tx
+ * queues, restart the port.
+ * Before calling this API, APP should stop the rx/tx. When tx is being
+stopped,
+ * APP can drop the packets and release the buffer instead of sending them.
+ * This function can also do some restore work for the port, for
+example, it can
+ * restore the added parameters of vlan, mac_addrs,
+promisc_unicast_enabled
+ * flag and promisc_multicast_enabled flag.
+ *
+ * The port identifier of the Ethernet device.
+ *
+ * - (0) if successful.
+ * - (-ENODEV) if port identifier is invalid.
+ * - (-ENOTSUP) if hardware doesn't support this function.
+ */
+int
+rte_eth_dev_reset(uint8_t port_id);
+
+/**
* Wrapper for use by pci drivers as a .probe function to attach to a ethdev
* interface.
diff --git a/lib/librte_ether/rte_ether_version.map
b/lib/librte_ether/rte_ether_version.map
index c6c9d0d..529b27f 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -154,3 +154,9 @@ DPDK_17.02 {
rte_flow_validate;
} DPDK_16.11;
+
+DPDK_17.05 {
+
+ rte_eth_dev_reset;
+} DPDK_17.02;
\ No newline at end of file
--
2.9.3
Zhao1, Wei
2017-04-10 02:21:17 UTC
Permalink
Hi, Qiming
-----Original Message-----
From: Yang, Qiming
Sent: Friday, April 7, 2017 2:59 PM
Subject: RE: [dpdk-dev] [PATCH v6 1/3] lib/librte_ether: add support for port
reset
Hi, Wei
-----Original Message-----
Sent: Thursday, April 6, 2017 2:51 PM
Subject: [dpdk-dev] [PATCH v6 1/3] lib/librte_ether: add support for
port reset
Add support for port reset in rte layer.This reset feature can not
only used in vf port reset in later code develop, but alsopf port.But
in this patch set, we only limit the discussion scope to vf reset.
This patch Add an API to restart the device.
' alsopf' should add space. 'Add' should be lowercase.
Ok, I will fix it in next version.
It's for VF device in this scenario, kernel PF + DPDK VF.
When the PF port down->up, APP should call this API to restart VF
port. Most likely, APP should call it in its management thread and
guarantee the thread safe. It means APP should stop the rx/tx and the
device, then restart the device, then recover the device and
rx/tx.This API can also do some restore work for the port.
Please check the grammar problems in this paragraph.
Ok, I will do a double check.
---
lib/librte_ether/rte_ethdev.c | 17 +++++++++++++++++
lib/librte_ether/rte_ethdev.h | 28
++++++++++++++++++++++++++++
lib/librte_ether/rte_ether_version.map | 6 ++++++
3 files changed, 51 insertions(+)
diff --git a/lib/librte_ether/rte_ethdev.c
b/lib/librte_ether/rte_ethdev.c index eb0a94a..2e06dca 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3273,3 +3273,20 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
-ENOTSUP);
return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel,
mask,
en); }
+
+int
+rte_eth_dev_reset(uint8_t port_id)
+{
+ struct rte_eth_dev *dev;
+ int diag;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+ dev = &rte_eth_devices[port_id];
+
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -
ENOTSUP);
+
+ diag = (*dev->dev_ops->dev_reset)(dev);
+
+ return diag;
+}
diff --git a/lib/librte_ether/rte_ethdev.h
b/lib/librte_ether/rte_ethdev.h index 4be217c..8287c50 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1367,6 +1367,9 @@ typedef int (*eth_l2_tunnel_offload_set_t)
uint8_t en);
+typedef int (*eth_dev_reset_t)(struct rte_eth_dev *dev); /**<
+
#ifdef RTE_NIC_BYPASS
enum {
@@ -1509,6 +1512,9 @@ struct eth_dev_ops {
eth_l2_tunnel_offload_set_t l2_tunnel_offload_set;
/** Enable/disable l2 tunnel offload functions. */
+ /** Reset device. */
+ eth_dev_reset_t dev_reset;
+
eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue
rate
limit. */
rss_hash_update_t rss_hash_update; /** Configure RSS hash protocols. */
@@ -4413,6 +4419,28 @@ int
rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);
/**
+ * Reset an ethernet device when it's not working. One scenario is,
+after PF
+ * port is down and up, the related VF port should be reset.
Is there have other scenario? I don't know what is 'down and up' means? Can
you put it another way?
"down and up" ,you can see testpmd has the state for all DPDK bound port
The scenario for pf to reset vf is too much to list all, for example,
" ifconfig port promisc " can also cause the reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup the
+rx/tx
+ * queues, restart the port.
+ * Before calling this API, APP should stop the rx/tx. When tx is
+being stopped,
+ * APP can drop the packets and release the buffer instead of sending
them.
+ * This function can also do some restore work for the port, for
+example, it can
+ * restore the added parameters of vlan, mac_addrs,
+promisc_unicast_enabled
+ * flag and promisc_multicast_enabled flag.
+ *
+ * The port identifier of the Ethernet device.
+ *
+ * - (0) if successful.
+ * - (-ENODEV) if port identifier is invalid.
+ * - (-ENOTSUP) if hardware doesn't support this function.
+ */
+int
+rte_eth_dev_reset(uint8_t port_id);
+
+/**
* Wrapper for use by pci drivers as a .probe function to attach to a ethdev
* interface.
diff --git a/lib/librte_ether/rte_ether_version.map
b/lib/librte_ether/rte_ether_version.map
index c6c9d0d..529b27f 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -154,3 +154,9 @@ DPDK_17.02 {
rte_flow_validate;
} DPDK_16.11;
+
+DPDK_17.05 {
+
+ rte_eth_dev_reset;
+} DPDK_17.02;
\ No newline at end of file
--
2.9.3
Wei Zhao
2017-04-06 06:51:22 UTC
Permalink
This patch implement the device reset function on vf port.
This restart function will detach device then
attach device, reconfigure dev, re-setup
the Rx/Tx queues.

Signed-off-by: Wei Zhao <***@intel.com>
Signed-off-by: Wenzhuo Lu <***@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 2 +-
drivers/net/i40e/i40e_ethdev.h | 16 ++++
drivers/net/i40e/i40e_ethdev_vf.c | 185 ++++++++++++++++++++++++++++++++++++++
drivers/net/i40e/i40e_rxtx.c | 10 +++
drivers/net/i40e/i40e_rxtx.h | 4 +
5 files changed, 216 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 3702214..f2fdb2f 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -6020,7 +6020,7 @@ i40e_find_vlan_filter(struct i40e_vsi *vsi,
return 0;
}

-static void
+void
i40e_store_vlan_filter(struct i40e_vsi *vsi,
uint16_t vlan_id, bool on)
{
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index aebb097..515a288 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -652,6 +652,16 @@ struct i40e_vf_tx_queues {
uint32_t tx_ring_len;
};

+struct i40e_vf_reset_store {
+ struct ether_addr *mac_addrs; /* Device Ethernet Link address. */
+ bool promisc_unicast_enabled;
+ bool promisc_multicast_enabled;
+ uint16_t vlan_num; /* Total VLAN number */
+ uint32_t vfta[I40E_VFTA_SIZE]; /* VLAN bitmap */
+ uint16_t mac_num; /* Total mac number */
+};
+
+
/*
* Structure to store private data specific for VF instance.
*/
@@ -709,6 +719,10 @@ struct i40e_adapter {
struct rte_timecounter systime_tc;
struct rte_timecounter rx_tstamp_tc;
struct rte_timecounter tx_tstamp_tc;
+
+ /* For port reset */
+ volatile uint8_t reset_flag;
+ void *reset_store_data;
};

extern const struct rte_flow_ops i40e_flow_ops;
@@ -749,6 +763,8 @@ int i40e_dev_link_update(struct rte_eth_dev *dev,
__rte_unused int wait_to_complete);
void i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi);
void i40e_vsi_queues_unbind_intr(struct i40e_vsi *vsi);
+void i40e_store_vlan_filter(struct i40e_vsi *vsi,
+ uint16_t vlan_id, bool on);
int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
struct i40e_vsi_vlan_pvid_info *info);
int i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 55fd344..9ab035c 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -161,6 +161,9 @@ i40evf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id);
static void i40evf_handle_pf_event(__rte_unused struct rte_eth_dev *dev,
uint8_t *msg,
uint16_t msglen);
+static int i40evf_dev_uninit(struct rte_eth_dev *eth_dev);
+static int i40evf_dev_init(struct rte_eth_dev *eth_dev);
+static int i40evf_reset_dev(struct rte_eth_dev *dev);

/* Default hash key buffer for RSS */
static uint32_t rss_key_default[I40E_VFQF_HKEY_MAX_INDEX + 1];
@@ -230,6 +233,7 @@ static const struct eth_dev_ops i40evf_eth_dev_ops = {
.rss_hash_conf_get = i40evf_dev_rss_hash_conf_get,
.mtu_set = i40evf_dev_mtu_set,
.mac_addr_set = i40evf_set_default_mac_addr,
+ .dev_reset = i40evf_reset_dev,
};

/*
@@ -889,6 +893,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
PMD_DRV_LOG(ERR, "fail to execute command "
"OP_ADD_ETHER_ADDRESS");

+ vf->vsi.mac_num++;
return;
}

@@ -926,6 +931,7 @@ i40evf_del_mac_addr_by_addr(struct rte_eth_dev *dev,
if (err)
PMD_DRV_LOG(ERR, "fail to execute command "
"OP_DEL_ETHER_ADDRESS");
+ vf->vsi.mac_num--;
return;
}

@@ -1047,6 +1053,7 @@ static int
i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
{
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+ struct i40e_vsi *vsi = &vf->vsi;
struct i40e_virtchnl_vlan_filter_list *vlan_list;
uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
sizeof(uint16_t)];
@@ -1066,6 +1073,8 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
err = i40evf_execute_vf_cmd(dev, &args);
if (err)
PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+ i40e_store_vlan_filter(vsi, vlanid, 1);
+ vsi->vlan_num++;

return err;
}
@@ -1074,6 +1083,7 @@ static int
i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
{
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+ struct i40e_vsi *vsi = &vf->vsi;
struct i40e_virtchnl_vlan_filter_list *vlan_list;
uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
sizeof(uint16_t)];
@@ -1093,6 +1103,8 @@ i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
err = i40evf_execute_vf_cmd(dev, &args);
if (err)
PMD_DRV_LOG(ERR, "fail to execute command OP_DEL_VLAN");
+ i40e_store_vlan_filter(vsi, vlanid, 0);
+ vsi->vlan_num--;

return err;
}
@@ -2716,3 +2728,176 @@ i40evf_set_default_mac_addr(struct rte_eth_dev *dev,

i40evf_add_mac_addr(dev, mac_addr, 0, 0);
}
+
+static void
+i40evf_restore_vlan_filter(struct rte_eth_dev *dev,
+ uint32_t *vfta)
+{
+ uint32_t vid_idx, vid_bit;
+ uint16_t vlan_id;
+
+ for (vid_idx = 0; vid_idx < I40E_VFTA_SIZE; vid_idx++) {
+ for (vid_bit = 0; vid_bit < I40E_UINT32_BIT_SIZE; vid_bit++) {
+ if (vfta[vid_idx] & (1 << vid_bit)) {
+ vlan_id = (vid_idx << 5) + vid_bit;
+ i40evf_add_vlan(dev, vlan_id);
+ }
+ }
+ }
+}
+
+static void
+i40evf_restore_macaddr(struct rte_eth_dev *dev,
+ struct ether_addr *mac_addrs)
+{
+ struct ether_addr *addr;
+ uint16_t i;
+
+ /* replay MAC address configuration including default MAC */
+ addr = &mac_addrs[0];
+
+ i40evf_set_default_mac_addr(dev, addr);
+ memcpy(dev->data->mac_addrs, mac_addrs,
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+ for (i = 1; i < I40E_NUM_MACADDR_MAX; i++) {
+ addr = &mac_addrs[i];
+
+ /* skip zero address */
+ if (is_zero_ether_addr(addr))
+ continue;
+
+ i40evf_add_mac_addr(dev, addr, 0, 0);
+ }
+}
+
+
+static void
+i40e_vf_queue_reset(struct rte_eth_dev *dev)
+{
+ uint16_t i;
+
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ struct i40e_rx_queue *rxq = dev->data->rx_queues[i];
+
+ if (rxq->q_set) {
+ i40e_dev_rx_queue_setup(dev,
+ rxq->queue_id,
+ rxq->nb_rx_desc,
+ rxq->socket_id,
+ &rxq->rxconf,
+ rxq->mp);
+ }
+ }
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ struct i40e_tx_queue *txq = dev->data->tx_queues[i];
+
+ if (txq->q_set) {
+ i40e_dev_tx_queue_setup(dev,
+ txq->queue_id,
+ txq->nb_tx_desc,
+ txq->socket_id,
+ &txq->txconf);
+ }
+ }
+}
+
+static int
+i40evf_store_before_reset(struct rte_eth_dev *dev)
+{
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+ struct i40e_vf_reset_store *store_data;
+ struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+
+ adapter->reset_store_data = rte_zmalloc("i40evf_store_reset",
+ sizeof(struct i40e_vf_reset_store), 0);
+ if (adapter->reset_store_data == NULL) {
+ PMD_INIT_LOG(ERR, "Failed to allocate %ld bytes needed to"
+ " to store data when reset vf",
+ sizeof(struct i40e_vf_reset_store));
+ return -ENOMEM;
+ }
+ store_data =
+ (struct i40e_vf_reset_store *)adapter->reset_store_data;
+ store_data->mac_addrs = rte_zmalloc("i40evf_mac_store_reset",
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX, 0);
+ if (store_data->mac_addrs == NULL) {
+ PMD_INIT_LOG(ERR, "Failed to allocate %d bytes needed to"
+ " to store MAC addresses when reset vf",
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+ }
+
+ memcpy(store_data->mac_addrs, dev->data->mac_addrs,
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+ store_data->promisc_unicast_enabled = vf->promisc_unicast_enabled;
+ store_data->promisc_multicast_enabled = vf->promisc_multicast_enabled;
+
+ store_data->vlan_num = vf->vsi.vlan_num;
+ memcpy(store_data->vfta, vf->vsi.vfta,
+ sizeof(uint32_t) * I40E_VFTA_SIZE);
+
+ store_data->mac_num = vf->vsi.mac_num;
+
+ return 0;
+}
+
+static void
+i40evf_restore_after_reset(struct rte_eth_dev *dev)
+{
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+ struct i40e_vf_reset_store *store_data =
+ (struct i40e_vf_reset_store *)adapter->reset_store_data;
+
+ if (store_data->promisc_unicast_enabled)
+ i40evf_dev_promiscuous_enable(dev);
+ if (store_data->promisc_multicast_enabled)
+ i40evf_dev_allmulticast_enable(dev);
+
+ if (store_data->vlan_num)
+ i40evf_restore_vlan_filter(dev, store_data->vfta);
+
+ if (store_data->mac_num)
+ i40evf_restore_macaddr(dev, store_data->mac_addrs);
+
+ rte_free(store_data->mac_addrs);
+ rte_free(adapter->reset_store_data);
+}
+
+static int
+i40evf_reset_dev(struct rte_eth_dev *dev)
+{
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+
+ adapter->reset_flag = 1;
+ i40evf_store_before_reset(dev);
+
+ i40evf_dev_close(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev close complete");
+
+ i40evf_dev_uninit(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev detached");
+
+ memset(dev->data->dev_private, 0,
+ (uint64_t)&adapter->reset_flag - (uint64_t)adapter);
+
+ i40evf_dev_init(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev attached");
+
+ i40evf_dev_configure(dev);
+
+ i40e_vf_queue_reset(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf queue reset");
+
+ i40evf_restore_after_reset(dev);
+
+ i40evf_dev_start(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev restart");
+
+ adapter->reset_flag = 0;
+
+ return 0;
+}
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 02367b7..d891a54 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1709,6 +1709,7 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
uint16_t len, i;
uint16_t base, bsf, tc_mapping;
int use_def_burst_func = 1;
+ struct rte_eth_rxconf conf = *rx_conf;

if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
struct i40e_vf *vf =
@@ -1748,6 +1749,8 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
}
rxq->mp = mp;
rxq->nb_rx_desc = nb_desc;
+ rxq->socket_id = socket_id;
+ rxq->rxconf = conf;
rxq->rx_free_thresh = rx_conf->rx_free_thresh;
rxq->queue_id = queue_idx;
if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF)
@@ -1932,6 +1935,7 @@ i40e_dev_tx_queue_setup(struct rte_eth_dev *dev,
uint32_t ring_size;
uint16_t tx_rs_thresh, tx_free_thresh;
uint16_t i, base, bsf, tc_mapping;
+ struct rte_eth_txconf conf = *tx_conf;

if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
struct i40e_vf *vf =
@@ -2054,6 +2058,8 @@ i40e_dev_tx_queue_setup(struct rte_eth_dev *dev,
}

txq->nb_tx_desc = nb_desc;
+ txq->socket_id = socket_id;
+ txq->txconf = conf;
txq->tx_rs_thresh = tx_rs_thresh;
txq->tx_free_thresh = tx_free_thresh;
txq->pthresh = tx_conf->tx_thresh.pthresh;
@@ -2520,8 +2526,12 @@ void
i40e_dev_free_queues(struct rte_eth_dev *dev)
{
uint16_t i;
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);

PMD_INIT_FUNC_TRACE();
+ if (adapter->reset_flag)
+ return;

for (i = 0; i < dev->data->nb_rx_queues; i++) {
if (!dev->data->rx_queues[i])
diff --git a/drivers/net/i40e/i40e_rxtx.h b/drivers/net/i40e/i40e_rxtx.h
index a87bdb0..0e3cc19 100644
--- a/drivers/net/i40e/i40e_rxtx.h
+++ b/drivers/net/i40e/i40e_rxtx.h
@@ -136,6 +136,8 @@ struct i40e_rx_queue {
bool rx_deferred_start; /**< don't start this queue in dev start */
uint16_t rx_using_sse; /**<flag indicate the usage of vPMD for rx */
uint8_t dcb_tc; /**< Traffic class of rx queue */
+ uint8_t socket_id;
+ struct rte_eth_rxconf rxconf;
};

struct i40e_tx_entry {
@@ -177,6 +179,8 @@ struct i40e_tx_queue {
bool q_set; /**< indicate if tx queue has been configured */
bool tx_deferred_start; /**< don't start this queue in dev start */
uint8_t dcb_tc; /**< Traffic class of tx queue */
+ uint8_t socket_id;
+ struct rte_eth_txconf txconf;
};

/** Offload features */
--
2.9.3
Wei Zhao
2017-04-06 06:51:23 UTC
Permalink
Add port reset command into testpmd project,
it is the interface for user to reset port.
And also it is not limit to be used only in vf reset,
but also for pf port reset.But this patch set only
realted to vf reset feature.

Signed-off-by: Wei Zhao <***@intel.com>
Signed-off-by: Wenzhuo Lu <***@intel.com>
---
app/test-pmd/cmdline.c | 17 +++++++++----
app/test-pmd/testpmd.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
app/test-pmd/testpmd.h | 1 +
3 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 47f935d..6cbdcc8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -599,6 +599,9 @@ static void cmd_help_long_parsed(void *parsed_result,
"port close (port_id|all)\n"
" Close all ports or port_id.\n\n"

+ "port reset (port_id|all)\n"
+ " Reset all ports or port_id.\n\n"
+
"port attach (ident)\n"
" Attach physical or virtual dev by pci address or virtual device name\n\n"

@@ -909,6 +912,8 @@ static void cmd_operate_port_parsed(void *parsed_result,
stop_port(RTE_PORT_ALL);
else if (!strcmp(res->name, "close"))
close_port(RTE_PORT_ALL);
+ else if (!strcmp(res->name, "reset"))
+ reset_port(RTE_PORT_ALL);
else
printf("Unknown parameter\n");
}
@@ -918,14 +923,15 @@ cmdline_parse_token_string_t cmd_operate_port_all_cmd =
"port");
cmdline_parse_token_string_t cmd_operate_port_all_port =
TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, name,
- "start#stop#close");
+ "start#stop#close#reset");
cmdline_parse_token_string_t cmd_operate_port_all_all =
TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, value, "all");

cmdline_parse_inst_t cmd_operate_port = {
.f = cmd_operate_port_parsed,
.data = NULL,
- .help_str = "port start|stop|close all: Start/Stop/Close all ports",
+ .help_str = "port start|stop|close|reset all: Start/Stop/Close/"
+ "Reset all ports",
.tokens = {
(void *)&cmd_operate_port_all_cmd,
(void *)&cmd_operate_port_all_port,
@@ -953,6 +959,8 @@ static void cmd_operate_specific_port_parsed(void *parsed_result,
stop_port(res->value);
else if (!strcmp(res->name, "close"))
close_port(res->value);
+ else if (!strcmp(res->name, "reset"))
+ reset_port(res->value);
else
printf("Unknown parameter\n");
}
@@ -962,7 +970,7 @@ cmdline_parse_token_string_t cmd_operate_specific_port_cmd =
keyword, "port");
cmdline_parse_token_string_t cmd_operate_specific_port_port =
TOKEN_STRING_INITIALIZER(struct cmd_operate_specific_port_result,
- name, "start#stop#close");
+ name, "start#stop#close#reset");
cmdline_parse_token_num_t cmd_operate_specific_port_id =
TOKEN_NUM_INITIALIZER(struct cmd_operate_specific_port_result,
value, UINT8);
@@ -970,7 +978,8 @@ cmdline_parse_token_num_t cmd_operate_specific_port_id =
cmdline_parse_inst_t cmd_operate_specific_port = {
.f = cmd_operate_specific_port_parsed,
.data = NULL,
- .help_str = "port start|stop|close <port_id>: Start/Stop/Close port_id",
+ .help_str = "port start|stop|close|reset <port_id>: Start/Stop/Close/"
+ "Reset port_id",
.tokens = {
(void *)&cmd_operate_specific_port_cmd,
(void *)&cmd_operate_specific_port_port,
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 484c19b..a4f5330 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -137,6 +137,7 @@ portid_t nb_fwd_ports; /**< Number of forwarding ports. */

unsigned int fwd_lcores_cpuids[RTE_MAX_LCORE]; /**< CPU ids configuration. */
portid_t fwd_ports_ids[RTE_MAX_ETHPORTS]; /**< Port ids configuration. */
+volatile char reset_ports[RTE_MAX_ETHPORTS] = {0}; /**< Port reset flag. */

struct fwd_stream **fwd_streams; /**< For each RX queue of each port. */
streamid_t nb_fwd_streams; /**< Is equal to (nb_ports * nb_rxq). */
@@ -1305,6 +1306,18 @@ port_is_closed(portid_t port_id)
return 1;
}

+static void
+reset_event_callback(uint8_t port_id, enum rte_eth_event_type type, void *param)
+{
+ RTE_SET_USED(param);
+
+ printf("Event type: %s on port %d\n",
+ type == RTE_ETH_EVENT_INTR_RESET ? "RESET interrupt" :
+ "unknown event", port_id);
+ reset_ports[port_id] = 1;
+ rte_wmb();
+}
+
int
start_port(portid_t pid)
{
@@ -1350,6 +1363,10 @@ start_port(portid_t pid)
return -1;
}
}
+
+ /* register reset interrupt callback */
+ rte_eth_dev_callback_register(pi, RTE_ETH_EVENT_INTR_RESET,
+ reset_event_callback, NULL);
if (port->need_reconfig_queues > 0) {
port->need_reconfig_queues = 0;
/* setup tx queues */
@@ -1559,6 +1576,54 @@ close_port(portid_t pid)
}

void
+reset_port(portid_t pid)
+{
+ portid_t pi;
+ struct rte_port *port;
+
+ if (port_id_is_invalid(pid, ENABLED_WARN))
+ return;
+
+ printf("Reset ports...\n");
+
+ FOREACH_PORT(pi, ports) {
+ if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
+ continue;
+
+ if (port_is_forwarding(pi) != 0 && test_done == 0) {
+ printf("Please remove port %d from forwarding "
+ "configuration.\n", pi);
+ continue;
+ }
+
+ if (port_is_bonding_slave(pi)) {
+ printf("Please remove port %d from "
+ "bonded device.\n", pi);
+ continue;
+ }
+
+ if (!reset_ports[pi]) {
+ printf("vf must get reset port %d info from "
+ "pf before reset.\n", pi);
+ continue;
+ }
+
+ port = &ports[pi];
+ if (rte_atomic16_cmpset(&(port->port_status),
+ RTE_PORT_STOPPED, RTE_PORT_HANDLING) == 1) {
+ printf("Port %d is not stopped\n", pi);
+ continue;
+ }
+
+ rte_eth_dev_reset(pi);
+ reset_ports[pi] = 0;
+ rte_wmb();
+ }
+
+ printf("Done\n");
+}
+
+void
attach_port(char *identifier)
{
portid_t pi = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 8cf2860..0c7e44c 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -586,6 +586,7 @@ int init_port_dcb_config(portid_t pid, enum dcb_mode_enable dcb_mode,
int start_port(portid_t pid);
void stop_port(portid_t pid);
void close_port(portid_t pid);
+void reset_port(portid_t pid);
void attach_port(char *identifier);
void detach_port(uint8_t port_id);
int all_ports_stopped(void);
--
2.9.3
Wei Zhao
2017-04-10 03:02:26 UTC
Permalink
The patches mainly finish following functions:
1) get pf reset vf comamand falg from interrupt server function.
2) reset vf port from testpmd app using a command.
3) sore and restore vf related parameters.

v2:
-change the order of patch in the set.
-add more details in patch comment to clarify that
the rte and tespmd patch can also used in pf port reset.
-add rte_free for memory free after restore.
-change varible name of reset_number to reset_falg.
-fix patch check warning.

v3:
-fix error of author mail address

v4:
-fix typo error
-add rte_wmb() after change the reset_ports.

v5:
-add more comments for rte reset function.

v6:
-fix checkpatch warning.

v7:
-fix typo in patch commet log

zhao wei (3):
app/testpmd: add port reset command into testpmd
lib/librte_ether: add support for port reset
net/i40e: implement device reset on port

app/test-pmd/cmdline.c | 17 ++-
app/test-pmd/testpmd.c | 65 ++++++++++++
app/test-pmd/testpmd.h | 1 +
drivers/net/i40e/i40e_ethdev.c | 2 +-
drivers/net/i40e/i40e_ethdev.h | 16 +++
drivers/net/i40e/i40e_ethdev_vf.c | 185 +++++++++++++++++++++++++++++++++
drivers/net/i40e/i40e_rxtx.c | 10 ++
drivers/net/i40e/i40e_rxtx.h | 4 +
lib/librte_ether/rte_ethdev.c | 17 +++
lib/librte_ether/rte_ethdev.h | 28 +++++
lib/librte_ether/rte_ether_version.map | 6 ++
11 files changed, 346 insertions(+), 5 deletions(-)
--
2.9.3

Acked-by: Jingjing Wu <***@intel.com>
Wei Zhao
2017-04-10 03:02:27 UTC
Permalink
Add support for port reset in rte layer.This reset
feature can be used not only in vf port reset in the following
code develop process later, but also pf port.But in this patch
set, we only limit the discussion scope to vf reset.
This patch add an API to restart the device.
It's for VF device in this scenario, kernel PF + DPDK VF.
When the PF port state is down->up, APP should call this API to
restart VF port. Most likely, APP should call it in its
management thread and guarantee the thread safe. It means
APP should stop the rx/tx and the device, then restart
the device, then recover the device and rx/tx.This API can
also do some restore work for the port.

Signed-off-by: Wenzhuo Lu <***@intel.com>
Signed-off-by: Wei Zhao <***@intel.com>
---
lib/librte_ether/rte_ethdev.c | 17 +++++++++++++++++
lib/librte_ether/rte_ethdev.h | 28 ++++++++++++++++++++++++++++
lib/librte_ether/rte_ether_version.map | 6 ++++++
3 files changed, 51 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index eb0a94a..2e06dca 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3273,3 +3273,20 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
-ENOTSUP);
return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel, mask, en);
}
+
+int
+rte_eth_dev_reset(uint8_t port_id)
+{
+ struct rte_eth_dev *dev;
+ int diag;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+ dev = &rte_eth_devices[port_id];
+
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -ENOTSUP);
+
+ diag = (*dev->dev_ops->dev_reset)(dev);
+
+ return diag;
+}
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 4be217c..8287c50 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1367,6 +1367,9 @@ typedef int (*eth_l2_tunnel_offload_set_t)
uint8_t en);
/**< @internal enable/disable the l2 tunnel offload functions */

+typedef int (*eth_dev_reset_t)(struct rte_eth_dev *dev);
+/**< @internal Function used to reset a configured Ethernet device. */
+
#ifdef RTE_NIC_BYPASS

enum {
@@ -1509,6 +1512,9 @@ struct eth_dev_ops {
eth_l2_tunnel_offload_set_t l2_tunnel_offload_set;
/** Enable/disable l2 tunnel offload functions. */

+ /** Reset device. */
+ eth_dev_reset_t dev_reset;
+
eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue rate limit. */

rss_hash_update_t rss_hash_update; /** Configure RSS hash protocols. */
@@ -4413,6 +4419,28 @@ int
rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);

/**
+ * Reset an ethernet device when it's not working. One scenario is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup the rx/tx
+ * queues, restart the port.
+ * Before calling this API, APP should stop the rx/tx. When tx is being stopped,
+ * APP can drop the packets and release the buffer instead of sending them.
+ * This function can also do some restore work for the port, for example, it can
+ * restore the added parameters of vlan, mac_addrs, promisc_unicast_enabled
+ * flag and promisc_multicast_enabled flag.
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ *
+ * @return
+ * - (0) if successful.
+ * - (-ENODEV) if port identifier is invalid.
+ * - (-ENOTSUP) if hardware doesn't support this function.
+ */
+int
+rte_eth_dev_reset(uint8_t port_id);
+
+/**
* @internal
* Wrapper for use by pci drivers as a .probe function to attach to a ethdev
* interface.
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index c6c9d0d..529b27f 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -154,3 +154,9 @@ DPDK_17.02 {
rte_flow_validate;

} DPDK_16.11;
+
+DPDK_17.05 {
+ global:
+
+ rte_eth_dev_reset;
+} DPDK_17.02;
\ No newline at end of file
--
2.9.3
Thomas Monjalon
2017-04-20 20:49:58 UTC
Permalink
Post by Wei Zhao
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1509,6 +1512,9 @@ struct eth_dev_ops {
eth_l2_tunnel_offload_set_t l2_tunnel_offload_set;
/** Enable/disable l2 tunnel offload functions. */
+ /** Reset device. */
+ eth_dev_reset_t dev_reset;
+
eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue rate limit. */
rss_hash_update_t rss_hash_update; /** Configure RSS hash
This new op should be added at the end of the structure
to avoid ABI issue.
Post by Wei Zhao
rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);
/**
+ * Reset an ethernet device when it's not working. One scenario is, after
PF + * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup the rx/tx
+ * queues, restart the port.
+ * Before calling this API, APP should stop the rx/tx. When tx is being
stopped, + * APP can drop the packets and release the buffer instead of
sending them. + * This function can also do some restore work for the port,
for example, it can + * restore the added parameters of vlan, mac_addrs,
promisc_unicast_enabled + * flag and promisc_multicast_enabled flag.
+ *
+ * The port identifier of the Ethernet device.
+ *
+ * - (0) if successful.
+ * - (-ENODEV) if port identifier is invalid.
+ * - (-ENOTSUP) if hardware doesn't support this function.
+ */
+int
+rte_eth_dev_reset(uint8_t port_id);
The declarations and function definitions should be better placed
after start and stop functions.
Zhao1, Wei
2017-04-21 03:20:21 UTC
Permalink
Hi, Thomas
-----Original Message-----
Sent: Friday, April 21, 2017 4:50 AM
Subject: Re: [dpdk-dev] [PATCH v7 1/3] lib/librte_ether: add support for port
reset
Post by Wei Zhao
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1509,6 +1512,9 @@ struct eth_dev_ops {
eth_l2_tunnel_offload_set_t l2_tunnel_offload_set;
/** Enable/disable l2 tunnel offload functions. */
+ /** Reset device. */
+ eth_dev_reset_t dev_reset;
+
eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue
rate
Post by Wei Zhao
limit. */
rss_hash_update_t rss_hash_update; /** Configure RSS hash
This new op should be added at the end of the structure to avoid ABI issue.
Ok, I will change as your suggestion in next version.
Post by Wei Zhao
rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);
/**
+ * Reset an ethernet device when it's not working. One scenario is,
+ after
PF + * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup the
+ rx/tx
+ * queues, restart the port.
+ * Before calling this API, APP should stop the rx/tx. When tx is
+ being
stopped, + * APP can drop the packets and release the buffer instead
of sending them. + * This function can also do some restore work for
the port, for example, it can + * restore the added parameters of
vlan, mac_addrs, promisc_unicast_enabled + * flag and
promisc_multicast_enabled flag.
Post by Wei Zhao
+ *
+ * The port identifier of the Ethernet device.
+ *
+ * - (0) if successful.
+ * - (-ENODEV) if port identifier is invalid.
+ * - (-ENOTSUP) if hardware doesn't support this function.
+ */
+int
+rte_eth_dev_reset(uint8_t port_id);
The declarations and function definitions should be better placed after start
and stop functions.
Ok, I will change as your suggestion in next version.
Thomas Monjalon
2017-04-20 20:52:05 UTC
Permalink
Post by Wei Zhao
Add support for port reset in rte layer.This reset
feature can be used not only in vf port reset in the following
code develop process later, but also pf port.But in this patch
set, we only limit the discussion scope to vf reset.
This patch add an API to restart the device.
It's for VF device in this scenario, kernel PF + DPDK VF.
When the PF port state is down->up, APP should call this API to
restart VF port. Most likely, APP should call it in its
management thread and guarantee the thread safe. It means
APP should stop the rx/tx and the device, then restart
the device, then recover the device and rx/tx.This API can
also do some restore work for the port.
---
lib/librte_ether/rte_ethdev.c | 17 +++++++++++++++++
lib/librte_ether/rte_ethdev.h | 28 ++++++++++++++++++++++++++++
lib/librte_ether/rte_ether_version.map | 6 ++++++
3 files changed, 51 insertions(+)
This new feature should be referenced in the matrix
doc/guides/nics/features/default.ini
Wei Zhao
2017-04-10 03:02:28 UTC
Permalink
This patch implement the device reset function on vf port.
This restart function will detach device then
attach device, reconfigure dev, re-setup
the Rx/Tx queues.

Signed-off-by: Wei Zhao <***@intel.com>
Signed-off-by: Wenzhuo Lu <***@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 2 +-
drivers/net/i40e/i40e_ethdev.h | 16 ++++
drivers/net/i40e/i40e_ethdev_vf.c | 185 ++++++++++++++++++++++++++++++++++++++
drivers/net/i40e/i40e_rxtx.c | 10 +++
drivers/net/i40e/i40e_rxtx.h | 4 +
5 files changed, 216 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 3702214..f2fdb2f 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -6020,7 +6020,7 @@ i40e_find_vlan_filter(struct i40e_vsi *vsi,
return 0;
}

-static void
+void
i40e_store_vlan_filter(struct i40e_vsi *vsi,
uint16_t vlan_id, bool on)
{
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index aebb097..515a288 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -652,6 +652,16 @@ struct i40e_vf_tx_queues {
uint32_t tx_ring_len;
};

+struct i40e_vf_reset_store {
+ struct ether_addr *mac_addrs; /* Device Ethernet Link address. */
+ bool promisc_unicast_enabled;
+ bool promisc_multicast_enabled;
+ uint16_t vlan_num; /* Total VLAN number */
+ uint32_t vfta[I40E_VFTA_SIZE]; /* VLAN bitmap */
+ uint16_t mac_num; /* Total mac number */
+};
+
+
/*
* Structure to store private data specific for VF instance.
*/
@@ -709,6 +719,10 @@ struct i40e_adapter {
struct rte_timecounter systime_tc;
struct rte_timecounter rx_tstamp_tc;
struct rte_timecounter tx_tstamp_tc;
+
+ /* For port reset */
+ volatile uint8_t reset_flag;
+ void *reset_store_data;
};

extern const struct rte_flow_ops i40e_flow_ops;
@@ -749,6 +763,8 @@ int i40e_dev_link_update(struct rte_eth_dev *dev,
__rte_unused int wait_to_complete);
void i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi);
void i40e_vsi_queues_unbind_intr(struct i40e_vsi *vsi);
+void i40e_store_vlan_filter(struct i40e_vsi *vsi,
+ uint16_t vlan_id, bool on);
int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
struct i40e_vsi_vlan_pvid_info *info);
int i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 55fd344..9ab035c 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -161,6 +161,9 @@ i40evf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id);
static void i40evf_handle_pf_event(__rte_unused struct rte_eth_dev *dev,
uint8_t *msg,
uint16_t msglen);
+static int i40evf_dev_uninit(struct rte_eth_dev *eth_dev);
+static int i40evf_dev_init(struct rte_eth_dev *eth_dev);
+static int i40evf_reset_dev(struct rte_eth_dev *dev);

/* Default hash key buffer for RSS */
static uint32_t rss_key_default[I40E_VFQF_HKEY_MAX_INDEX + 1];
@@ -230,6 +233,7 @@ static const struct eth_dev_ops i40evf_eth_dev_ops = {
.rss_hash_conf_get = i40evf_dev_rss_hash_conf_get,
.mtu_set = i40evf_dev_mtu_set,
.mac_addr_set = i40evf_set_default_mac_addr,
+ .dev_reset = i40evf_reset_dev,
};

/*
@@ -889,6 +893,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
PMD_DRV_LOG(ERR, "fail to execute command "
"OP_ADD_ETHER_ADDRESS");

+ vf->vsi.mac_num++;
return;
}

@@ -926,6 +931,7 @@ i40evf_del_mac_addr_by_addr(struct rte_eth_dev *dev,
if (err)
PMD_DRV_LOG(ERR, "fail to execute command "
"OP_DEL_ETHER_ADDRESS");
+ vf->vsi.mac_num--;
return;
}

@@ -1047,6 +1053,7 @@ static int
i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
{
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+ struct i40e_vsi *vsi = &vf->vsi;
struct i40e_virtchnl_vlan_filter_list *vlan_list;
uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
sizeof(uint16_t)];
@@ -1066,6 +1073,8 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
err = i40evf_execute_vf_cmd(dev, &args);
if (err)
PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+ i40e_store_vlan_filter(vsi, vlanid, 1);
+ vsi->vlan_num++;

return err;
}
@@ -1074,6 +1083,7 @@ static int
i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
{
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+ struct i40e_vsi *vsi = &vf->vsi;
struct i40e_virtchnl_vlan_filter_list *vlan_list;
uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
sizeof(uint16_t)];
@@ -1093,6 +1103,8 @@ i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
err = i40evf_execute_vf_cmd(dev, &args);
if (err)
PMD_DRV_LOG(ERR, "fail to execute command OP_DEL_VLAN");
+ i40e_store_vlan_filter(vsi, vlanid, 0);
+ vsi->vlan_num--;

return err;
}
@@ -2716,3 +2728,176 @@ i40evf_set_default_mac_addr(struct rte_eth_dev *dev,

i40evf_add_mac_addr(dev, mac_addr, 0, 0);
}
+
+static void
+i40evf_restore_vlan_filter(struct rte_eth_dev *dev,
+ uint32_t *vfta)
+{
+ uint32_t vid_idx, vid_bit;
+ uint16_t vlan_id;
+
+ for (vid_idx = 0; vid_idx < I40E_VFTA_SIZE; vid_idx++) {
+ for (vid_bit = 0; vid_bit < I40E_UINT32_BIT_SIZE; vid_bit++) {
+ if (vfta[vid_idx] & (1 << vid_bit)) {
+ vlan_id = (vid_idx << 5) + vid_bit;
+ i40evf_add_vlan(dev, vlan_id);
+ }
+ }
+ }
+}
+
+static void
+i40evf_restore_macaddr(struct rte_eth_dev *dev,
+ struct ether_addr *mac_addrs)
+{
+ struct ether_addr *addr;
+ uint16_t i;
+
+ /* replay MAC address configuration including default MAC */
+ addr = &mac_addrs[0];
+
+ i40evf_set_default_mac_addr(dev, addr);
+ memcpy(dev->data->mac_addrs, mac_addrs,
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+ for (i = 1; i < I40E_NUM_MACADDR_MAX; i++) {
+ addr = &mac_addrs[i];
+
+ /* skip zero address */
+ if (is_zero_ether_addr(addr))
+ continue;
+
+ i40evf_add_mac_addr(dev, addr, 0, 0);
+ }
+}
+
+
+static void
+i40e_vf_queue_reset(struct rte_eth_dev *dev)
+{
+ uint16_t i;
+
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ struct i40e_rx_queue *rxq = dev->data->rx_queues[i];
+
+ if (rxq->q_set) {
+ i40e_dev_rx_queue_setup(dev,
+ rxq->queue_id,
+ rxq->nb_rx_desc,
+ rxq->socket_id,
+ &rxq->rxconf,
+ rxq->mp);
+ }
+ }
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ struct i40e_tx_queue *txq = dev->data->tx_queues[i];
+
+ if (txq->q_set) {
+ i40e_dev_tx_queue_setup(dev,
+ txq->queue_id,
+ txq->nb_tx_desc,
+ txq->socket_id,
+ &txq->txconf);
+ }
+ }
+}
+
+static int
+i40evf_store_before_reset(struct rte_eth_dev *dev)
+{
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+ struct i40e_vf_reset_store *store_data;
+ struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+
+ adapter->reset_store_data = rte_zmalloc("i40evf_store_reset",
+ sizeof(struct i40e_vf_reset_store), 0);
+ if (adapter->reset_store_data == NULL) {
+ PMD_INIT_LOG(ERR, "Failed to allocate %ld bytes needed to"
+ " to store data when reset vf",
+ sizeof(struct i40e_vf_reset_store));
+ return -ENOMEM;
+ }
+ store_data =
+ (struct i40e_vf_reset_store *)adapter->reset_store_data;
+ store_data->mac_addrs = rte_zmalloc("i40evf_mac_store_reset",
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX, 0);
+ if (store_data->mac_addrs == NULL) {
+ PMD_INIT_LOG(ERR, "Failed to allocate %d bytes needed to"
+ " to store MAC addresses when reset vf",
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+ }
+
+ memcpy(store_data->mac_addrs, dev->data->mac_addrs,
+ ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+ store_data->promisc_unicast_enabled = vf->promisc_unicast_enabled;
+ store_data->promisc_multicast_enabled = vf->promisc_multicast_enabled;
+
+ store_data->vlan_num = vf->vsi.vlan_num;
+ memcpy(store_data->vfta, vf->vsi.vfta,
+ sizeof(uint32_t) * I40E_VFTA_SIZE);
+
+ store_data->mac_num = vf->vsi.mac_num;
+
+ return 0;
+}
+
+static void
+i40evf_restore_after_reset(struct rte_eth_dev *dev)
+{
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+ struct i40e_vf_reset_store *store_data =
+ (struct i40e_vf_reset_store *)adapter->reset_store_data;
+
+ if (store_data->promisc_unicast_enabled)
+ i40evf_dev_promiscuous_enable(dev);
+ if (store_data->promisc_multicast_enabled)
+ i40evf_dev_allmulticast_enable(dev);
+
+ if (store_data->vlan_num)
+ i40evf_restore_vlan_filter(dev, store_data->vfta);
+
+ if (store_data->mac_num)
+ i40evf_restore_macaddr(dev, store_data->mac_addrs);
+
+ rte_free(store_data->mac_addrs);
+ rte_free(adapter->reset_store_data);
+}
+
+static int
+i40evf_reset_dev(struct rte_eth_dev *dev)
+{
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+
+ adapter->reset_flag = 1;
+ i40evf_store_before_reset(dev);
+
+ i40evf_dev_close(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev close complete");
+
+ i40evf_dev_uninit(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev detached");
+
+ memset(dev->data->dev_private, 0,
+ (uint64_t)&adapter->reset_flag - (uint64_t)adapter);
+
+ i40evf_dev_init(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev attached");
+
+ i40evf_dev_configure(dev);
+
+ i40e_vf_queue_reset(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf queue reset");
+
+ i40evf_restore_after_reset(dev);
+
+ i40evf_dev_start(dev);
+ PMD_DRV_LOG(DEBUG, "i40evf dev restart");
+
+ adapter->reset_flag = 0;
+
+ return 0;
+}
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 02367b7..d891a54 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1709,6 +1709,7 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
uint16_t len, i;
uint16_t base, bsf, tc_mapping;
int use_def_burst_func = 1;
+ struct rte_eth_rxconf conf = *rx_conf;

if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
struct i40e_vf *vf =
@@ -1748,6 +1749,8 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
}
rxq->mp = mp;
rxq->nb_rx_desc = nb_desc;
+ rxq->socket_id = socket_id;
+ rxq->rxconf = conf;
rxq->rx_free_thresh = rx_conf->rx_free_thresh;
rxq->queue_id = queue_idx;
if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF)
@@ -1932,6 +1935,7 @@ i40e_dev_tx_queue_setup(struct rte_eth_dev *dev,
uint32_t ring_size;
uint16_t tx_rs_thresh, tx_free_thresh;
uint16_t i, base, bsf, tc_mapping;
+ struct rte_eth_txconf conf = *tx_conf;

if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
struct i40e_vf *vf =
@@ -2054,6 +2058,8 @@ i40e_dev_tx_queue_setup(struct rte_eth_dev *dev,
}

txq->nb_tx_desc = nb_desc;
+ txq->socket_id = socket_id;
+ txq->txconf = conf;
txq->tx_rs_thresh = tx_rs_thresh;
txq->tx_free_thresh = tx_free_thresh;
txq->pthresh = tx_conf->tx_thresh.pthresh;
@@ -2520,8 +2526,12 @@ void
i40e_dev_free_queues(struct rte_eth_dev *dev)
{
uint16_t i;
+ struct i40e_adapter *adapter =
+ I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);

PMD_INIT_FUNC_TRACE();
+ if (adapter->reset_flag)
+ return;

for (i = 0; i < dev->data->nb_rx_queues; i++) {
if (!dev->data->rx_queues[i])
diff --git a/drivers/net/i40e/i40e_rxtx.h b/drivers/net/i40e/i40e_rxtx.h
index a87bdb0..0e3cc19 100644
--- a/drivers/net/i40e/i40e_rxtx.h
+++ b/drivers/net/i40e/i40e_rxtx.h
@@ -136,6 +136,8 @@ struct i40e_rx_queue {
bool rx_deferred_start; /**< don't start this queue in dev start */
uint16_t rx_using_sse; /**<flag indicate the usage of vPMD for rx */
uint8_t dcb_tc; /**< Traffic class of rx queue */
+ uint8_t socket_id;
+ struct rte_eth_rxconf rxconf;
};

struct i40e_tx_entry {
@@ -177,6 +179,8 @@ struct i40e_tx_queue {
bool q_set; /**< indicate if tx queue has been configured */
bool tx_deferred_start; /**< don't start this queue in dev start */
uint8_t dcb_tc; /**< Traffic class of tx queue */
+ uint8_t socket_id;
+ struct rte_eth_txconf txconf;
};

/** Offload features */
--
2.9.3
Thomas Monjalon
2017-04-20 21:12:08 UTC
Permalink
Post by Wei Zhao
+ if (adapter->reset_store_data == NULL) {
+ PMD_INIT_LOG(ERR, "Failed to allocate %ld bytes needed to"
+ " to store data when reset vf",
+ sizeof(struct i40e_vf_reset_store));
Compilation fails for 32-bit.
%ld must be replaced by %zu
Zhao1, Wei
2017-04-21 03:39:42 UTC
Permalink
Hi, Thomas
-----Original Message-----
Sent: Friday, April 21, 2017 5:12 AM
Subject: Re: [dpdk-dev] [PATCH v7 2/3] net/i40e: implement device reset on
port
Post by Wei Zhao
+ if (adapter->reset_store_data == NULL) {
+ PMD_INIT_LOG(ERR, "Failed to allocate %ld bytes needed
to"
Post by Wei Zhao
+ " to store data when reset vf",
+ sizeof(struct i40e_vf_reset_store));
Compilation fails for 32-bit.
%ld must be replaced by %zu
Ok, I will change as your suggestion in next version.
Thomas Monjalon
2017-04-20 21:20:13 UTC
Permalink
Post by Wei Zhao
+ memset(dev->data->dev_private, 0,
+ (uint64_t)&adapter->reset_flag - (uint64_t)adapter);
It does not compile for 32-bit.
Should it be replaced by offsetof()?

Does it mean that new fields should be added before reset_flag?
There is no comment about position importance of this field in the struct.
By the way, there is a field ptype_tbl appeared recently. Where should it
be positionned after rebase?
Wei Zhao
2017-04-10 03:02:29 UTC
Permalink
Add port reset command into testpmd project,
it is the interface for user to reset port.
And also it is not limit to be used only in vf reset,
but also for pf port reset.But this patch set only
realted to vf reset feature.

Signed-off-by: Wei Zhao <***@intel.com>
Signed-off-by: Wenzhuo Lu <***@intel.com>
---
app/test-pmd/cmdline.c | 17 +++++++++----
app/test-pmd/testpmd.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
app/test-pmd/testpmd.h | 1 +
3 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 47f935d..6cbdcc8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -599,6 +599,9 @@ static void cmd_help_long_parsed(void *parsed_result,
"port close (port_id|all)\n"
" Close all ports or port_id.\n\n"

+ "port reset (port_id|all)\n"
+ " Reset all ports or port_id.\n\n"
+
"port attach (ident)\n"
" Attach physical or virtual dev by pci address or virtual device name\n\n"

@@ -909,6 +912,8 @@ static void cmd_operate_port_parsed(void *parsed_result,
stop_port(RTE_PORT_ALL);
else if (!strcmp(res->name, "close"))
close_port(RTE_PORT_ALL);
+ else if (!strcmp(res->name, "reset"))
+ reset_port(RTE_PORT_ALL);
else
printf("Unknown parameter\n");
}
@@ -918,14 +923,15 @@ cmdline_parse_token_string_t cmd_operate_port_all_cmd =
"port");
cmdline_parse_token_string_t cmd_operate_port_all_port =
TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, name,
- "start#stop#close");
+ "start#stop#close#reset");
cmdline_parse_token_string_t cmd_operate_port_all_all =
TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, value, "all");

cmdline_parse_inst_t cmd_operate_port = {
.f = cmd_operate_port_parsed,
.data = NULL,
- .help_str = "port start|stop|close all: Start/Stop/Close all ports",
+ .help_str = "port start|stop|close|reset all: Start/Stop/Close/"
+ "Reset all ports",
.tokens = {
(void *)&cmd_operate_port_all_cmd,
(void *)&cmd_operate_port_all_port,
@@ -953,6 +959,8 @@ static void cmd_operate_specific_port_parsed(void *parsed_result,
stop_port(res->value);
else if (!strcmp(res->name, "close"))
close_port(res->value);
+ else if (!strcmp(res->name, "reset"))
+ reset_port(res->value);
else
printf("Unknown parameter\n");
}
@@ -962,7 +970,7 @@ cmdline_parse_token_string_t cmd_operate_specific_port_cmd =
keyword, "port");
cmdline_parse_token_string_t cmd_operate_specific_port_port =
TOKEN_STRING_INITIALIZER(struct cmd_operate_specific_port_result,
- name, "start#stop#close");
+ name, "start#stop#close#reset");
cmdline_parse_token_num_t cmd_operate_specific_port_id =
TOKEN_NUM_INITIALIZER(struct cmd_operate_specific_port_result,
value, UINT8);
@@ -970,7 +978,8 @@ cmdline_parse_token_num_t cmd_operate_specific_port_id =
cmdline_parse_inst_t cmd_operate_specific_port = {
.f = cmd_operate_specific_port_parsed,
.data = NULL,
- .help_str = "port start|stop|close <port_id>: Start/Stop/Close port_id",
+ .help_str = "port start|stop|close|reset <port_id>: Start/Stop/Close/"
+ "Reset port_id",
.tokens = {
(void *)&cmd_operate_specific_port_cmd,
(void *)&cmd_operate_specific_port_port,
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 484c19b..a4f5330 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -137,6 +137,7 @@ portid_t nb_fwd_ports; /**< Number of forwarding ports. */

unsigned int fwd_lcores_cpuids[RTE_MAX_LCORE]; /**< CPU ids configuration. */
portid_t fwd_ports_ids[RTE_MAX_ETHPORTS]; /**< Port ids configuration. */
+volatile char reset_ports[RTE_MAX_ETHPORTS] = {0}; /**< Port reset flag. */

struct fwd_stream **fwd_streams; /**< For each RX queue of each port. */
streamid_t nb_fwd_streams; /**< Is equal to (nb_ports * nb_rxq). */
@@ -1305,6 +1306,18 @@ port_is_closed(portid_t port_id)
return 1;
}

+static void
+reset_event_callback(uint8_t port_id, enum rte_eth_event_type type, void *param)
+{
+ RTE_SET_USED(param);
+
+ printf("Event type: %s on port %d\n",
+ type == RTE_ETH_EVENT_INTR_RESET ? "RESET interrupt" :
+ "unknown event", port_id);
+ reset_ports[port_id] = 1;
+ rte_wmb();
+}
+
int
start_port(portid_t pid)
{
@@ -1350,6 +1363,10 @@ start_port(portid_t pid)
return -1;
}
}
+
+ /* register reset interrupt callback */
+ rte_eth_dev_callback_register(pi, RTE_ETH_EVENT_INTR_RESET,
+ reset_event_callback, NULL);
if (port->need_reconfig_queues > 0) {
port->need_reconfig_queues = 0;
/* setup tx queues */
@@ -1559,6 +1576,54 @@ close_port(portid_t pid)
}

void
+reset_port(portid_t pid)
+{
+ portid_t pi;
+ struct rte_port *port;
+
+ if (port_id_is_invalid(pid, ENABLED_WARN))
+ return;
+
+ printf("Reset ports...\n");
+
+ FOREACH_PORT(pi, ports) {
+ if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
+ continue;
+
+ if (port_is_forwarding(pi) != 0 && test_done == 0) {
+ printf("Please remove port %d from forwarding "
+ "configuration.\n", pi);
+ continue;
+ }
+
+ if (port_is_bonding_slave(pi)) {
+ printf("Please remove port %d from "
+ "bonded device.\n", pi);
+ continue;
+ }
+
+ if (!reset_ports[pi]) {
+ printf("vf must get reset port %d info from "
+ "pf before reset.\n", pi);
+ continue;
+ }
+
+ port = &ports[pi];
+ if (rte_atomic16_cmpset(&(port->port_status),
+ RTE_PORT_STOPPED, RTE_PORT_HANDLING) == 1) {
+ printf("Port %d is not stopped\n", pi);
+ continue;
+ }
+
+ rte_eth_dev_reset(pi);
+ reset_ports[pi] = 0;
+ rte_wmb();
+ }
+
+ printf("Done\n");
+}
+
+void
attach_port(char *identifier)
{
portid_t pi = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 8cf2860..0c7e44c 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -586,6 +586,7 @@ int init_port_dcb_config(portid_t pid, enum dcb_mode_enable dcb_mode,
int start_port(portid_t pid);
void stop_port(portid_t pid);
void close_port(portid_t pid);
+void reset_port(portid_t pid);
void attach_port(char *identifier);
void detach_port(uint8_t port_id);
int all_ports_stopped(void);
--
2.9.3
Thomas Monjalon
2017-04-20 21:37:34 UTC
Permalink
Post by Wei Zhao
1) get pf reset vf comamand falg from interrupt server function.
2) reset vf port from testpmd app using a command.
3) sore and restore vf related parameters.
[...]
Post by Wei Zhao
app/testpmd: add port reset command into testpmd
lib/librte_ether: add support for port reset
net/i40e: implement device reset on port
There are some misses and compilation issues in this series.

As there is neither a support from other maintainers no strong nack,
I was ready to apply this series.
However after a better look and compilation issues revealed in RC2 phase,
I think it should be postponed to 17.08.
In any case, PMD maintainers are welcome to give their opinions.
Loading...