[dpdk-dev] [PATCH 0/3] drivers/net: add support for IF-MIB and EtherLike-MIB

[dpdk-dev] [PATCH 0/3] drivers/net: add support for IF-MIB and EtherLike-MIB

[Michal Jastrzebski]

Extend Intel NICs (i40e, ixgbe, e1000) xstats for IF-MIB and
EtherLike-MIB.

If-MIB xstats:
ifNumber
ifIndex
ifType
ifMtu
ifSpeed
ifPhysAddress
ifOperStatus
ifLastChange
ifHighSpeed
ifConnectorPresent
ifCounterDiscontinuityTime

EtherLike-MIB xstats:
dot3PauseOperMode
dot3StatsDuplexStatus
dot3StatsRateControlAbility
dot3StatsRateControlStatus
dot3ControlFunctionsSupported

Michal Jastrzebski (3):
  drivers/net: add support for IF-MIB and EtherLike-MIB for e1000
  drivers/net: add support for IF-MIB and EtherLike-MIB for i40e
  drivers/net: add support for IF-MIB and EtherLike-MIB for ixgbe

 drivers/net/e1000/e1000_ethdev.h |   59 ++++++++
 drivers/net/e1000/igb_ethdev.c   |  296 ++++++++++++++++++++++++++++++++++----
 drivers/net/i40e/i40e_ethdev.c   |  171 +++++++++++++++++++++-
 drivers/net/i40e/i40e_ethdev.h   |   60 ++++++++
 drivers/net/ixgbe/ixgbe_ethdev.c |  275 ++++++++++++++++++++++++++++++++---
 drivers/net/ixgbe/ixgbe_ethdev.h |   59 ++++++++
 6 files changed, 878 insertions(+), 42 deletions(-)

-- 
1.7.9.5

[dpdk-dev] [PATCH 1/3] drivers/net: add support for IF-MIB and EtherLike-MIB for e1000

[Michal Jastrzebski]

If-MIB xstats:
ifNumber
ifIndex
ifType
ifMtu
ifSpeed
ifPhysAddress
ifOperStatus
ifLastChange
ifHighSpeed
ifConnectorPresent
ifCounterDiscontinuityTime

EtherLike-MIB xstats:
dot3PauseOperMode
dot3StatsDuplexStatus
dot3StatsRateControlAbility
dot3StatsRateControlStatus
dot3ControlFunctionsSupported

Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
---
 drivers/net/e1000/e1000_ethdev.h |   59 ++++++++
 drivers/net/e1000/igb_ethdev.c   |  296 ++++++++++++++++++++++++++++++++++----
 2 files changed, 331 insertions(+), 24 deletions(-)

diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 8352d0a..d242990 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -254,6 +254,62 @@ struct e1000_filter_info {
 	struct e1000_2tuple_filter_list twotuple_list;
 };
 
+#define E1000_MIB_IF_TYPE_ETHERNETCSMACD	6
+
+enum e1000_mib_truth_value {
+	e1000_mib_truth_true = 1,
+	e1000_mib_truth_false
+};
+
+/* IF-MIB statistics */
+struct e1000_if_mib_stats {
+	uint64_t if_number;			/* ifNumber */
+	uint64_t if_index;			/* ifIndex */
+	uint64_t if_type;			/* ifType */
+	uint64_t if_mtu;			/* ifMtu */
+	uint64_t if_speed;			/* ifSpeed */
+	uint64_t if_phys_address;		/* ifPhysAddress */
+	uint64_t if_oper_status;		/* ifOperStatus */
+	uint64_t if_last_change;		/* ifLastChange */
+	uint64_t if_high_speed;			/* ifHighSpeed */
+	uint64_t if_connector_present;		/* ifConnectorPresent */
+	uint64_t if_counter_discontinuity_time;	/* ifCounterDiscontinuityTime */
+};
+
+enum e1000_dot3_pause_oper_mode {
+	e1000_dot3_pause_disabled = 1,
+	e1000_dot3_pause_enabledxmit,
+	e1000_dot3_pause_enabledrcv,
+	e1000_dot3_pause_enabledxmitandrcv
+};
+
+enum e1000_dot3_stats_duplex_status {
+	e1000_dot3_duplex_unknown = 1,
+	e1000_dot3_duplex_halfduplex,
+	e1000_dot3_duplex_fullduplex
+};
+
+enum e1000_dot3_stats_rate_control_status {
+	e1000_dot3_rate_control_off = 1,
+	e1000_dot3_rate_control_on,
+	e1000_dot3_rate_control_unknown
+};
+
+#define E1000_DOT3_CF_PAUSE	(1 << 0) /* PAUSE command implemented */
+#define E1000_DOT3_CF_MPCP	(1 << 1) /* MPCP implemented */
+#define E1000_DOT3_CF_PFC	(1 << 2) /* PFC implemented */
+
+/* Ethernet-like-MIB statistics */
+struct e1000_ether_like_mib_stats {
+	uint64_t dot3_pause_oper_mode;		/* dot3PauseOperMode */
+	uint64_t dot3_stats_duplex_status;	/* dot3StatsDuplexStatus */
+	uint64_t dot3_stats_rate_control_ability;
+	/* dot3StatsRateControlAbility */
+	uint64_t dot3_stats_rate_control_status;/* dot3StatsRateControlStatus */
+	uint64_t dot3_control_functions_supported;
+	/* dot3ControlFunctionsSupported */
+};
+
 /*
  * Structure to store private data for each driver instance (for each port).
  */
@@ -268,6 +324,9 @@ struct e1000_adapter {
 	struct rte_timecounter  systime_tc;
 	struct rte_timecounter  rx_tstamp_tc;
 	struct rte_timecounter  tx_tstamp_tc;
+	uint64_t                sys_up_time_start;
+	uint64_t                if_last_change;
+	uint64_t                if_counter_discontinuity_time;
 };
 
 #define E1000_DEV_PRIVATE(adapter) \
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index e1702d8..705b591 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -560,6 +560,45 @@ struct rte_igb_xstats_name_off {
 #define IGBVF_NB_XSTATS (sizeof(rte_igbvf_stats_strings) / \
 		sizeof(rte_igbvf_stats_strings[0]))
 
+static const struct rte_igb_xstats_name_off igb_if_mib_strings[] = {
+	{"ifNumber", offsetof(struct e1000_if_mib_stats, if_number)},
+	{"ifIndex", offsetof(struct e1000_if_mib_stats, if_index)},
+	{"ifType", offsetof(struct e1000_if_mib_stats, if_type)},
+	{"ifMtu", offsetof(struct e1000_if_mib_stats, if_mtu)},
+	{"ifSpeed", offsetof(struct e1000_if_mib_stats, if_speed)},
+	{"ifPhysAddress", offsetof(struct e1000_if_mib_stats, if_phys_address)},
+	{"ifOperStatus", offsetof(struct e1000_if_mib_stats, if_oper_status)},
+	{"ifLastChange", offsetof(struct e1000_if_mib_stats, if_last_change)},
+	{"ifHighSpeed", offsetof(struct e1000_if_mib_stats, if_high_speed)},
+	{"ifConnectorPresent", offsetof(struct e1000_if_mib_stats,
+			if_connector_present)},
+	{"ifCounterDiscontinuityTime", offsetof(struct e1000_if_mib_stats,
+			if_counter_discontinuity_time)},
+};
+
+#define IGB_NB_IF_MIB_XSTATS (sizeof(igb_if_mib_strings) / \
+		sizeof(igb_if_mib_strings[0]))
+
+static const struct rte_igb_xstats_name_off igb_ether_like_mib_strings[] = {
+	{"dot3PauseOperMode", offsetof(struct e1000_ether_like_mib_stats,
+			dot3_pause_oper_mode)},
+	{"dot3StatsDuplexStatus", offsetof(struct e1000_ether_like_mib_stats,
+			dot3_stats_duplex_status)},
+	{"dot3StatsRateControlAbility", offsetof(
+			struct e1000_ether_like_mib_stats,
+			dot3_stats_rate_control_ability)},
+	{"dot3StatsRateControlStatus", offsetof(
+			struct e1000_ether_like_mib_stats,
+			dot3_stats_rate_control_status)},
+	{"dot3ControlFunctionsSupported", offsetof(
+			struct e1000_ether_like_mib_stats,
+			dot3_control_functions_supported)},
+};
+
+#define IGB_NB_ETHER_LIKE_MIB_XSTATS \
+		(sizeof(igb_ether_like_mib_strings) / \
+		sizeof(igb_ether_like_mib_strings[0]))
+
 /**
  * Atomically reads the link status information from global
  * structure rte_eth_dev.
@@ -896,6 +935,11 @@ struct rte_igb_xstats_name_off {
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id);
 
+	/* indicate sysUpTime start */
+	adapter->sys_up_time_start = rte_rdtsc();
+	adapter->if_last_change = 0;
+	adapter->if_counter_discontinuity_time = 0;
+
 	rte_intr_callback_register(&pci_dev->intr_handle,
 				   eth_igb_interrupt_handler,
 				   (void *)eth_dev);
@@ -1059,6 +1103,11 @@ struct rte_igb_xstats_name_off {
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id, "igb_mac_82576_vf");
 
+	/* indicate sysUpTime start */
+	adapter->sys_up_time_start = rte_rdtsc();
+	adapter->if_last_change = 0;
+	adapter->if_counter_discontinuity_time = 0;
+
 	intr_handle = &pci_dev->intr_handle;
 	rte_intr_callback_register(intr_handle,
 				   eth_igbvf_interrupt_handler, eth_dev);
@@ -1830,12 +1879,17 @@ static int eth_igbvf_pci_remove(struct rte_pci_device *pci_dev)
 {
 	struct e1000_hw_stats *hw_stats =
 			E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
+	struct e1000_adapter *adapter =
+		E1000_DEV_PRIVATE(dev->data->dev_private);
 
 	/* HW registers are cleared on read */
 	eth_igb_stats_get(dev, NULL);
 
 	/* Reset software totals */
 	memset(hw_stats, 0, sizeof(*hw_stats));
+
+	adapter->if_counter_discontinuity_time =
+			rte_rdtsc() - adapter->sys_up_time_start;
 }
 
 static void
@@ -1843,31 +1897,138 @@ static int eth_igbvf_pci_remove(struct rte_pci_device *pci_dev)
 {
 	struct e1000_hw_stats *stats =
 			E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
+	struct e1000_adapter *adapter =
+		E1000_DEV_PRIVATE(dev->data->dev_private);
 
 	/* HW registers are cleared on read */
 	eth_igb_xstats_get(dev, NULL, IGB_NB_XSTATS);
 
 	/* Reset software totals */
 	memset(stats, 0, sizeof(*stats));
+
+	adapter->if_counter_discontinuity_time =
+			rte_rdtsc() - adapter->sys_up_time_start;
+}
+
+static unsigned int
+igb_xstats_calc_num(void)
+{
+	return IGB_NB_XSTATS + IGB_NB_IF_MIB_XSTATS +
+		IGB_NB_ETHER_LIKE_MIB_XSTATS;
 }
 
 static int eth_igb_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	struct rte_eth_xstat_name *xstats_names,
 	__rte_unused unsigned int size)
 {
-	unsigned i;
+	unsigned int i, count = 0;
 
 	if (xstats_names == NULL)
-		return IGB_NB_XSTATS;
+		return igb_xstats_calc_num();
 
 	/* Note: limit checked in rte_eth_xstats_names() */
 
 	for (i = 0; i < IGB_NB_XSTATS; i++) {
-		snprintf(xstats_names[i].name, sizeof(xstats_names[i].name),
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
 			 "%s", rte_igb_stats_strings[i].name);
+		count++;
+	}
+
+	/* Get stats from IF-MIB objects */
+	for (i = 0; i < IGB_NB_IF_MIB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			 "%s", igb_if_mib_strings[i].name);
+		count++;
+	}
+
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < IGB_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			 "%s", igb_ether_like_mib_strings[i].name);
+		count++;
 	}
 
-	return IGB_NB_XSTATS;
+	return count;
+}
+
+static void
+igb_read_if_mib(struct rte_eth_dev *dev, struct e1000_if_mib_stats *stats)
+{
+	struct rte_eth_dev_data *data = dev->data;
+	struct rte_device *device = dev->device;
+	struct e1000_adapter *adapter =
+			E1000_DEV_PRIVATE(dev->data->dev_private);
+
+	stats->if_number = rte_eth_dev_count();
+	stats->if_index = data->port_id + 1;
+	stats->if_type = E1000_MIB_IF_TYPE_ETHERNETCSMACD;
+	stats->if_mtu = data->mtu;
+	stats->if_speed = (data->dev_link.link_speed < (UINT32_MAX / 1000000)) ?
+			(data->dev_link.link_speed * 1000000) : UINT32_MAX;
+	stats->if_phys_address = 0;
+	ether_addr_copy(data->mac_addrs,
+			(struct ether_addr *)&stats->if_phys_address);
+	stats->if_oper_status = data->dev_link.link_status ?
+			e1000_mib_truth_true : e1000_mib_truth_false;
+	stats->if_last_change = adapter->if_last_change /
+			(rte_get_tsc_hz() * 100);
+	stats->if_high_speed = data->dev_link.link_speed;
+	if (device->devargs)
+		stats->if_connector_present =
+				(device->devargs->type == RTE_DEVTYPE_VIRTUAL) ?
+						e1000_mib_truth_false :
+						e1000_mib_truth_true;
+	else
+		stats->if_connector_present = 0;
+	stats->if_counter_discontinuity_time =
+			adapter->if_counter_discontinuity_time /
+			(rte_get_tsc_hz() * 100);
+}
+
+static void
+igb_read_ether_like_mib(struct rte_eth_dev *dev,
+		struct e1000_ether_like_mib_stats *stats)
+{
+	struct rte_eth_dev_data *data = dev->data;
+	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	switch (hw->fc.current_mode) {
+	case e1000_fc_none:
+		stats->dot3_pause_oper_mode = e1000_dot3_pause_disabled;
+		break;
+	case e1000_fc_rx_pause:
+		stats->dot3_pause_oper_mode = e1000_dot3_pause_enabledrcv;
+		break;
+	case e1000_fc_tx_pause:
+		stats->dot3_pause_oper_mode = e1000_dot3_pause_enabledxmit;
+		break;
+	case e1000_fc_full:
+		stats->dot3_pause_oper_mode =
+				e1000_dot3_pause_enabledxmitandrcv;
+		break;
+	default:
+		stats->dot3_pause_oper_mode = 0;
+		break;
+	}
+
+	switch (data->dev_link.link_duplex) {
+	case ETH_LINK_FULL_DUPLEX:
+		stats->dot3_stats_duplex_status = e1000_dot3_duplex_fullduplex;
+		break;
+	case ETH_LINK_HALF_DUPLEX:
+		stats->dot3_stats_duplex_status = e1000_dot3_duplex_halfduplex;
+		break;
+	default:
+		stats->dot3_stats_duplex_status = e1000_dot3_duplex_unknown;
+		break;
+	}
+
+	stats->dot3_stats_rate_control_ability = e1000_mib_truth_false;
+	stats->dot3_stats_rate_control_status = e1000_dot3_rate_control_off;
+	stats->dot3_control_functions_supported = E1000_DOT3_CF_PAUSE;
 }
 
 static int eth_igb_xstats_get_names_by_id(struct rte_eth_dev *dev,
@@ -1912,27 +2073,53 @@ static int eth_igb_xstats_get_names_by_id(struct rte_eth_dev *dev,
 	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct e1000_hw_stats *hw_stats =
 			E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
-	unsigned i;
+	struct e1000_if_mib_stats if_mib_stats;
+	struct e1000_ether_like_mib_stats ether_like_mib_stats;
+	unsigned int i, count;
 
-	if (n < IGB_NB_XSTATS)
-		return IGB_NB_XSTATS;
+	count = igb_xstats_calc_num();
+	if (n < count)
+		return count;
 
 	igb_read_stats_registers(hw, hw_stats);
 
+	igb_read_if_mib(dev, &if_mib_stats);
+	igb_read_ether_like_mib(dev, &ether_like_mib_stats);
+
 	/* If this is a reset xstats is NULL, and we have cleared the
 	 * registers by reading them.
 	 */
 	if (!xstats)
 		return 0;
 
+	count = 0;
+
 	/* Extended stats */
 	for (i = 0; i < IGB_NB_XSTATS; i++) {
-		xstats[i].id = i;
-		xstats[i].value = *(uint64_t *)(((char *)hw_stats) +
+		xstats[count].id = count;
+		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 			rte_igb_stats_strings[i].offset);
+		count++;
+	}
+
+	/* Get stats from IF-MIB objects */
+	for (i = 0; i < IGB_NB_IF_MIB_XSTATS; i++) {
+		xstats[count].value = *(uint64_t *)(((char *)&if_mib_stats) +
+			igb_if_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
 	}
 
-	return IGB_NB_XSTATS;
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < IGB_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		xstats[count].value =
+			*(uint64_t *)(((char *)&ether_like_mib_stats) +
+			igb_ether_like_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
+
+	return count;
 }
 
 static int
@@ -2022,19 +2209,46 @@ static int eth_igb_xstats_get_names_by_id(struct rte_eth_dev *dev,
 	    hw_stats->last_gotlbc, hw_stats->gotlbc);
 }
 
+static unsigned int
+igbvf_xstats_calc_num(void)
+{
+	return IGBVF_NB_XSTATS + IGB_NB_IF_MIB_XSTATS +
+		IGB_NB_ETHER_LIKE_MIB_XSTATS;
+}
+
 static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 				     struct rte_eth_xstat_name *xstats_names,
 				     __rte_unused unsigned limit)
 {
-	unsigned i;
+	unsigned int i, count = 0;
 
-	if (xstats_names != NULL)
-		for (i = 0; i < IGBVF_NB_XSTATS; i++) {
-			snprintf(xstats_names[i].name,
-				sizeof(xstats_names[i].name), "%s",
-				rte_igbvf_stats_strings[i].name);
-		}
-	return IGBVF_NB_XSTATS;
+	if (xstats_names == NULL)
+		return igbvf_xstats_calc_num();
+
+	for (i = 0; i < IGBVF_NB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name), "%s",
+			rte_igbvf_stats_strings[i].name);
+		count++;
+	}
+
+	/* Get stats from IF-MIB objects */
+	for (i = 0; i < IGB_NB_IF_MIB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			 "%s", igb_if_mib_strings[i].name);
+		count++;
+	}
+
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < IGB_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			 "%s", igb_ether_like_mib_strings[i].name);
+		count++;
+	}
+
+	return count;
 }
 
 static int
@@ -2044,23 +2258,49 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct e1000_vf_stats *hw_stats = (struct e1000_vf_stats *)
 			E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
-	unsigned i;
+	struct e1000_if_mib_stats if_mib_stats;
+	struct e1000_ether_like_mib_stats ether_like_mib_stats;
+	unsigned int i, count;
 
-	if (n < IGBVF_NB_XSTATS)
-		return IGBVF_NB_XSTATS;
+	count = igbvf_xstats_calc_num();
+	if (n < count)
+		return count;
 
 	igbvf_read_stats_registers(hw, hw_stats);
 
+	igb_read_if_mib(dev, &if_mib_stats);
+	igb_read_ether_like_mib(dev, &ether_like_mib_stats);
+
 	if (!xstats)
 		return 0;
 
+	count = 0;
+
 	for (i = 0; i < IGBVF_NB_XSTATS; i++) {
-		xstats[i].id = i;
-		xstats[i].value = *(uint64_t *)(((char *)hw_stats) +
+		xstats[count].id = count;
+		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 			rte_igbvf_stats_strings[i].offset);
+		count++;
 	}
 
-	return IGBVF_NB_XSTATS;
+	/* Get stats from IF-MIB objects */
+	for (i = 0; i < IGB_NB_IF_MIB_XSTATS; i++) {
+		xstats[count].value = *(uint64_t *)(((char *)&if_mib_stats) +
+			igb_if_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
+
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < IGB_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		xstats[count].value =
+			*(uint64_t *)(((char *)&ether_like_mib_stats) +
+			igb_ether_like_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
+
+	return count;
 }
 
 static void
@@ -2086,6 +2326,8 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 {
 	struct e1000_vf_stats *hw_stats = (struct e1000_vf_stats*)
 			E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
+	struct e1000_adapter *adapter =
+		E1000_DEV_PRIVATE(dev->data->dev_private);
 
 	/* Sync HW register to the last stats */
 	eth_igbvf_stats_get(dev, NULL);
@@ -2093,6 +2335,9 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	/* reset HW current stats*/
 	memset(&hw_stats->gprc, 0, sizeof(*hw_stats) -
 	       offsetof(struct e1000_vf_stats, gprc));
+
+	adapter->if_counter_discontinuity_time =
+			rte_rdtsc() - adapter->sys_up_time_start;
 }
 
 static int
@@ -2338,6 +2583,8 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 {
 	struct e1000_hw *hw =
 		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct e1000_adapter *adapter =
+		E1000_DEV_PRIVATE(dev->data->dev_private);
 	struct rte_eth_link link, old;
 	int link_check, count;
 
@@ -2406,6 +2653,7 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 		return -1;
 
 	/* changed */
+	adapter->if_last_change = rte_rdtsc() - adapter->sys_up_time_start;
 	return 0;
 }
 
-- 
1.7.9.5

[dpdk-dev] [PATCH 2/3] drivers/net: add support for IF-MIB and EtherLike-MIB for i40e

[Michal Jastrzebski]

If-MIB xstats:
ifNumber
ifIndex
ifType
ifMtu
ifSpeed
ifPhysAddress
ifOperStatus
ifLastChange
ifHighSpeed
ifConnectorPresent
ifCounterDiscontinuityTime

EtherLike-MIB xstats:
dot3PauseOperMode
dot3StatsDuplexStatus
dot3StatsRateControlAbility
dot3StatsRateControlStatus
dot3ControlFunctionsSupported

Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c |  171 +++++++++++++++++++++++++++++++++++++++-
 drivers/net/i40e/i40e_ethdev.h |   60 ++++++++++++++
 2 files changed, 230 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 4c49673..52bc566 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -630,6 +630,45 @@ struct rte_i40e_xstats_name_off {
 #define I40E_NB_TXQ_PRIO_XSTATS (sizeof(rte_i40e_txq_prio_strings) / \
 		sizeof(rte_i40e_txq_prio_strings[0]))
 
+static const struct rte_i40e_xstats_name_off i40e_if_mib_strings[] = {
+	{"ifNumber", offsetof(struct i40e_if_mib_stats, if_number)},
+	{"ifIndex", offsetof(struct i40e_if_mib_stats, if_index)},
+	{"ifType", offsetof(struct i40e_if_mib_stats, if_type)},
+	{"ifMtu", offsetof(struct i40e_if_mib_stats, if_mtu)},
+	{"ifSpeed", offsetof(struct i40e_if_mib_stats, if_speed)},
+	{"ifPhysAddress", offsetof(struct i40e_if_mib_stats, if_phys_address)},
+	{"ifOperStatus", offsetof(struct i40e_if_mib_stats, if_oper_status)},
+	{"ifLastChange", offsetof(struct i40e_if_mib_stats, if_last_change)},
+	{"ifHighSpeed", offsetof(struct i40e_if_mib_stats, if_high_speed)},
+	{"ifConnectorPresent", offsetof(struct i40e_if_mib_stats,
+			if_connector_present)},
+	{"ifCounterDiscontinuityTime", offsetof(struct i40e_if_mib_stats,
+			if_counter_discontinuity_time)},
+};
+
+#define I40E_NB_IF_MIB_XSTATS (sizeof(i40e_if_mib_strings) / \
+		sizeof(i40e_if_mib_strings[0]))
+
+static const struct rte_i40e_xstats_name_off i40e_ether_like_mib_strings[] = {
+	{"dot3PauseOperMode", offsetof(struct i40e_ether_like_mib_stats,
+			dot3_pause_oper_mode)},
+	{"dot3StatsDuplexStatus", offsetof(struct i40e_ether_like_mib_stats,
+			dot3_stats_duplex_status)},
+	{"dot3StatsRateControlAbility", offsetof(
+			struct i40e_ether_like_mib_stats,
+			dot3_stats_rate_control_ability)},
+	{"dot3StatsRateControlStatus", offsetof(
+			struct i40e_ether_like_mib_stats,
+			dot3_stats_rate_control_status)},
+	{"dot3ControlFunctionsSupported", offsetof(
+			struct i40e_ether_like_mib_stats,
+			dot3_control_functions_supported)},
+};
+
+#define I40E_NB_ETHER_LIKE_MIB_XSTATS \
+		(sizeof(i40e_ether_like_mib_strings) / \
+		sizeof(i40e_ether_like_mib_strings[0]))
+
 static int eth_i40e_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	struct rte_pci_device *pci_dev)
 {
@@ -1273,6 +1312,11 @@ static inline void i40e_GLQF_reg_init(struct i40e_hw *hw)
 	/* initialize pf host driver to setup SRIOV resource if applicable */
 	i40e_pf_host_init(dev);
 
+	/* indicate sysUpTime start */
+	pf->adapter->sys_up_time_start = rte_rdtsc();
+	pf->adapter->if_last_change = 0;
+	pf->adapter->if_counter_discontinuity_time = 0;
+
 	/* register callback func to eal lib */
 	rte_intr_callback_register(intr_handle,
 				   i40e_dev_interrupt_handler, dev);
@@ -2235,6 +2279,8 @@ static inline void i40e_GLQF_reg_init(struct i40e_hw *hw)
 #define CHECK_INTERVAL 100  /* 100ms */
 #define MAX_REPEAT_TIME 10  /* 1s (10 * 100ms) in total */
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct i40e_adapter *adapter =
+		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct i40e_link_status link_status;
 	struct rte_eth_link link, old;
 	int status;
@@ -2303,6 +2349,7 @@ static inline void i40e_GLQF_reg_init(struct i40e_hw *hw)
 	if (link.link_status == old.link_status)
 		return -1;
 
+	adapter->if_last_change = rte_rdtsc() - adapter->sys_up_time_start;
 	i40e_notify_all_vfs_link_status(dev);
 
 	return 0;
@@ -2683,6 +2730,9 @@ static inline void i40e_GLQF_reg_init(struct i40e_hw *hw)
 
 	/* read the stats, reading current register values into offset */
 	i40e_read_stats_registers(pf, hw);
+
+	pf->adapter->if_counter_discontinuity_time =
+			rte_rdtsc() - pf->adapter->sys_up_time_start;
 }
 
 static uint32_t
@@ -2690,7 +2740,8 @@ static inline void i40e_GLQF_reg_init(struct i40e_hw *hw)
 {
 	return I40E_NB_ETH_XSTATS + I40E_NB_HW_PORT_XSTATS +
 		(I40E_NB_RXQ_PRIO_XSTATS * 8) +
-		(I40E_NB_TXQ_PRIO_XSTATS * 8);
+		(I40E_NB_TXQ_PRIO_XSTATS * 8) +
+		I40E_NB_IF_MIB_XSTATS + I40E_NB_ETHER_LIKE_MIB_XSTATS;
 }
 
 static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
@@ -2740,9 +2791,105 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 			count++;
 		}
 	}
+
+	/* Get stats from IF-MIB objects */
+	for (i = 0; i < I40E_NB_IF_MIB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			 "%s", i40e_if_mib_strings[i].name);
+		count++;
+	}
+
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < I40E_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			 "%s", i40e_ether_like_mib_strings[i].name);
+		count++;
+	}
+
 	return count;
 }
 
+static void
+i40e_read_if_mib(struct rte_eth_dev *dev, struct i40e_if_mib_stats *stats)
+{
+	struct rte_eth_dev_data *data = dev->data;
+	struct rte_device *device = dev->device;
+	struct i40e_adapter *adapter =
+			I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+
+	stats->if_number = rte_eth_dev_count();
+	stats->if_index = data->port_id + 1;
+	stats->if_type = I40E_MIB_IF_TYPE_ETHERNETCSMACD;
+	stats->if_mtu = data->mtu;
+	stats->if_speed = (data->dev_link.link_speed < (UINT32_MAX / 1000000)) ?
+			(data->dev_link.link_speed * 1000000) : UINT32_MAX;
+	stats->if_phys_address = 0;
+	ether_addr_copy(data->mac_addrs,
+			(struct ether_addr *)&stats->if_phys_address);
+	stats->if_oper_status = data->dev_link.link_status ?
+			i40e_mib_truth_true : i40e_mib_truth_false;
+	stats->if_last_change = adapter->if_last_change /
+			(rte_get_tsc_hz() * 100);
+	stats->if_high_speed = data->dev_link.link_speed;
+	if (device->devargs)
+		stats->if_connector_present =
+				(device->devargs->type == RTE_DEVTYPE_VIRTUAL) ?
+						i40e_mib_truth_false :
+						i40e_mib_truth_true;
+	else
+		stats->if_connector_present = 0;
+	stats->if_counter_discontinuity_time =
+			adapter->if_counter_discontinuity_time /
+			(rte_get_tsc_hz() * 100);
+}
+
+static void
+i40e_read_ether_like_mib(struct rte_eth_dev *dev,
+		struct i40e_ether_like_mib_stats *stats)
+{
+	struct rte_eth_dev_data *data = dev->data;
+	struct i40e_hw *hw =
+			I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	switch (hw->fc.current_mode) {
+	case I40E_FC_NONE:
+		stats->dot3_pause_oper_mode = i40e_dot3_pause_disabled;
+		break;
+	case I40E_FC_RX_PAUSE:
+		stats->dot3_pause_oper_mode = i40e_dot3_pause_enabledrcv;
+		break;
+	case I40E_FC_TX_PAUSE:
+		stats->dot3_pause_oper_mode = i40e_dot3_pause_enabledxmit;
+		break;
+	case I40E_FC_FULL:
+		stats->dot3_pause_oper_mode =
+				i40e_dot3_pause_enabledxmitandrcv;
+		break;
+	default:
+		stats->dot3_pause_oper_mode = 0;
+		break;
+	}
+
+	switch (data->dev_link.link_duplex) {
+	case ETH_LINK_FULL_DUPLEX:
+		stats->dot3_stats_duplex_status = i40e_dot3_duplex_fullduplex;
+		break;
+	case ETH_LINK_HALF_DUPLEX:
+		stats->dot3_stats_duplex_status = i40e_dot3_duplex_halfduplex;
+		break;
+	default:
+		stats->dot3_stats_duplex_status = i40e_dot3_duplex_unknown;
+		break;
+	}
+
+	stats->dot3_stats_rate_control_ability = i40e_mib_truth_false;
+	stats->dot3_stats_rate_control_status = i40e_dot3_rate_control_off;
+	stats->dot3_control_functions_supported = I40E_DOT3_CF_PAUSE |
+			I40E_DOT3_CF_PFC;
+}
+
 static int
 i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 		    unsigned n)
@@ -2751,6 +2898,8 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	unsigned i, count, prio;
 	struct i40e_hw_port_stats *hw_stats = &pf->stats;
+	struct i40e_if_mib_stats if_mib_stats;
+	struct i40e_ether_like_mib_stats ether_like_mib_stats;
 
 	count = i40e_xstats_calc_num();
 	if (n < count)
@@ -2758,6 +2907,9 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 
 	i40e_read_stats_registers(pf, hw);
 
+	i40e_read_if_mib(dev, &if_mib_stats);
+	i40e_read_ether_like_mib(dev, &ether_like_mib_stats);
+
 	if (xstats == NULL)
 		return 0;
 
@@ -2801,6 +2953,23 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 		}
 	}
 
+	/* Get stats from IF-MIB objects */
+	for (i = 0; i < I40E_NB_IF_MIB_XSTATS; i++) {
+		xstats[count].value = *(uint64_t *)(((char *)&if_mib_stats) +
+			i40e_if_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
+
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < I40E_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		xstats[count].value =
+			*(uint64_t *)(((char *)&ether_like_mib_stats) +
+			i40e_ether_like_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
+
 	return count;
 }
 
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 2ff8282..7a4d9ac 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -625,6 +625,62 @@ struct rte_flow {
 
 TAILQ_HEAD(i40e_flow_list, rte_flow);
 
+#define I40E_MIB_IF_TYPE_ETHERNETCSMACD		6
+
+enum i40e_mib_truth_value {
+	i40e_mib_truth_true = 1,
+	i40e_mib_truth_false
+};
+
+/* IF-MIB statistics */
+struct i40e_if_mib_stats {
+	uint64_t if_number;			/* ifNumber */
+	uint64_t if_index;			/* ifIndex */
+	uint64_t if_type;			/* ifType */
+	uint64_t if_mtu;			/* ifMtu */
+	uint64_t if_speed;			/* ifSpeed */
+	uint64_t if_phys_address;		/* ifPhysAddress */
+	uint64_t if_oper_status;		/* ifOperStatus */
+	uint64_t if_last_change;		/* ifLastChange */
+	uint64_t if_high_speed;			/* ifHighSpeed */
+	uint64_t if_connector_present;		/* ifConnectorPresent */
+	uint64_t if_counter_discontinuity_time;	/* ifCounterDiscontinuityTime */
+};
+
+enum i40e_dot3_pause_oper_mode {
+	i40e_dot3_pause_disabled = 1,
+	i40e_dot3_pause_enabledxmit,
+	i40e_dot3_pause_enabledrcv,
+	i40e_dot3_pause_enabledxmitandrcv
+};
+
+enum i40e_dot3_stats_duplex_status {
+	i40e_dot3_duplex_unknown = 1,
+	i40e_dot3_duplex_halfduplex,
+	i40e_dot3_duplex_fullduplex
+};
+
+enum i40e_dot3_stats_rate_control_status {
+	i40e_dot3_rate_control_off = 1,
+	i40e_dot3_rate_control_on,
+	i40e_dot3_rate_control_unknown
+};
+
+#define I40E_DOT3_CF_PAUSE	(1 << 0) /* PAUSE command implemented */
+#define I40E_DOT3_CF_MPCP	(1 << 1) /* MPCP implemented */
+#define I40E_DOT3_CF_PFC	(1 << 2) /* PFC implemented */
+
+/* Ethernet-like-MIB statistics */
+struct i40e_ether_like_mib_stats {
+	uint64_t dot3_pause_oper_mode;		/* dot3PauseOperMode */
+	uint64_t dot3_stats_duplex_status;	/* dot3StatsDuplexStatus */
+	uint64_t dot3_stats_rate_control_ability;
+	/* dot3StatsRateControlAbility */
+	uint64_t dot3_stats_rate_control_status;/* dot3StatsRateControlStatus */
+	uint64_t dot3_control_functions_supported;
+	/* dot3ControlFunctionsSupported */
+};
+
 /*
  * Structure to store private data specific for PF instance.
  */
@@ -778,6 +834,10 @@ struct i40e_adapter {
 	struct rte_timecounter rx_tstamp_tc;
 	struct rte_timecounter tx_tstamp_tc;
 
+	uint64_t sys_up_time_start;
+	uint64_t if_last_change;
+	uint64_t if_counter_discontinuity_time;
+
 	/* ptype mapping table */
 	uint32_t ptype_tbl[I40E_MAX_PKT_TYPE] __rte_cache_min_aligned;
 };
-- 
1.7.9.5

[dpdk-dev] [PATCH 3/3] drivers/net: add support for IF-MIB and EtherLike-MIB for ixgbe

[Michal Jastrzebski]

If-MIB xstats:
ifNumber
ifIndex
ifType
ifMtu
ifSpeed
ifPhysAddress
ifOperStatus
ifLastChange
ifHighSpeed
ifConnectorPresent
ifCounterDiscontinuityTime

EtherLike-MIB xstats:
dot3PauseOperMode
dot3StatsDuplexStatus
dot3StatsRateControlAbility
dot3StatsRateControlStatus
dot3ControlFunctionsSupported

Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c |  275 +++++++++++++++++++++++++++++++++++---
 drivers/net/ixgbe/ixgbe_ethdev.h |   59 ++++++++
 2 files changed, 317 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 2083cde..c2a7000 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -820,6 +820,45 @@ struct rte_ixgbe_xstats_name_off {
 #define IXGBEVF_NB_XSTATS (sizeof(rte_ixgbevf_stats_strings) /	\
 		sizeof(rte_ixgbevf_stats_strings[0]))
 
+static const struct rte_ixgbe_xstats_name_off ixgbe_if_mib_strings[] = {
+	{"ifNumber", offsetof(struct ixgbe_if_mib_stats, if_number)},
+	{"ifIndex", offsetof(struct ixgbe_if_mib_stats, if_index)},
+	{"ifType", offsetof(struct ixgbe_if_mib_stats, if_type)},
+	{"ifMtu", offsetof(struct ixgbe_if_mib_stats, if_mtu)},
+	{"ifSpeed", offsetof(struct ixgbe_if_mib_stats, if_speed)},
+	{"ifPhysAddress", offsetof(struct ixgbe_if_mib_stats, if_phys_address)},
+	{"ifOperStatus", offsetof(struct ixgbe_if_mib_stats, if_oper_status)},
+	{"ifLastChange", offsetof(struct ixgbe_if_mib_stats, if_last_change)},
+	{"ifHighSpeed", offsetof(struct ixgbe_if_mib_stats, if_high_speed)},
+	{"ifConnectorPresent", offsetof(struct ixgbe_if_mib_stats,
+			if_connector_present)},
+	{"ifCounterDiscontinuityTime", offsetof(struct ixgbe_if_mib_stats,
+			if_counter_discontinuity_time)},
+};
+
+#define IXGBE_NB_IF_MIB_XSTATS (sizeof(ixgbe_if_mib_strings) / \
+		sizeof(ixgbe_if_mib_strings[0]))
+
+static const struct rte_ixgbe_xstats_name_off ixgbe_ether_like_mib_strings[] = {
+	{"dot3PauseOperMode", offsetof(struct ixgbe_ether_like_mib_stats,
+			dot3_pause_oper_mode)},
+	{"dot3StatsDuplexStatus", offsetof(struct ixgbe_ether_like_mib_stats,
+			dot3_stats_duplex_status)},
+	{"dot3StatsRateControlAbility", offsetof(
+			struct ixgbe_ether_like_mib_stats,
+			dot3_stats_rate_control_ability)},
+	{"dot3StatsRateControlStatus", offsetof(
+			struct ixgbe_ether_like_mib_stats,
+			dot3_stats_rate_control_status)},
+	{"dot3ControlFunctionsSupported", offsetof(
+			struct ixgbe_ether_like_mib_stats,
+			dot3_control_functions_supported)},
+};
+
+#define IXGBE_NB_ETHER_LIKE_MIB_XSTATS \
+		(sizeof(ixgbe_ether_like_mib_strings) / \
+		sizeof(ixgbe_ether_like_mib_strings[0]))
+
 /**
  * Atomically reads the link status information from global
  * structure rte_eth_dev.
@@ -1145,6 +1184,8 @@ struct rte_ixgbe_xstats_name_off {
 		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(eth_dev->data->dev_private);
 	struct ixgbe_bw_conf *bw_conf =
 		IXGBE_DEV_PRIVATE_TO_BW_CONF(eth_dev->data->dev_private);
+	struct ixgbe_adapter *adapter =
+			(struct ixgbe_adapter *)eth_dev->data->dev_private;
 	uint32_t ctrl_ext;
 	uint16_t csum;
 	int diag, i;
@@ -1327,6 +1368,11 @@ struct rte_ixgbe_xstats_name_off {
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id);
 
+	/* indicate sysUpTime start */
+	adapter->sys_up_time_start = rte_rdtsc();
+	adapter->if_last_change = 0;
+	adapter->if_counter_discontinuity_time = 0;
+
 	rte_intr_callback_register(intr_handle,
 				   ixgbe_dev_interrupt_handler, eth_dev);
 
@@ -1607,6 +1653,8 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
 	struct ixgbe_hwstrip *hwstrip =
 		IXGBE_DEV_PRIVATE_TO_HWSTRIP_BITMAP(eth_dev->data->dev_private);
 	struct ether_addr *perm_addr = (struct ether_addr *) hw->mac.perm_addr;
+	struct ixgbe_adapter *adapter =
+			(struct ixgbe_adapter *)eth_dev->data->dev_private;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -1730,6 +1778,11 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
 		return -EIO;
 	}
 
+	/* indicate sysUpTime start */
+	adapter->sys_up_time_start = rte_rdtsc();
+	adapter->if_last_change = 0;
+	adapter->if_counter_discontinuity_time = 0;
+
 	rte_intr_callback_register(intr_handle,
 				   ixgbevf_dev_interrupt_handler, eth_dev);
 	rte_intr_enable(intr_handle);
@@ -3121,12 +3174,17 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev)
 {
 	struct ixgbe_hw_stats *stats =
 			IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
+	struct ixgbe_adapter *adapter =
+			(struct ixgbe_adapter *)dev->data->dev_private;
 
 	/* HW registers are cleared on read */
 	ixgbe_dev_stats_get(dev, NULL);
 
 	/* Reset software totals */
 	memset(stats, 0, sizeof(*stats));
+
+	adapter->if_counter_discontinuity_time =
+			rte_rdtsc() - adapter->sys_up_time_start;
 }
 
 /* This function calculates the number of xstats based on the current config */
@@ -3134,7 +3192,8 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev)
 ixgbe_xstats_calc_num(void) {
 	return IXGBE_NB_HW_STATS + IXGBE_NB_MACSEC_STATS +
 		(IXGBE_NB_RXQ_PRIO_STATS * IXGBE_NB_RXQ_PRIO_VALUES) +
-		(IXGBE_NB_TXQ_PRIO_STATS * IXGBE_NB_TXQ_PRIO_VALUES);
+		(IXGBE_NB_TXQ_PRIO_STATS * IXGBE_NB_TXQ_PRIO_VALUES) +
+		IXGBE_NB_IF_MIB_XSTATS + IXGBE_NB_ETHER_LIKE_MIB_XSTATS;
 }
 
 static int ixgbe_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
@@ -3189,10 +3248,32 @@ static int ixgbe_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 				count++;
 			}
 		}
+
+		/* Get stats from IF-MIB objects */
+		for (i = 0; i < IXGBE_NB_IF_MIB_XSTATS; i++) {
+			snprintf(xstats_names[count].name,
+				sizeof(xstats_names[count].name),
+				 "%s", ixgbe_if_mib_strings[i].name);
+			count++;
+		}
+
+		/* Get stats from Ethernet-like-MIB objects */
+		for (i = 0; i < IXGBE_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+			snprintf(xstats_names[count].name,
+				sizeof(xstats_names[count].name),
+				 "%s", ixgbe_ether_like_mib_strings[i].name);
+			count++;
+		}
 	}
 	return cnt_stats;
 }
 
+static unsigned int
+ixgbevf_xstats_calc_num(void) {
+	return IXGBEVF_NB_XSTATS + IXGBE_NB_IF_MIB_XSTATS +
+		IXGBE_NB_ETHER_LIKE_MIB_XSTATS;
+}
+
 static int ixgbe_dev_xstats_get_names_by_id(
 	__rte_unused struct rte_eth_dev *dev,
 	struct rte_eth_xstat_name *xstats_names,
@@ -3272,19 +3353,117 @@ static int ixgbe_dev_xstats_get_names_by_id(
 }
 
 static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
-	struct rte_eth_xstat_name *xstats_names, unsigned limit)
+	struct rte_eth_xstat_name *xstats_names,
+	__rte_unused unsigned int limit)
 {
-	unsigned i;
+	unsigned int i, count = 0;
 
-	if (limit < IXGBEVF_NB_XSTATS && xstats_names != NULL)
-		return -ENOMEM;
+	if (xstats_names == NULL)
+		return ixgbevf_xstats_calc_num();
+
+	for (i = 0; i < IXGBEVF_NB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			"%s", rte_ixgbevf_stats_strings[i].name);
+		count++;
+	}
+
+	/* Get stats from IF-MIB objects */
+	for (i = 0; i < IXGBE_NB_IF_MIB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			 "%s", ixgbe_if_mib_strings[i].name);
+		count++;
+	}
+
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < IXGBE_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			 "%s", ixgbe_ether_like_mib_strings[i].name);
+		count++;
+	}
+
+	return count;
+}
+
+static void
+ixgbe_read_if_mib(struct rte_eth_dev *dev, struct ixgbe_if_mib_stats *stats)
+{
+	struct rte_eth_dev_data *data = dev->data;
+	struct rte_device *device = dev->device;
+	struct ixgbe_adapter *adapter =
+			(struct ixgbe_adapter *)dev->data->dev_private;
+
+	stats->if_number = rte_eth_dev_count();
+	stats->if_index = data->port_id + 1;
+	stats->if_type = IXGBE_MIB_IF_TYPE_ETHERNETCSMACD;
+	stats->if_mtu = data->mtu;
+	stats->if_speed = (data->dev_link.link_speed < (UINT32_MAX / 1000000)) ?
+			(data->dev_link.link_speed * 1000000) : UINT32_MAX;
+	stats->if_phys_address = 0;
+	ether_addr_copy(data->mac_addrs,
+			(struct ether_addr *)&stats->if_phys_address);
+	stats->if_oper_status = data->dev_link.link_status ?
+			ixgbe_mib_truth_true : ixgbe_mib_truth_false;
+	stats->if_last_change = adapter->if_last_change /
+			(rte_get_tsc_hz() * 100);
+	stats->if_high_speed = data->dev_link.link_speed;
+	if (device->devargs)
+		stats->if_connector_present =
+				(device->devargs->type == RTE_DEVTYPE_VIRTUAL) ?
+						ixgbe_mib_truth_false :
+						ixgbe_mib_truth_true;
+	else
+		stats->if_connector_present = 0;
+	stats->if_counter_discontinuity_time =
+			adapter->if_counter_discontinuity_time /
+			(rte_get_tsc_hz() * 100);
+}
+
+static void
+ixgbe_read_ether_like_mib(struct rte_eth_dev *dev,
+		struct ixgbe_ether_like_mib_stats *stats)
+{
+	struct rte_eth_dev_data *data = dev->data;
+	struct ixgbe_hw *hw =
+			IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	switch (hw->fc.current_mode) {
+	case ixgbe_fc_none:
+		stats->dot3_pause_oper_mode = ixgbe_dot3_pause_disabled;
+		break;
+	case ixgbe_fc_rx_pause:
+		stats->dot3_pause_oper_mode = ixgbe_dot3_pause_enabledrcv;
+		break;
+	case ixgbe_fc_tx_pause:
+		stats->dot3_pause_oper_mode = ixgbe_dot3_pause_enabledxmit;
+		break;
+	case ixgbe_fc_full:
+		stats->dot3_pause_oper_mode =
+				ixgbe_dot3_pause_enabledxmitandrcv;
+		break;
+	default:
+		stats->dot3_pause_oper_mode = 0;
+		break;
+	}
 
-	if (xstats_names != NULL)
-		for (i = 0; i < IXGBEVF_NB_XSTATS; i++)
-			snprintf(xstats_names[i].name,
-				sizeof(xstats_names[i].name),
-				"%s", rte_ixgbevf_stats_strings[i].name);
-	return IXGBEVF_NB_XSTATS;
+	switch (data->dev_link.link_duplex) {
+	case ETH_LINK_FULL_DUPLEX:
+		stats->dot3_stats_duplex_status = ixgbe_dot3_duplex_fullduplex;
+		break;
+	case ETH_LINK_HALF_DUPLEX:
+		stats->dot3_stats_duplex_status = ixgbe_dot3_duplex_halfduplex;
+		break;
+	default:
+		stats->dot3_stats_duplex_status = ixgbe_dot3_duplex_unknown;
+		break;
+	}
+
+	stats->dot3_stats_rate_control_ability = ixgbe_mib_truth_false;
+	stats->dot3_stats_rate_control_status = ixgbe_dot3_rate_control_off;
+	stats->dot3_control_functions_supported = IXGBE_DOT3_CF_PAUSE |
+			IXGBE_DOT3_CF_PFC;
 }
 
 static int
@@ -3298,6 +3477,8 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	struct ixgbe_macsec_stats *macsec_stats =
 			IXGBE_DEV_PRIVATE_TO_MACSEC_STATS(
 				dev->data->dev_private);
+	struct ixgbe_if_mib_stats if_mib_stats;
+	struct ixgbe_ether_like_mib_stats ether_like_mib_stats;
 	uint64_t total_missed_rx, total_qbrc, total_qprc, total_qprdc;
 	unsigned i, stat, count = 0;
 
@@ -3314,6 +3495,9 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	ixgbe_read_stats_registers(hw, hw_stats, macsec_stats, &total_missed_rx,
 			&total_qbrc, &total_qprc, &total_qprdc);
 
+	ixgbe_read_if_mib(dev, &if_mib_stats);
+	ixgbe_read_ether_like_mib(dev, &ether_like_mib_stats);
+
 	/* If this is a reset xstats is NULL, and we have cleared the
 	 * registers by reading them.
 	 */
@@ -3358,6 +3542,23 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 			count++;
 		}
 	}
+
+/* Get stats from IF-MIB objects */
+	for (i = 0; i < IXGBE_NB_IF_MIB_XSTATS; i++) {
+		xstats[count].value = *(uint64_t *)(((char *)&if_mib_stats) +
+			ixgbe_if_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
+
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < IXGBE_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		xstats[count].value =
+			*(uint64_t *)(((char *)&ether_like_mib_stats) +
+			ixgbe_ether_like_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
 	return count;
 }
 
@@ -3460,6 +3661,8 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	struct ixgbe_macsec_stats *macsec_stats =
 			IXGBE_DEV_PRIVATE_TO_MACSEC_STATS(
 				dev->data->dev_private);
+	struct ixgbe_adapter *adapter =
+			(struct ixgbe_adapter *)dev->data->dev_private;
 
 	unsigned count = ixgbe_xstats_calc_num();
 
@@ -3469,6 +3672,9 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	/* Reset software totals */
 	memset(stats, 0, sizeof(*stats));
 	memset(macsec_stats, 0, sizeof(*macsec_stats));
+
+	adapter->if_counter_discontinuity_time =
+			rte_rdtsc() - adapter->sys_up_time_start;
 }
 
 static void
@@ -3505,24 +3711,50 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 {
 	struct ixgbevf_hw_stats *hw_stats = (struct ixgbevf_hw_stats *)
 			IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
-	unsigned i;
+	struct ixgbe_if_mib_stats if_mib_stats;
+	struct ixgbe_ether_like_mib_stats ether_like_mib_stats;
+	unsigned int i, count;
 
-	if (n < IXGBEVF_NB_XSTATS)
-		return IXGBEVF_NB_XSTATS;
+	count = ixgbevf_xstats_calc_num();
+	if (n < count)
+		return count;
 
 	ixgbevf_update_stats(dev);
 
+	ixgbe_read_if_mib(dev, &if_mib_stats);
+	ixgbe_read_ether_like_mib(dev, &ether_like_mib_stats);
+
 	if (!xstats)
 		return 0;
 
+	count = 0;
+
 	/* Extended stats */
 	for (i = 0; i < IXGBEVF_NB_XSTATS; i++) {
-		xstats[i].id = i;
-		xstats[i].value = *(uint64_t *)(((char *)hw_stats) +
+		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 			rte_ixgbevf_stats_strings[i].offset);
+		xstats[count].id = count;
+		count++;
 	}
 
-	return IXGBEVF_NB_XSTATS;
+	/* Get stats from IF-MIB objects */
+	for (i = 0; i < IXGBE_NB_IF_MIB_XSTATS; i++) {
+		xstats[count].value = *(uint64_t *)(((char *)&if_mib_stats) +
+			ixgbe_if_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
+
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < IXGBE_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		xstats[count].value =
+			*(uint64_t *)(((char *)&ether_like_mib_stats) +
+			ixgbe_ether_like_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
+
+	return count;
 }
 
 static void
@@ -3547,6 +3779,8 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 {
 	struct ixgbevf_hw_stats *hw_stats = (struct ixgbevf_hw_stats *)
 			IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
+	struct ixgbe_adapter *adapter =
+			(struct ixgbe_adapter *)dev->data->dev_private;
 
 	/* Sync HW register to the last stats */
 	ixgbevf_dev_stats_get(dev, NULL);
@@ -3556,6 +3790,9 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	hw_stats->vfgorc = 0;
 	hw_stats->vfgptc = 0;
 	hw_stats->vfgotc = 0;
+
+	adapter->if_counter_discontinuity_time =
+			rte_rdtsc() - adapter->sys_up_time_start;
 }
 
 static int
@@ -3781,6 +4018,8 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 ixgbe_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_adapter *adapter =
+			(struct ixgbe_adapter *)dev->data->dev_private;
 	struct rte_eth_link link, old;
 	ixgbe_link_speed link_speed = IXGBE_LINK_SPEED_UNKNOWN;
 	struct ixgbe_interrupt *intr =
@@ -3856,6 +4095,8 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	if (link.link_status == old.link_status)
 		return -1;
 
+	adapter->if_last_change = rte_rdtsc() - adapter->sys_up_time_start;
+
 	return 0;
 }
 
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index b576a6f..00828b0 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -429,6 +429,62 @@ struct ixgbe_macsec_stats {
 	uint64_t in_pkts_notusingsa;
 };
 
+#define IXGBE_MIB_IF_TYPE_ETHERNETCSMACD	6
+
+enum ixgbe_mib_truth_value {
+	ixgbe_mib_truth_true = 1,
+	ixgbe_mib_truth_false
+};
+
+/* IF-MIB statistics */
+struct ixgbe_if_mib_stats {
+	uint64_t if_number;			/* ifNumber */
+	uint64_t if_index;			/* ifIndex */
+	uint64_t if_type;			/* ifType */
+	uint64_t if_mtu;			/* ifMtu */
+	uint64_t if_speed;			/* ifSpeed */
+	uint64_t if_phys_address;		/* ifPhysAddress */
+	uint64_t if_oper_status;		/* ifOperStatus */
+	uint64_t if_last_change;		/* ifLastChange */
+	uint64_t if_high_speed;			/* ifHighSpeed */
+	uint64_t if_connector_present;		/* ifConnectorPresent */
+	uint64_t if_counter_discontinuity_time;	/* ifCounterDiscontinuityTime */
+};
+
+enum ixgbe_dot3_pause_oper_mode {
+	ixgbe_dot3_pause_disabled = 1,
+	ixgbe_dot3_pause_enabledxmit,
+	ixgbe_dot3_pause_enabledrcv,
+	ixgbe_dot3_pause_enabledxmitandrcv
+};
+
+enum ixgbe_dot3_stats_duplex_status {
+	ixgbe_dot3_duplex_unknown = 1,
+	ixgbe_dot3_duplex_halfduplex,
+	ixgbe_dot3_duplex_fullduplex
+};
+
+enum ixgbe_dot3_stats_rate_control_status {
+	ixgbe_dot3_rate_control_off = 1,
+	ixgbe_dot3_rate_control_on,
+	ixgbe_dot3_rate_control_unknown
+};
+
+#define IXGBE_DOT3_CF_PAUSE	(1 << 0) /* PAUSE command implemented */
+#define IXGBE_DOT3_CF_MPCP	(1 << 1) /* MPCP implemented */
+#define IXGBE_DOT3_CF_PFC	(1 << 2) /* PFC implemented */
+
+/* Ethernet-like-MIB statistics */
+struct ixgbe_ether_like_mib_stats {
+	uint64_t dot3_pause_oper_mode;		/* dot3PauseOperMode */
+	uint64_t dot3_stats_duplex_status;	/* dot3StatsDuplexStatus */
+	uint64_t dot3_stats_rate_control_ability;
+	/* dot3StatsRateControlAbility */
+	uint64_t dot3_stats_rate_control_status;/* dot3StatsRateControlStatus */
+	uint64_t dot3_control_functions_supported;
+	/* dot3ControlFunctionsSupported */
+};
+
 /* The configuration of bandwidth */
 struct ixgbe_bw_conf {
 	uint8_t tc_num; /* Number of TCs. */
@@ -462,6 +518,9 @@ struct ixgbe_adapter {
 	struct rte_timecounter      systime_tc;
 	struct rte_timecounter      rx_tstamp_tc;
 	struct rte_timecounter      tx_tstamp_tc;
+	uint64_t                    sys_up_time_start;
+	uint64_t                    if_last_change;
+	uint64_t                    if_counter_discontinuity_time;
 };
 
 #define IXGBE_DEV_TO_PCI(eth_dev) \
-- 
1.7.9.5

Re: [dpdk-dev] [PATCH 0/3] drivers/net: add support for IF-MIB and EtherLike-MIB

[Stephen Hemminger]

On Mon, 22 May 2017 16:31:59 +0200
Michal Jastrzebski <michalx.k.jastrzebski@intel.com> wrote:

> Extend Intel NICs (i40e, ixgbe, e1000) xstats for IF-MIB and
> EtherLike-MIB.
> 
> If-MIB xstats:
> ifNumber
> ifIndex
> ifType
> ifMtu
> ifSpeed
> ifPhysAddress
> ifOperStatus
> ifLastChange
> ifHighSpeed
> ifConnectorPresent
> ifCounterDiscontinuityTime
> 
> EtherLike-MIB xstats:
> dot3PauseOperMode
> dot3StatsDuplexStatus
> dot3StatsRateControlAbility
> dot3StatsRateControlStatus
> dot3ControlFunctionsSupported
> 
> Michal Jastrzebski (3):
>   drivers/net: add support for IF-MIB and EtherLike-MIB for e1000
>   drivers/net: add support for IF-MIB and EtherLike-MIB for i40e
>   drivers/net: add support for IF-MIB and EtherLike-MIB for ixgbe
> 
>  drivers/net/e1000/e1000_ethdev.h |   59 ++++++++
>  drivers/net/e1000/igb_ethdev.c   |  296 ++++++++++++++++++++++++++++++++++----
>  drivers/net/i40e/i40e_ethdev.c   |  171 +++++++++++++++++++++-
>  drivers/net/i40e/i40e_ethdev.h   |   60 ++++++++
>  drivers/net/ixgbe/ixgbe_ethdev.c |  275 ++++++++++++++++++++++++++++++++---
>  drivers/net/ixgbe/ixgbe_ethdev.h |   59 ++++++++
>  6 files changed, 878 insertions(+), 42 deletions(-)
> 

Having all the SNMP MIB information is a worth goal. Thank you for starting
the effort.

But you need to rethink how you are implementing this.
Doing it in a driver specific manner is not helpful to application writers.
All API's for features like this should be at ethdev level, and be supported
by generic code.

If you are going to add something like this it has to work for all devices
virtual, physical, layered and not just Intel hardware. It is a much
bigger task.

Re: [dpdk-dev] [PATCH 1/3] drivers/net: add support for IF-MIB and EtherLike-MIB for e1000

[Pattan, Reshma]

Hi,


-----Original Message-----
From: Jastrzebski, MichalX K 
Sent: Monday, May 22, 2017 3:32 PM
To: dev@dpdk.org
Cc: Pattan, Reshma <reshma.pattan@intel.com>; Jain, Deepak K <deepak.k.jain@intel.com>; Van Haaren, Harry <harry.van.haaren@intel.com>; Jastrzebski, MichalX K <michalx.k.jastrzebski@intel.com>; Azarewicz, PiotrX T <piotrx.t.azarewicz@intel.com>
Subject: [PATCH 1/3] drivers/net: add support for IF-MIB and EtherLike-MIB for e1000

Can you add the description to the commit message, describing what has been done.

General doubt I have , I see all IF-MIB and Ether-Like-MIB attributes are added as Xstats.
But most of  IF-MIB attributes are not stats, instead they are information about the interface/port.
So I guess such attributes should be displayed using separate method?   Everything being displayed as xstats might not be correct.

diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 8352d0a..d242990 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -254,6 +254,62 @@ struct e1000_filter_info {
 	struct e1000_2tuple_filter_list twotuple_list;  };
 
+#define E1000_MIB_IF_TYPE_ETHERNETCSMACD	6
+
+enum e1000_mib_truth_value {
+	e1000_mib_truth_true = 1,
+	e1000_mib_truth_false
+};

1)All enums should be in capital as per coding guidelines. Similarly correct author enums too.
We no need to have enum identifier, just enumerator list should be suffice like below. So you can check all enumerations added newly and correct them?
enum
{  
enumerator-list  
}  
  
+
+/* IF-MIB statistics */
+struct e1000_if_mib_stats {
+	uint64_t if_number;			/* ifNumber */
+	uint64_t if_index;			/* ifIndex */
+	uint64_t if_type;			/* ifType */
+	uint64_t if_mtu;			/* ifMtu */
+	uint64_t if_speed;			/* ifSpeed */
+	uint64_t if_phys_address;		/* ifPhysAddress */
+	uint64_t if_oper_status;		/* ifOperStatus */
+	uint64_t if_last_change;		/* ifLastChange */
+	uint64_t if_high_speed;			/* ifHighSpeed */
+	uint64_t if_connector_present;		/* ifConnectorPresent */
+	uint64_t if_counter_discontinuity_time;	/* ifCounterDiscontinuityTime */
+};
+

The comments should be inside /**<  Here the comment goes. */
Refer other files to see how it is done.

+#define E1000_DOT3_CF_PAUSE	(1 << 0) /* PAUSE command implemented */
+#define E1000_DOT3_CF_MPCP	(1 << 1) /* MPCP implemented */
+#define E1000_DOT3_CF_PFC	(1 << 2) /* PFC implemented */

The comments should be inside /**<  Here the comment goes. */

+	uint64_t dot3_stats_rate_control_ability;
+	/* dot3StatsRateControlAbility */
+	uint64_t dot3_stats_rate_control_status;/* dot3StatsRateControlStatus */
+	uint64_t dot3_control_functions_supported;
+	/* dot3ControlFunctionsSupported */
+};

The comments line should go on top of the variables.

+
 /*
  * Structure to store private data for each driver instance (for each port).
  */
@@ -268,6 +324,9 @@ struct e1000_adapter {
 	struct rte_timecounter  systime_tc;
 	struct rte_timecounter  rx_tstamp_tc;
 	struct rte_timecounter  tx_tstamp_tc;
+	uint64_t                sys_up_time_start;
+	uint64_t                if_last_change;
+	uint64_t                if_counter_discontinuity_time;
 };
 
 #define E1000_DEV_PRIVATE(adapter) \
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index e1702d8..705b591 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c

Can you rename igb_if_mib_strings to rte_ igb_if_mib_strings and igb_if_mib_strings to rte_ igb_if_mib_strings? Similar to other stats/xstats.

eth_igb_stats_reset(), inside this function  we might not need below piece of code, as below piece of code you have already added to 
eth_igb_xstats_reset().
+
+	adapter->if_counter_discontinuity_time =
+			rte_rdtsc() - adapter->sys_up_time_start;
 }

Thanks,
Reshma

Re: [dpdk-dev] [PATCH 1/3] drivers/net: add support for IF-MIB and EtherLike-MIB for e1000

[Pattan, Reshma]

Hi,

Can you add the description to the commit message, describing what has been done.

General doubt I have , I see all IF-MIB and Ether-Like-MIB attributes are added as Xstats.
But most of  IF-MIB attributes are not stats, instead they are information about the interface/port.
So I guess such attributes should be displayed using separate method?   Everything being displayed as xstats might not be correct.

> ---
> 
> +#define E1000_MIB_IF_TYPE_ETHERNETCSMACD	6
> +
> +enum e1000_mib_truth_value {
> +	e1000_mib_truth_true = 1,
> +	e1000_mib_truth_false
> +};
> +

All enums should be in capital as per coding guidelines. Similarly correct author enums too.
We no need to have enum identifier, just enumerator list should be suffice like below. 
So you can check all enumerations added newly and correct them?
Ex:
enum
{  
enumerator-list  
}  

> +/* IF-MIB statistics */
> +struct e1000_if_mib_stats {
> +	uint64_t if_number;			/* ifNumber */
> +	uint64_t if_index;			/* ifIndex */
> +	uint64_t if_type;			/* ifType */
> +	uint64_t if_mtu;			/* ifMtu */
> +	uint64_t if_speed;			/* ifSpeed */
> +	uint64_t if_phys_address;		/* ifPhysAddress */
> +	uint64_t if_oper_status;		/* ifOperStatus */
> +	uint64_t if_last_change;		/* ifLastChange */
> +	uint64_t if_high_speed;			/* ifHighSpeed */
> +	uint64_t if_connector_present;		/* ifConnectorPresent */
> +	uint64_t if_counter_discontinuity_time;	/*
> ifCounterDiscontinuityTime */
> +};
> +

The comments should be inside /**<  Here the comment goes. */
Refer other files to see how it is done.

> +#define E1000_DOT3_CF_PAUSE	(1 << 0) /* PAUSE command
> implemented */
> +#define E1000_DOT3_CF_MPCP	(1 << 1) /* MPCP implemented */
> +#define E1000_DOT3_CF_PFC	(1 << 2) /* PFC implemented */
> +

The comments should be inside /**<  Here the comment goes. */

> +	uint64_t dot3_stats_rate_control_ability;
> +	/* dot3StatsRateControlAbility */
> +	uint64_t dot3_stats_rate_control_status;/*
> dot3StatsRateControlStatus */
> +	uint64_t dot3_control_functions_supported;
> +	/* dot3ControlFunctionsSupported */
> +};
> +

The comments line should go on top of the variables.

>  #define E1000_DEV_PRIVATE(adapter) \
> diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
> index e1702d8..705b591 100644
> --- a/drivers/net/e1000/igb_ethdev.c

Can you rename igb_if_mib_strings to rte_ igb_if_mib_strings and igb_if_mib_strings to rte_ igb_if_mib_strings? Similar to other stats/xstats.

eth_igb_stats_reset(), inside this function  we might not need below piece of code, as below piece of code you have already added to 
eth_igb_xstats_reset().

> stats->if_speed = (data->dev_link.link_speed < (UINT32_MAX / 1000000)) ?
>			(data->dev_link.link_speed * 1000000) : UINT32_MAX;

Can you clarify this link speed calculation logic

>	stats->if_last_change = adapter->if_last_change /
>			(rte_get_tsc_hz() * 100);

Can you clarify this logic?

Thanks,
Reshma

Re: [dpdk-dev] [PATCH 1/3] drivers/net: add support for IF-MIB and EtherLike-MIB for e1000

[Lu, Wenzhuo]

Hi Michal,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Pattan, Reshma
> Sent: Tuesday, May 23, 2017 12:35 AM
> To: Jastrzebski, MichalX K; dev@dpdk.org
> Cc: Jain, Deepak K; Van Haaren, Harry; Azarewicz, PiotrX T
> Subject: Re: [dpdk-dev] [PATCH 1/3] drivers/net: add support for IF-MIB and
> EtherLike-MIB for e1000
> 
> Hi,
> 
> Can you add the description to the commit message, describing what has
> been done.
> 
> General doubt I have , I see all IF-MIB and Ether-Like-MIB attributes are
> added as Xstats.
> But most of  IF-MIB attributes are not stats, instead they are information
> about the interface/port.
> So I guess such attributes should be displayed using separate method?
> Everything being displayed as xstats might not be correct.
The same question as Reshma. Seems most of the added parameters (if not all) are not stats.

Re: [dpdk-dev] [PATCH 2/3] drivers/net: add support for IF-MIB and EtherLike-MIB for i40e

[Xing, Beilei]

Hi Michal,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michal Jastrzebski
> Sent: Monday, May 22, 2017 10:32 PM
> To: dev@dpdk.org
> Cc: Pattan, Reshma <reshma.pattan@intel.com>; Jain, Deepak K
> <deepak.k.jain@intel.com>; Van Haaren, Harry
> <harry.van.haaren@intel.com>; Jastrzebski, MichalX K
> <michalx.k.jastrzebski@intel.com>; Azarewicz, PiotrX T
> <piotrx.t.azarewicz@intel.com>
> Subject: [dpdk-dev] [PATCH 2/3] drivers/net: add support for IF-MIB and
> EtherLike-MIB for i40e
> 
> If-MIB xstats:
> ifNumber
> ifIndex
> ifType
> ifMtu
> ifSpeed
> ifPhysAddress
> ifOperStatus
> ifLastChange
> ifHighSpeed
> ifConnectorPresent
> ifCounterDiscontinuityTime
> 
> EtherLike-MIB xstats:
> dot3PauseOperMode
> dot3StatsDuplexStatus
> dot3StatsRateControlAbility
> dot3StatsRateControlStatus
> dot3ControlFunctionsSupported
> 
> Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
> Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
> ---

Could you help to detail the purpose of the patch? seems you want to get some interface and link info instead of statistics.

Re: [dpdk-dev] [PATCH 1/3] drivers/net: add support for IF-MIB and EtherLike-MIB for e1000

[Radu Nicolau]

Hi,


On 5/23/2017 6:53 AM, Lu, Wenzhuo wrote:
> Hi Michal,
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Pattan, Reshma
>> Sent: Tuesday, May 23, 2017 12:35 AM
>> To: Jastrzebski, MichalX K; dev@dpdk.org
>> Cc: Jain, Deepak K; Van Haaren, Harry; Azarewicz, PiotrX T
>> Subject: Re: [dpdk-dev] [PATCH 1/3] drivers/net: add support for IF-MIB and
>> EtherLike-MIB for e1000
>>
>> Hi,
>>
>> Can you add the description to the commit message, describing what has
>> been done.
>>
>> General doubt I have , I see all IF-MIB and Ether-Like-MIB attributes are
>> added as Xstats.
>> But most of  IF-MIB attributes are not stats, instead they are information
>> about the interface/port.
>> So I guess such attributes should be displayed using separate method?
>> Everything being displayed as xstats might not be correct.
> The same question as Reshma. Seems most of the added parameters (if not all) are not stats.

Yes, they are indeed mostly information rather than actual statistics, 
but I would say it's a better choice to have them in the xstats rather 
than add a new API.

[dpdk-dev] [PATCH v2] drivers/net: add support for IF-MIB and EtherLike-MIB for ixgbe

[Radu Nicolau]

From: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>

If-MIB xstats:
ifNumber
ifIndex
ifType
ifMtu
ifSpeed
ifPhysAddress
ifOperStatus
ifLastChange
ifHighSpeed
ifConnectorPresent
ifCounterDiscontinuityTime

EtherLike-MIB xstats:
dot3PauseOperMode
dot3StatsDuplexStatus
dot3StatsRateControlAbility
dot3StatsRateControlStatus
dot3ControlFunctionsSupported

-updated in v2: coding style

Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 275 ++++++++++++++++++++++++++++++++++++---
 drivers/net/ixgbe/ixgbe_ethdev.h |  59 +++++++++
 2 files changed, 317 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index ebc5467..16f79c2 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -809,6 +809,45 @@ static const struct rte_ixgbe_xstats_name_off rte_ixgbevf_stats_strings[] = {
 #define IXGBEVF_NB_XSTATS (sizeof(rte_ixgbevf_stats_strings) /	\
 		sizeof(rte_ixgbevf_stats_strings[0]))
 
+static const struct rte_ixgbe_xstats_name_off ixgbe_if_mib_strings[] = {
+	{"ifNumber", offsetof(struct ixgbe_if_mib_stats, if_number)},
+	{"ifIndex", offsetof(struct ixgbe_if_mib_stats, if_index)},
+	{"ifType", offsetof(struct ixgbe_if_mib_stats, if_type)},
+	{"ifMtu", offsetof(struct ixgbe_if_mib_stats, if_mtu)},
+	{"ifSpeed", offsetof(struct ixgbe_if_mib_stats, if_speed)},
+	{"ifPhysAddress", offsetof(struct ixgbe_if_mib_stats, if_phys_address)},
+	{"ifOperStatus", offsetof(struct ixgbe_if_mib_stats, if_oper_status)},
+	{"ifLastChange", offsetof(struct ixgbe_if_mib_stats, if_last_change)},
+	{"ifHighSpeed", offsetof(struct ixgbe_if_mib_stats, if_high_speed)},
+	{"ifConnectorPresent", offsetof(struct ixgbe_if_mib_stats,
+			if_connector_present)},
+	{"ifCounterDiscontinuityTime", offsetof(struct ixgbe_if_mib_stats,
+			if_counter_discontinuity_time)},
+};
+
+#define IXGBE_NB_IF_MIB_XSTATS (sizeof(ixgbe_if_mib_strings) / \
+		sizeof(ixgbe_if_mib_strings[0]))
+
+static const struct rte_ixgbe_xstats_name_off ixgbe_ether_like_mib_strings[] = {
+	{"dot3PauseOperMode", offsetof(struct ixgbe_ether_like_mib_stats,
+			dot3_pause_oper_mode)},
+	{"dot3StatsDuplexStatus", offsetof(struct ixgbe_ether_like_mib_stats,
+			dot3_stats_duplex_status)},
+	{"dot3StatsRateControlAbility", offsetof(
+			struct ixgbe_ether_like_mib_stats,
+			dot3_stats_rate_control_ability)},
+	{"dot3StatsRateControlStatus", offsetof(
+			struct ixgbe_ether_like_mib_stats,
+			dot3_stats_rate_control_status)},
+	{"dot3ControlFunctionsSupported", offsetof(
+			struct ixgbe_ether_like_mib_stats,
+			dot3_control_functions_supported)},
+};
+
+#define IXGBE_NB_ETHER_LIKE_MIB_XSTATS \
+		(sizeof(ixgbe_ether_like_mib_strings) / \
+		sizeof(ixgbe_ether_like_mib_strings[0]))
+
 /**
  * Atomically reads the link status information from global
  * structure rte_eth_dev.
@@ -1134,6 +1173,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
 		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(eth_dev->data->dev_private);
 	struct ixgbe_bw_conf *bw_conf =
 		IXGBE_DEV_PRIVATE_TO_BW_CONF(eth_dev->data->dev_private);
+	struct ixgbe_adapter *adapter =
+			(struct ixgbe_adapter *)eth_dev->data->dev_private;
 	uint32_t ctrl_ext;
 	uint16_t csum;
 	int diag, i;
@@ -1316,6 +1357,11 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id);
 
+	/* indicate sysUpTime start */
+	adapter->sys_up_time_start = rte_rdtsc();
+	adapter->if_last_change = 0;
+	adapter->if_counter_discontinuity_time = 0;
+
 	rte_intr_callback_register(intr_handle,
 				   ixgbe_dev_interrupt_handler, eth_dev);
 
@@ -1596,6 +1642,8 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 	struct ixgbe_hwstrip *hwstrip =
 		IXGBE_DEV_PRIVATE_TO_HWSTRIP_BITMAP(eth_dev->data->dev_private);
 	struct ether_addr *perm_addr = (struct ether_addr *) hw->mac.perm_addr;
+	struct ixgbe_adapter *adapter =
+			(struct ixgbe_adapter *)eth_dev->data->dev_private;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -1719,6 +1767,11 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 		return -EIO;
 	}
 
+	/* indicate sysUpTime start */
+	adapter->sys_up_time_start = rte_rdtsc();
+	adapter->if_last_change = 0;
+	adapter->if_counter_discontinuity_time = 0;
+
 	rte_intr_callback_register(intr_handle,
 				   ixgbevf_dev_interrupt_handler, eth_dev);
 	rte_intr_enable(intr_handle);
@@ -3110,12 +3163,17 @@ ixgbe_dev_stats_reset(struct rte_eth_dev *dev)
 {
 	struct ixgbe_hw_stats *stats =
 			IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
+	struct ixgbe_adapter *adapter =
+			(struct ixgbe_adapter *)dev->data->dev_private;
 
 	/* HW registers are cleared on read */
 	ixgbe_dev_stats_get(dev, NULL);
 
 	/* Reset software totals */
 	memset(stats, 0, sizeof(*stats));
+
+	adapter->if_counter_discontinuity_time =
+			rte_rdtsc() - adapter->sys_up_time_start;
 }
 
 /* This function calculates the number of xstats based on the current config */
@@ -3123,7 +3181,8 @@ static unsigned
 ixgbe_xstats_calc_num(void) {
 	return IXGBE_NB_HW_STATS + IXGBE_NB_MACSEC_STATS +
 		(IXGBE_NB_RXQ_PRIO_STATS * IXGBE_NB_RXQ_PRIO_VALUES) +
-		(IXGBE_NB_TXQ_PRIO_STATS * IXGBE_NB_TXQ_PRIO_VALUES);
+		(IXGBE_NB_TXQ_PRIO_STATS * IXGBE_NB_TXQ_PRIO_VALUES) +
+		IXGBE_NB_IF_MIB_XSTATS + IXGBE_NB_ETHER_LIKE_MIB_XSTATS;
 }
 
 static int ixgbe_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
@@ -3178,10 +3237,32 @@ static int ixgbe_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 				count++;
 			}
 		}
+
+		/* Get stats from IF-MIB objects */
+		for (i = 0; i < IXGBE_NB_IF_MIB_XSTATS; i++) {
+			snprintf(xstats_names[count].name,
+				sizeof(xstats_names[count].name),
+				 "%s", ixgbe_if_mib_strings[i].name);
+			count++;
+		}
+
+		/* Get stats from Ethernet-like-MIB objects */
+		for (i = 0; i < IXGBE_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+			snprintf(xstats_names[count].name,
+				sizeof(xstats_names[count].name),
+				 "%s", ixgbe_ether_like_mib_strings[i].name);
+			count++;
+		}
 	}
 	return cnt_stats;
 }
 
+static unsigned int
+ixgbevf_xstats_calc_num(void) {
+	return IXGBEVF_NB_XSTATS + IXGBE_NB_IF_MIB_XSTATS +
+		IXGBE_NB_ETHER_LIKE_MIB_XSTATS;
+}
+
 static int ixgbe_dev_xstats_get_names_by_id(
 	struct rte_eth_dev *dev,
 	struct rte_eth_xstat_name *xstats_names,
@@ -3261,19 +3342,117 @@ static int ixgbe_dev_xstats_get_names_by_id(
 }
 
 static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
-	struct rte_eth_xstat_name *xstats_names, unsigned limit)
+	struct rte_eth_xstat_name *xstats_names,
+	__rte_unused unsigned int limit)
 {
-	unsigned i;
+	unsigned int i, count = 0;
 
-	if (limit < IXGBEVF_NB_XSTATS && xstats_names != NULL)
-		return -ENOMEM;
+	if (xstats_names == NULL)
+		return ixgbevf_xstats_calc_num();
+
+	for (i = 0; i < IXGBEVF_NB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			"%s", rte_ixgbevf_stats_strings[i].name);
+		count++;
+	}
+
+	/* Get stats from IF-MIB objects */
+	for (i = 0; i < IXGBE_NB_IF_MIB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			 "%s", ixgbe_if_mib_strings[i].name);
+		count++;
+	}
+
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < IXGBE_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			 "%s", ixgbe_ether_like_mib_strings[i].name);
+		count++;
+	}
+
+	return count;
+}
+
+static void
+ixgbe_read_if_mib(struct rte_eth_dev *dev, struct ixgbe_if_mib_stats *stats)
+{
+	struct rte_eth_dev_data *data = dev->data;
+	struct rte_device *device = dev->device;
+	struct ixgbe_adapter *adapter =
+			(struct ixgbe_adapter *)dev->data->dev_private;
+
+	stats->if_number = rte_eth_dev_count();
+	stats->if_index = data->port_id + 1;
+	stats->if_type = IXGBE_MIB_IF_TYPE_ETHERNETCSMACD;
+	stats->if_mtu = data->mtu;
+	stats->if_speed = (data->dev_link.link_speed < (UINT32_MAX / 1000000)) ?
+			(data->dev_link.link_speed * 1000000) : UINT32_MAX;
+	stats->if_phys_address = 0;
+	ether_addr_copy(data->mac_addrs,
+			(struct ether_addr *)&stats->if_phys_address);
+	stats->if_oper_status = data->dev_link.link_status ?
+			IXGBE_MIB_TRUTH_TRUE : IXGBE_MIB_TRUTH_FALSE;
+	stats->if_last_change = adapter->if_last_change /
+			(rte_get_tsc_hz() * 100);
+	stats->if_high_speed = data->dev_link.link_speed;
+	if (device->devargs)
+		stats->if_connector_present =
+				(device->devargs->type == RTE_DEVTYPE_VIRTUAL) ?
+						IXGBE_MIB_TRUTH_FALSE :
+						IXGBE_MIB_TRUTH_TRUE;
+	else
+		stats->if_connector_present = 0;
+	stats->if_counter_discontinuity_time =
+			adapter->if_counter_discontinuity_time /
+			(rte_get_tsc_hz() * 100);
+}
+
+static void
+ixgbe_read_ether_like_mib(struct rte_eth_dev *dev,
+		struct ixgbe_ether_like_mib_stats *stats)
+{
+	struct rte_eth_dev_data *data = dev->data;
+	struct ixgbe_hw *hw =
+			IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	switch (hw->fc.current_mode) {
+	case ixgbe_fc_none:
+		stats->dot3_pause_oper_mode = IXGBE_DOT3_PAUSE_DISABLED;
+		break;
+	case ixgbe_fc_rx_pause:
+		stats->dot3_pause_oper_mode = IXGBE_DOT3_PAUSE_ENABLEDRCV;
+		break;
+	case ixgbe_fc_tx_pause:
+		stats->dot3_pause_oper_mode = IXGBE_DOT3_PAUSE_ENABLEDXMIT;
+		break;
+	case ixgbe_fc_full:
+		stats->dot3_pause_oper_mode =
+				IXGBE_DOT3_PAUSE_ENABLEDXMITANDRCV;
+		break;
+	default:
+		stats->dot3_pause_oper_mode = 0;
+		break;
+	}
 
-	if (xstats_names != NULL)
-		for (i = 0; i < IXGBEVF_NB_XSTATS; i++)
-			snprintf(xstats_names[i].name,
-				sizeof(xstats_names[i].name),
-				"%s", rte_ixgbevf_stats_strings[i].name);
-	return IXGBEVF_NB_XSTATS;
+	switch (data->dev_link.link_duplex) {
+	case ETH_LINK_FULL_DUPLEX:
+		stats->dot3_stats_duplex_status = IXGBE_DOT3_DUPLEX_FULLDUPLEX;
+		break;
+	case ETH_LINK_HALF_DUPLEX:
+		stats->dot3_stats_duplex_status = IXGBE_DOT3_DUPLEX_HALFDUPLEX;
+		break;
+	default:
+		stats->dot3_stats_duplex_status = IXGBE_DOT3_DUPLEX_UNKNOWN;
+		break;
+	}
+
+	stats->dot3_stats_rate_control_ability = IXGBE_MIB_TRUTH_FALSE;
+	stats->dot3_stats_rate_control_status = IXGBE_DOT3_RATE_CONTROL_OFF;
+	stats->dot3_control_functions_supported = IXGBE_DOT3_CF_PAUSE |
+			IXGBE_DOT3_CF_PFC;
 }
 
 static int
@@ -3287,6 +3466,8 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	struct ixgbe_macsec_stats *macsec_stats =
 			IXGBE_DEV_PRIVATE_TO_MACSEC_STATS(
 				dev->data->dev_private);
+	struct ixgbe_if_mib_stats if_mib_stats;
+	struct ixgbe_ether_like_mib_stats ether_like_mib_stats;
 	uint64_t total_missed_rx, total_qbrc, total_qprc, total_qprdc;
 	unsigned i, stat, count = 0;
 
@@ -3303,6 +3484,9 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	ixgbe_read_stats_registers(hw, hw_stats, macsec_stats, &total_missed_rx,
 			&total_qbrc, &total_qprc, &total_qprdc);
 
+	ixgbe_read_if_mib(dev, &if_mib_stats);
+	ixgbe_read_ether_like_mib(dev, &ether_like_mib_stats);
+
 	/* If this is a reset xstats is NULL, and we have cleared the
 	 * registers by reading them.
 	 */
@@ -3347,6 +3531,23 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 			count++;
 		}
 	}
+
+/* Get stats from IF-MIB objects */
+	for (i = 0; i < IXGBE_NB_IF_MIB_XSTATS; i++) {
+		xstats[count].value = *(uint64_t *)(((char *)&if_mib_stats) +
+			ixgbe_if_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
+
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < IXGBE_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		xstats[count].value =
+			*(uint64_t *)(((char *)&ether_like_mib_stats) +
+			ixgbe_ether_like_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
 	return count;
 }
 
@@ -3449,6 +3650,8 @@ ixgbe_dev_xstats_reset(struct rte_eth_dev *dev)
 	struct ixgbe_macsec_stats *macsec_stats =
 			IXGBE_DEV_PRIVATE_TO_MACSEC_STATS(
 				dev->data->dev_private);
+	struct ixgbe_adapter *adapter =
+			(struct ixgbe_adapter *)dev->data->dev_private;
 
 	unsigned count = ixgbe_xstats_calc_num();
 
@@ -3458,6 +3661,9 @@ ixgbe_dev_xstats_reset(struct rte_eth_dev *dev)
 	/* Reset software totals */
 	memset(stats, 0, sizeof(*stats));
 	memset(macsec_stats, 0, sizeof(*macsec_stats));
+
+	adapter->if_counter_discontinuity_time =
+			rte_rdtsc() - adapter->sys_up_time_start;
 }
 
 static void
@@ -3494,24 +3700,50 @@ ixgbevf_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 {
 	struct ixgbevf_hw_stats *hw_stats = (struct ixgbevf_hw_stats *)
 			IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
-	unsigned i;
+	struct ixgbe_if_mib_stats if_mib_stats;
+	struct ixgbe_ether_like_mib_stats ether_like_mib_stats;
+	unsigned int i, count;
 
-	if (n < IXGBEVF_NB_XSTATS)
-		return IXGBEVF_NB_XSTATS;
+	count = ixgbevf_xstats_calc_num();
+	if (n < count)
+		return count;
 
 	ixgbevf_update_stats(dev);
 
+	ixgbe_read_if_mib(dev, &if_mib_stats);
+	ixgbe_read_ether_like_mib(dev, &ether_like_mib_stats);
+
 	if (!xstats)
 		return 0;
 
+	count = 0;
+
 	/* Extended stats */
 	for (i = 0; i < IXGBEVF_NB_XSTATS; i++) {
-		xstats[i].id = i;
-		xstats[i].value = *(uint64_t *)(((char *)hw_stats) +
+		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 			rte_ixgbevf_stats_strings[i].offset);
+		xstats[count].id = count;
+		count++;
 	}
 
-	return IXGBEVF_NB_XSTATS;
+	/* Get stats from IF-MIB objects */
+	for (i = 0; i < IXGBE_NB_IF_MIB_XSTATS; i++) {
+		xstats[count].value = *(uint64_t *)(((char *)&if_mib_stats) +
+			ixgbe_if_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
+
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < IXGBE_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		xstats[count].value =
+			*(uint64_t *)(((char *)&ether_like_mib_stats) +
+			ixgbe_ether_like_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
+
+	return count;
 }
 
 static void
@@ -3536,6 +3768,8 @@ ixgbevf_dev_stats_reset(struct rte_eth_dev *dev)
 {
 	struct ixgbevf_hw_stats *hw_stats = (struct ixgbevf_hw_stats *)
 			IXGBE_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
+	struct ixgbe_adapter *adapter =
+			(struct ixgbe_adapter *)dev->data->dev_private;
 
 	/* Sync HW register to the last stats */
 	ixgbevf_dev_stats_get(dev, NULL);
@@ -3545,6 +3779,9 @@ ixgbevf_dev_stats_reset(struct rte_eth_dev *dev)
 	hw_stats->vfgorc = 0;
 	hw_stats->vfgptc = 0;
 	hw_stats->vfgotc = 0;
+
+	adapter->if_counter_discontinuity_time =
+			rte_rdtsc() - adapter->sys_up_time_start;
 }
 
 static int
@@ -3770,6 +4007,8 @@ static int
 ixgbe_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_adapter *adapter =
+			(struct ixgbe_adapter *)dev->data->dev_private;
 	struct rte_eth_link link, old;
 	ixgbe_link_speed link_speed = IXGBE_LINK_SPEED_UNKNOWN;
 	struct ixgbe_interrupt *intr =
@@ -3845,6 +4084,8 @@ ixgbe_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 	if (link.link_status == old.link_status)
 		return -1;
 
+	adapter->if_last_change = rte_rdtsc() - adapter->sys_up_time_start;
+
 	return 0;
 }
 
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 10b9967..c26323c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -430,6 +430,62 @@ struct ixgbe_macsec_stats {
 	uint64_t in_pkts_notusingsa;
 };
 
+#define IXGBE_MIB_IF_TYPE_ETHERNETCSMACD	6
+
+enum {
+	IXGBE_MIB_TRUTH_TRUE = 1,
+	IXGBE_MIB_TRUTH_FALSE
+};
+
+/* IF-MIB statistics */
+struct ixgbe_if_mib_stats {
+	uint64_t if_number;			/* ifNumber */
+	uint64_t if_index;			/* ifIndex */
+	uint64_t if_type;			/* ifType */
+	uint64_t if_mtu;			/* ifMtu */
+	uint64_t if_speed;			/* ifSpeed */
+	uint64_t if_phys_address;		/* ifPhysAddress */
+	uint64_t if_oper_status;		/* ifOperStatus */
+	uint64_t if_last_change;		/* ifLastChange */
+	uint64_t if_high_speed;			/* ifHighSpeed */
+	uint64_t if_connector_present;		/* ifConnectorPresent */
+	uint64_t if_counter_discontinuity_time;	/* ifCounterDiscontinuityTime */
+};
+
+enum {
+	IXGBE_DOT3_PAUSE_DISABLED = 1,
+	IXGBE_DOT3_PAUSE_ENABLEDXMIT,
+	IXGBE_DOT3_PAUSE_ENABLEDRCV,
+	IXGBE_DOT3_PAUSE_ENABLEDXMITANDRCV
+};
+
+enum {
+	IXGBE_DOT3_DUPLEX_UNKNOWN = 1,
+	IXGBE_DOT3_DUPLEX_HALFDUPLEX,
+	IXGBE_DOT3_DUPLEX_FULLDUPLEX
+};
+
+enum {
+	IXGBE_DOT3_RATE_CONTROL_OFF = 1,
+	IXGBE_DOT3_RATE_CONTROL_ON,
+	IXGBE_DOT3_RATE_CONTROL_UNKNOWN
+};
+
+#define IXGBE_DOT3_CF_PAUSE	(1 << 0) /* PAUSE command implemented */
+#define IXGBE_DOT3_CF_MPCP	(1 << 1) /* MPCP implemented */
+#define IXGBE_DOT3_CF_PFC	(1 << 2) /* PFC implemented */
+
+/* Ethernet-like-MIB statistics */
+struct ixgbe_ether_like_mib_stats {
+	uint64_t dot3_pause_oper_mode;		/* dot3PauseOperMode */
+	uint64_t dot3_stats_duplex_status;	/* dot3StatsDuplexStatus */
+	uint64_t dot3_stats_rate_control_ability;
+	/* dot3StatsRateControlAbility */
+	uint64_t dot3_stats_rate_control_status;/* dot3StatsRateControlStatus */
+	uint64_t dot3_control_functions_supported;
+	/* dot3ControlFunctionsSupported */
+};
+
 /* The configuration of bandwidth */
 struct ixgbe_bw_conf {
 	uint8_t tc_num; /* Number of TCs. */
@@ -463,6 +519,9 @@ struct ixgbe_adapter {
 	struct rte_timecounter      systime_tc;
 	struct rte_timecounter      rx_tstamp_tc;
 	struct rte_timecounter      tx_tstamp_tc;
+	uint64_t                    sys_up_time_start;
+	uint64_t                    if_last_change;
+	uint64_t                    if_counter_discontinuity_time;
 };
 
 #define IXGBE_DEV_PRIVATE_TO_HW(adapter)\
-- 
2.7.5

[dpdk-dev] [PATCH v2] drivers/net: add support for IF-MIB and EtherLike-MIB for i40e

[Radu Nicolau]

From: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>

If-MIB xstats:
ifNumber
ifIndex
ifType
ifMtu
ifSpeed
ifPhysAddress
ifOperStatus
ifLastChange
ifHighSpeed
ifConnectorPresent
ifCounterDiscontinuityTime

EtherLike-MIB xstats:
dot3PauseOperMode
dot3StatsDuplexStatus
dot3StatsRateControlAbility
dot3StatsRateControlStatus
dot3ControlFunctionsSupported

-updated in v2: coding style

Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 171 ++++++++++++++++++++++++++++++++++++++++-
 drivers/net/i40e/i40e_ethdev.h |  60 +++++++++++++++
 2 files changed, 230 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 4ee1113..1926ce7 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -630,6 +630,45 @@ static const struct rte_i40e_xstats_name_off rte_i40e_txq_prio_strings[] = {
 #define I40E_NB_TXQ_PRIO_XSTATS (sizeof(rte_i40e_txq_prio_strings) / \
 		sizeof(rte_i40e_txq_prio_strings[0]))
 
+static const struct rte_i40e_xstats_name_off i40e_if_mib_strings[] = {
+	{"ifNumber", offsetof(struct i40e_if_mib_stats, if_number)},
+	{"ifIndex", offsetof(struct i40e_if_mib_stats, if_index)},
+	{"ifType", offsetof(struct i40e_if_mib_stats, if_type)},
+	{"ifMtu", offsetof(struct i40e_if_mib_stats, if_mtu)},
+	{"ifSpeed", offsetof(struct i40e_if_mib_stats, if_speed)},
+	{"ifPhysAddress", offsetof(struct i40e_if_mib_stats, if_phys_address)},
+	{"ifOperStatus", offsetof(struct i40e_if_mib_stats, if_oper_status)},
+	{"ifLastChange", offsetof(struct i40e_if_mib_stats, if_last_change)},
+	{"ifHighSpeed", offsetof(struct i40e_if_mib_stats, if_high_speed)},
+	{"ifConnectorPresent", offsetof(struct i40e_if_mib_stats,
+			if_connector_present)},
+	{"ifCounterDiscontinuityTime", offsetof(struct i40e_if_mib_stats,
+			if_counter_discontinuity_time)},
+};
+
+#define I40E_NB_IF_MIB_XSTATS (sizeof(i40e_if_mib_strings) / \
+		sizeof(i40e_if_mib_strings[0]))
+
+static const struct rte_i40e_xstats_name_off i40e_ether_like_mib_strings[] = {
+	{"dot3PauseOperMode", offsetof(struct i40e_ether_like_mib_stats,
+			dot3_pause_oper_mode)},
+	{"dot3StatsDuplexStatus", offsetof(struct i40e_ether_like_mib_stats,
+			dot3_stats_duplex_status)},
+	{"dot3StatsRateControlAbility", offsetof(
+			struct i40e_ether_like_mib_stats,
+			dot3_stats_rate_control_ability)},
+	{"dot3StatsRateControlStatus", offsetof(
+			struct i40e_ether_like_mib_stats,
+			dot3_stats_rate_control_status)},
+	{"dot3ControlFunctionsSupported", offsetof(
+			struct i40e_ether_like_mib_stats,
+			dot3_control_functions_supported)},
+};
+
+#define I40E_NB_ETHER_LIKE_MIB_XSTATS \
+		(sizeof(i40e_ether_like_mib_strings) / \
+		sizeof(i40e_ether_like_mib_strings[0]))
+
 static int eth_i40e_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	struct rte_pci_device *pci_dev)
 {
@@ -1273,6 +1312,11 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
 	/* initialize pf host driver to setup SRIOV resource if applicable */
 	i40e_pf_host_init(dev);
 
+	/* indicate sysUpTime start */
+	pf->adapter->sys_up_time_start = rte_rdtsc();
+	pf->adapter->if_last_change = 0;
+	pf->adapter->if_counter_discontinuity_time = 0;
+
 	/* register callback func to eal lib */
 	rte_intr_callback_register(intr_handle,
 				   i40e_dev_interrupt_handler, dev);
@@ -2235,6 +2279,8 @@ i40e_dev_link_update(struct rte_eth_dev *dev,
 #define CHECK_INTERVAL 100  /* 100ms */
 #define MAX_REPEAT_TIME 10  /* 1s (10 * 100ms) in total */
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct i40e_adapter *adapter =
+		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct i40e_link_status link_status;
 	struct rte_eth_link link, old;
 	int status;
@@ -2303,6 +2349,7 @@ i40e_dev_link_update(struct rte_eth_dev *dev,
 	if (link.link_status == old.link_status)
 		return -1;
 
+	adapter->if_last_change = rte_rdtsc() - adapter->sys_up_time_start;
 	i40e_notify_all_vfs_link_status(dev);
 
 	return 0;
@@ -2703,6 +2750,9 @@ i40e_dev_stats_reset(struct rte_eth_dev *dev)
 
 	/* read the stats, reading current register values into offset */
 	i40e_read_stats_registers(pf, hw);
+
+	pf->adapter->if_counter_discontinuity_time =
+			rte_rdtsc() - pf->adapter->sys_up_time_start;
 }
 
 static uint32_t
@@ -2710,7 +2760,8 @@ i40e_xstats_calc_num(void)
 {
 	return I40E_NB_ETH_XSTATS + I40E_NB_HW_PORT_XSTATS +
 		(I40E_NB_RXQ_PRIO_XSTATS * 8) +
-		(I40E_NB_TXQ_PRIO_XSTATS * 8);
+		(I40E_NB_TXQ_PRIO_XSTATS * 8) +
+		I40E_NB_IF_MIB_XSTATS + I40E_NB_ETHER_LIKE_MIB_XSTATS;
 }
 
 static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
@@ -2760,9 +2811,105 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 			count++;
 		}
 	}
+
+	/* Get stats from IF-MIB objects */
+	for (i = 0; i < I40E_NB_IF_MIB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			 "%s", i40e_if_mib_strings[i].name);
+		count++;
+	}
+
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < I40E_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			 "%s", i40e_ether_like_mib_strings[i].name);
+		count++;
+	}
+
 	return count;
 }
 
+static void
+i40e_read_if_mib(struct rte_eth_dev *dev, struct i40e_if_mib_stats *stats)
+{
+	struct rte_eth_dev_data *data = dev->data;
+	struct rte_device *device = dev->device;
+	struct i40e_adapter *adapter =
+			I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+
+	stats->if_number = rte_eth_dev_count();
+	stats->if_index = data->port_id + 1;
+	stats->if_type = I40E_MIB_IF_TYPE_ETHERNETCSMACD;
+	stats->if_mtu = data->mtu;
+	stats->if_speed = (data->dev_link.link_speed < (UINT32_MAX / 1000000)) ?
+			(data->dev_link.link_speed * 1000000) : UINT32_MAX;
+	stats->if_phys_address = 0;
+	ether_addr_copy(data->mac_addrs,
+			(struct ether_addr *)&stats->if_phys_address);
+	stats->if_oper_status = data->dev_link.link_status ?
+			I40E_MIB_TRUTH_TRUE : I40E_MIB_TRUTH_FALSE;
+	stats->if_last_change = adapter->if_last_change /
+			(rte_get_tsc_hz() * 100);
+	stats->if_high_speed = data->dev_link.link_speed;
+	if (device->devargs)
+		stats->if_connector_present =
+				(device->devargs->type == RTE_DEVTYPE_VIRTUAL) ?
+						I40E_MIB_TRUTH_FALSE :
+						I40E_MIB_TRUTH_TRUE;
+	else
+		stats->if_connector_present = 0;
+	stats->if_counter_discontinuity_time =
+			adapter->if_counter_discontinuity_time /
+			(rte_get_tsc_hz() * 100);
+}
+
+static void
+i40e_read_ether_like_mib(struct rte_eth_dev *dev,
+		struct i40e_ether_like_mib_stats *stats)
+{
+	struct rte_eth_dev_data *data = dev->data;
+	struct i40e_hw *hw =
+			I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	switch (hw->fc.current_mode) {
+	case I40E_FC_NONE:
+		stats->dot3_pause_oper_mode = I40E_DOT3_PAUSE_DISABLED;
+		break;
+	case I40E_FC_RX_PAUSE:
+		stats->dot3_pause_oper_mode = I40E_DOT3_PAUSE_ENABLEDRCV;
+		break;
+	case I40E_FC_TX_PAUSE:
+		stats->dot3_pause_oper_mode = I40E_DOT3_PAUSE_ENABLEDXMIT;
+		break;
+	case I40E_FC_FULL:
+		stats->dot3_pause_oper_mode =
+				I40E_DOT3_PAUSE_ENABLEDXMITANDRCV;
+		break;
+	default:
+		stats->dot3_pause_oper_mode = 0;
+		break;
+	}
+
+	switch (data->dev_link.link_duplex) {
+	case ETH_LINK_FULL_DUPLEX:
+		stats->dot3_stats_duplex_status = I40E_DOT3_DUPLEX_FULLDUPLEX;
+		break;
+	case ETH_LINK_HALF_DUPLEX:
+		stats->dot3_stats_duplex_status = I40E_DOT3_DUPLEX_HALFDUPLEX;
+		break;
+	default:
+		stats->dot3_stats_duplex_status = I40E_DOT3_DUPLEX_UNKNOWN;
+		break;
+	}
+
+	stats->dot3_stats_rate_control_ability = I40E_MIB_TRUTH_FALSE;
+	stats->dot3_stats_rate_control_status = I40E_DOT3_RATE_CONTROL_OFF;
+	stats->dot3_control_functions_supported = I40E_DOT3_CF_PAUSE |
+			I40E_DOT3_CF_PFC;
+}
+
 static int
 i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 		    unsigned n)
@@ -2771,6 +2918,8 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	unsigned i, count, prio;
 	struct i40e_hw_port_stats *hw_stats = &pf->stats;
+	struct i40e_if_mib_stats if_mib_stats;
+	struct i40e_ether_like_mib_stats ether_like_mib_stats;
 
 	count = i40e_xstats_calc_num();
 	if (n < count)
@@ -2778,6 +2927,9 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 
 	i40e_read_stats_registers(pf, hw);
 
+	i40e_read_if_mib(dev, &if_mib_stats);
+	i40e_read_ether_like_mib(dev, &ether_like_mib_stats);
+
 	if (xstats == NULL)
 		return 0;
 
@@ -2821,6 +2973,23 @@ i40e_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 		}
 	}
 
+	/* Get stats from IF-MIB objects */
+	for (i = 0; i < I40E_NB_IF_MIB_XSTATS; i++) {
+		xstats[count].value = *(uint64_t *)(((char *)&if_mib_stats) +
+			i40e_if_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
+
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < I40E_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		xstats[count].value =
+			*(uint64_t *)(((char *)&ether_like_mib_stats) +
+			i40e_ether_like_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
+
 	return count;
 }
 
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 183dc17..c8b6944 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -650,6 +650,62 @@ struct rte_flow {
 
 TAILQ_HEAD(i40e_flow_list, rte_flow);
 
+#define I40E_MIB_IF_TYPE_ETHERNETCSMACD		6
+
+enum {
+	I40E_MIB_TRUTH_TRUE = 1,
+	I40E_MIB_TRUTH_FALSE
+};
+
+/* IF-MIB statistics */
+struct i40e_if_mib_stats {
+	uint64_t if_number;			/* ifNumber */
+	uint64_t if_index;			/* ifIndex */
+	uint64_t if_type;			/* ifType */
+	uint64_t if_mtu;			/* ifMtu */
+	uint64_t if_speed;			/* ifSpeed */
+	uint64_t if_phys_address;		/* ifPhysAddress */
+	uint64_t if_oper_status;		/* ifOperStatus */
+	uint64_t if_last_change;		/* ifLastChange */
+	uint64_t if_high_speed;			/* ifHighSpeed */
+	uint64_t if_connector_present;		/* ifConnectorPresent */
+	uint64_t if_counter_discontinuity_time;	/* ifCounterDiscontinuityTime */
+};
+
+enum {
+	I40E_DOT3_PAUSE_DISABLED = 1,
+	I40E_DOT3_PAUSE_ENABLEDXMIT,
+	I40E_DOT3_PAUSE_ENABLEDRCV,
+	I40E_DOT3_PAUSE_ENABLEDXMITANDRCV
+};
+
+enum {
+	I40E_DOT3_DUPLEX_UNKNOWN = 1,
+	I40E_DOT3_DUPLEX_HALFDUPLEX,
+	I40E_DOT3_DUPLEX_FULLDUPLEX
+};
+
+enum {
+	I40E_DOT3_RATE_CONTROL_OFF = 1,
+	I40E_DOT3_RATE_CONTROL_ON,
+	I40E_DOT3_RATE_CONTROL_UNKNOWN
+};
+
+#define I40E_DOT3_CF_PAUSE	(1 << 0) /* PAUSE command implemented */
+#define I40E_DOT3_CF_MPCP	(1 << 1) /* MPCP implemented */
+#define I40E_DOT3_CF_PFC	(1 << 2) /* PFC implemented */
+
+/* Ethernet-like-MIB statistics */
+struct i40e_ether_like_mib_stats {
+	uint64_t dot3_pause_oper_mode;		/* dot3PauseOperMode */
+	uint64_t dot3_stats_duplex_status;	/* dot3StatsDuplexStatus */
+	uint64_t dot3_stats_rate_control_ability;
+	/* dot3StatsRateControlAbility */
+	uint64_t dot3_stats_rate_control_status;/* dot3StatsRateControlStatus */
+	uint64_t dot3_control_functions_supported;
+	/* dot3ControlFunctionsSupported */
+};
+
 /*
  * Structure to store private data specific for PF instance.
  */
@@ -808,6 +864,10 @@ struct i40e_adapter {
 	struct rte_timecounter rx_tstamp_tc;
 	struct rte_timecounter tx_tstamp_tc;
 
+	uint64_t sys_up_time_start;
+	uint64_t if_last_change;
+	uint64_t if_counter_discontinuity_time;
+
 	/* ptype mapping table */
 	uint32_t ptype_tbl[I40E_MAX_PKT_TYPE] __rte_cache_min_aligned;
 };
-- 
2.7.5

[dpdk-dev] [PATCH v2] drivers/net: add support for IF-MIB and EtherLike-MIB for e1000

[Radu Nicolau]

From: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>

If-MIB xstats:
ifNumber
ifIndex
ifType
ifMtu
ifSpeed
ifPhysAddress
ifOperStatus
ifLastChange
ifHighSpeed
ifConnectorPresent
ifCounterDiscontinuityTime

EtherLike-MIB xstats:
dot3PauseOperMode
dot3StatsDuplexStatus
dot3StatsRateControlAbility
dot3StatsRateControlStatus
dot3ControlFunctionsSupported

-updated in v2: coding style

Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
 drivers/net/e1000/e1000_ethdev.h |  79 +++++++++--
 drivers/net/e1000/igb_ethdev.c   | 296 +++++++++++++++++++++++++++++++++++----
 2 files changed, 341 insertions(+), 34 deletions(-)

diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 9266540..1fc82b4 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -275,20 +275,79 @@ struct e1000_filter_info {
 	uint32_t syn_info;
 };
 
+#define E1000_MIB_IF_TYPE_ETHERNETCSMACD	6
+
+enum {
+	E1000_MIB_TRUTH_TRUE = 1,
+	E1000_MIB_TRUTH_FALSE
+};
+
+/* IF-MIB statistics */
+struct e1000_if_mib_stats {
+	uint64_t if_number;			/* ifNumber */
+	uint64_t if_index;			/* ifIndex */
+	uint64_t if_type;			/* ifType */
+	uint64_t if_mtu;			/* ifMtu */
+	uint64_t if_speed;			/* ifSpeed */
+	uint64_t if_phys_address;		/* ifPhysAddress */
+	uint64_t if_oper_status;		/* ifOperStatus */
+	uint64_t if_last_change;		/* ifLastChange */
+	uint64_t if_high_speed;			/* ifHighSpeed */
+	uint64_t if_connector_present;		/* ifConnectorPresent */
+	uint64_t if_counter_discontinuity_time;	/* ifCounterDiscontinuityTime */
+};
+
+enum {
+	E1000_DOT3_PAUSE_DISABLED = 1,
+	E1000_DOT3_PAUSE_ENABLEDXMIT,
+	E1000_DOT3_PAUSE_ENABLEDRCV,
+	E1000_DOT3_PAUSE_ENABLEDXMITANDRCV
+};
+
+enum {
+	E1000_DOT3_DUPLEX_UNKNOWN = 1,
+	E1000_DOT3_DUPLEX_HALFDUPLEX,
+	E1000_DOT3_DUPLEX_FULLDUPLEX
+};
+
+enum {
+	E1000_DOT3_RATE_CONTROL_OFF = 1,
+	E1000_DOT3_RATE_CONTROL_ON,
+	E1000_DOT3_RATE_CONTROL_UNKNOWN
+};
+
+#define E1000_DOT3_CF_PAUSE	(1 << 0) /* PAUSE command implemented */
+#define E1000_DOT3_CF_MPCP	(1 << 1) /* MPCP implemented */
+#define E1000_DOT3_CF_PFC	(1 << 2) /* PFC implemented */
+
+/* Ethernet-like-MIB statistics */
+struct e1000_ether_like_mib_stats {
+	uint64_t dot3_pause_oper_mode;		/* dot3PauseOperMode */
+	uint64_t dot3_stats_duplex_status;	/* dot3StatsDuplexStatus */
+	uint64_t dot3_stats_rate_control_ability;
+	/* dot3StatsRateControlAbility */
+	uint64_t dot3_stats_rate_control_status;/* dot3StatsRateControlStatus */
+	uint64_t dot3_control_functions_supported;
+	/* dot3ControlFunctionsSupported */
+};
+
 /*
  * Structure to store private data for each driver instance (for each port).
  */
 struct e1000_adapter {
-	struct e1000_hw         hw;
-	struct e1000_hw_stats   stats;
-	struct e1000_interrupt  intr;
-	struct e1000_vfta       shadow_vfta;
-	struct e1000_vf_info    *vfdata;
-	struct e1000_filter_info filter;
-	bool stopped;
-	struct rte_timecounter  systime_tc;
-	struct rte_timecounter  rx_tstamp_tc;
-	struct rte_timecounter  tx_tstamp_tc;
+	struct e1000_hw			hw;
+	struct e1000_hw_stats		stats;
+	struct e1000_interrupt		intr;
+	struct e1000_vfta		shadow_vfta;
+	struct e1000_vf_info		*vfdata;
+	struct e1000_filter_info	filter;
+	bool				stopped;
+	struct rte_timecounter		systime_tc;
+	struct rte_timecounter		rx_tstamp_tc;
+	struct rte_timecounter		tx_tstamp_tc;
+	uint64_t			sys_up_time_start;
+	uint64_t			if_last_change;
+	uint64_t			if_counter_discontinuity_time;
 };
 
 #define E1000_DEV_PRIVATE(adapter) \
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index a0da9d5..1f6fcdb 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -549,6 +549,45 @@ static const struct rte_igb_xstats_name_off rte_igbvf_stats_strings[] = {
 #define IGBVF_NB_XSTATS (sizeof(rte_igbvf_stats_strings) / \
 		sizeof(rte_igbvf_stats_strings[0]))
 
+static const struct rte_igb_xstats_name_off igb_if_mib_strings[] = {
+	{"ifNumber", offsetof(struct e1000_if_mib_stats, if_number)},
+	{"ifIndex", offsetof(struct e1000_if_mib_stats, if_index)},
+	{"ifType", offsetof(struct e1000_if_mib_stats, if_type)},
+	{"ifMtu", offsetof(struct e1000_if_mib_stats, if_mtu)},
+	{"ifSpeed", offsetof(struct e1000_if_mib_stats, if_speed)},
+	{"ifPhysAddress", offsetof(struct e1000_if_mib_stats, if_phys_address)},
+	{"ifOperStatus", offsetof(struct e1000_if_mib_stats, if_oper_status)},
+	{"ifLastChange", offsetof(struct e1000_if_mib_stats, if_last_change)},
+	{"ifHighSpeed", offsetof(struct e1000_if_mib_stats, if_high_speed)},
+	{"ifConnectorPresent", offsetof(struct e1000_if_mib_stats,
+			if_connector_present)},
+	{"ifCounterDiscontinuityTime", offsetof(struct e1000_if_mib_stats,
+			if_counter_discontinuity_time)},
+};
+
+#define IGB_NB_IF_MIB_XSTATS (sizeof(igb_if_mib_strings) / \
+		sizeof(igb_if_mib_strings[0]))
+
+static const struct rte_igb_xstats_name_off igb_ether_like_mib_strings[] = {
+	{"dot3PauseOperMode", offsetof(struct e1000_ether_like_mib_stats,
+			dot3_pause_oper_mode)},
+	{"dot3StatsDuplexStatus", offsetof(struct e1000_ether_like_mib_stats,
+			dot3_stats_duplex_status)},
+	{"dot3StatsRateControlAbility", offsetof(
+			struct e1000_ether_like_mib_stats,
+			dot3_stats_rate_control_ability)},
+	{"dot3StatsRateControlStatus", offsetof(
+			struct e1000_ether_like_mib_stats,
+			dot3_stats_rate_control_status)},
+	{"dot3ControlFunctionsSupported", offsetof(
+			struct e1000_ether_like_mib_stats,
+			dot3_control_functions_supported)},
+};
+
+#define IGB_NB_ETHER_LIKE_MIB_XSTATS \
+		(sizeof(igb_ether_like_mib_strings) / \
+		sizeof(igb_ether_like_mib_strings[0]))
+
 /**
  * Atomically reads the link status information from global
  * structure rte_eth_dev.
@@ -925,6 +964,11 @@ eth_igb_dev_init(struct rte_eth_dev *eth_dev)
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id);
 
+	/* indicate sysUpTime start */
+	adapter->sys_up_time_start = rte_rdtsc();
+	adapter->if_last_change = 0;
+	adapter->if_counter_discontinuity_time = 0;
+
 	rte_intr_callback_register(&pci_dev->intr_handle,
 				   eth_igb_interrupt_handler,
 				   (void *)eth_dev);
@@ -1114,6 +1158,11 @@ eth_igbvf_dev_init(struct rte_eth_dev *eth_dev)
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id, "igb_mac_82576_vf");
 
+	/* indicate sysUpTime start */
+	adapter->sys_up_time_start = rte_rdtsc();
+	adapter->if_last_change = 0;
+	adapter->if_counter_discontinuity_time = 0;
+
 	intr_handle = &pci_dev->intr_handle;
 	rte_intr_callback_register(intr_handle,
 				   eth_igbvf_interrupt_handler, eth_dev);
@@ -1858,12 +1907,17 @@ eth_igb_stats_reset(struct rte_eth_dev *dev)
 {
 	struct e1000_hw_stats *hw_stats =
 			E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
+	struct e1000_adapter *adapter =
+		E1000_DEV_PRIVATE(dev->data->dev_private);
 
 	/* HW registers are cleared on read */
 	eth_igb_stats_get(dev, NULL);
 
 	/* Reset software totals */
 	memset(hw_stats, 0, sizeof(*hw_stats));
+
+	adapter->if_counter_discontinuity_time =
+			rte_rdtsc() - adapter->sys_up_time_start;
 }
 
 static void
@@ -1871,31 +1925,138 @@ eth_igb_xstats_reset(struct rte_eth_dev *dev)
 {
 	struct e1000_hw_stats *stats =
 			E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
+	struct e1000_adapter *adapter =
+		E1000_DEV_PRIVATE(dev->data->dev_private);
 
 	/* HW registers are cleared on read */
 	eth_igb_xstats_get(dev, NULL, IGB_NB_XSTATS);
 
 	/* Reset software totals */
 	memset(stats, 0, sizeof(*stats));
+
+	adapter->if_counter_discontinuity_time =
+			rte_rdtsc() - adapter->sys_up_time_start;
+}
+
+static unsigned int
+igb_xstats_calc_num(void)
+{
+	return IGB_NB_XSTATS + IGB_NB_IF_MIB_XSTATS +
+		IGB_NB_ETHER_LIKE_MIB_XSTATS;
 }
 
 static int eth_igb_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	struct rte_eth_xstat_name *xstats_names,
 	__rte_unused unsigned int size)
 {
-	unsigned i;
+	unsigned int i, count = 0;
 
 	if (xstats_names == NULL)
-		return IGB_NB_XSTATS;
+		return igb_xstats_calc_num();
 
 	/* Note: limit checked in rte_eth_xstats_names() */
 
 	for (i = 0; i < IGB_NB_XSTATS; i++) {
-		snprintf(xstats_names[i].name, sizeof(xstats_names[i].name),
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
 			 "%s", rte_igb_stats_strings[i].name);
+		count++;
+	}
+
+	/* Get stats from IF-MIB objects */
+	for (i = 0; i < IGB_NB_IF_MIB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			 "%s", igb_if_mib_strings[i].name);
+		count++;
+	}
+
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < IGB_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			 "%s", igb_ether_like_mib_strings[i].name);
+		count++;
 	}
 
-	return IGB_NB_XSTATS;
+	return count;
+}
+
+static void
+igb_read_if_mib(struct rte_eth_dev *dev, struct e1000_if_mib_stats *stats)
+{
+	struct rte_eth_dev_data *data = dev->data;
+	struct rte_device *device = dev->device;
+	struct e1000_adapter *adapter =
+			E1000_DEV_PRIVATE(dev->data->dev_private);
+
+	stats->if_number = rte_eth_dev_count();
+	stats->if_index = data->port_id + 1;
+	stats->if_type = E1000_MIB_IF_TYPE_ETHERNETCSMACD;
+	stats->if_mtu = data->mtu;
+	stats->if_speed = (data->dev_link.link_speed < (UINT32_MAX / 1000000)) ?
+			(data->dev_link.link_speed * 1000000) : UINT32_MAX;
+	stats->if_phys_address = 0;
+	ether_addr_copy(data->mac_addrs,
+			(struct ether_addr *)&stats->if_phys_address);
+	stats->if_oper_status = data->dev_link.link_status ?
+			E1000_MIB_TRUTH_TRUE : E1000_MIB_TRUTH_FALSE;
+	stats->if_last_change = adapter->if_last_change /
+			(rte_get_tsc_hz() * 100);
+	stats->if_high_speed = data->dev_link.link_speed;
+	if (device->devargs)
+		stats->if_connector_present =
+				(device->devargs->type == RTE_DEVTYPE_VIRTUAL) ?
+						E1000_MIB_TRUTH_FALSE :
+						E1000_MIB_TRUTH_TRUE;
+	else
+		stats->if_connector_present = 0;
+	stats->if_counter_discontinuity_time =
+			adapter->if_counter_discontinuity_time /
+			(rte_get_tsc_hz() * 100);
+}
+
+static void
+igb_read_ether_like_mib(struct rte_eth_dev *dev,
+		struct e1000_ether_like_mib_stats *stats)
+{
+	struct rte_eth_dev_data *data = dev->data;
+	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	switch (hw->fc.current_mode) {
+	case e1000_fc_none:
+		stats->dot3_pause_oper_mode = E1000_DOT3_PAUSE_DISABLED;
+		break;
+	case e1000_fc_rx_pause:
+		stats->dot3_pause_oper_mode = E1000_DOT3_PAUSE_ENABLEDRCV;
+		break;
+	case e1000_fc_tx_pause:
+		stats->dot3_pause_oper_mode = E1000_DOT3_PAUSE_ENABLEDXMIT;
+		break;
+	case e1000_fc_full:
+		stats->dot3_pause_oper_mode =
+				E1000_DOT3_PAUSE_ENABLEDXMITANDRCV;
+		break;
+	default:
+		stats->dot3_pause_oper_mode = 0;
+		break;
+	}
+
+	switch (data->dev_link.link_duplex) {
+	case ETH_LINK_FULL_DUPLEX:
+		stats->dot3_stats_duplex_status = E1000_DOT3_DUPLEX_FULLDUPLEX;
+		break;
+	case ETH_LINK_HALF_DUPLEX:
+		stats->dot3_stats_duplex_status = E1000_DOT3_DUPLEX_HALFDUPLEX;
+		break;
+	default:
+		stats->dot3_stats_duplex_status = E1000_DOT3_DUPLEX_UNKNOWN;
+		break;
+	}
+
+	stats->dot3_stats_rate_control_ability = E1000_MIB_TRUTH_FALSE;
+	stats->dot3_stats_rate_control_status = E1000_DOT3_RATE_CONTROL_OFF;
+	stats->dot3_control_functions_supported = E1000_DOT3_CF_PAUSE;
 }
 
 static int eth_igb_xstats_get_names_by_id(struct rte_eth_dev *dev,
@@ -1940,27 +2101,53 @@ eth_igb_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct e1000_hw_stats *hw_stats =
 			E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
-	unsigned i;
+	struct e1000_if_mib_stats if_mib_stats;
+	struct e1000_ether_like_mib_stats ether_like_mib_stats;
+	unsigned int i, count;
 
-	if (n < IGB_NB_XSTATS)
-		return IGB_NB_XSTATS;
+	count = igb_xstats_calc_num();
+	if (n < count)
+		return count;
 
 	igb_read_stats_registers(hw, hw_stats);
 
+	igb_read_if_mib(dev, &if_mib_stats);
+	igb_read_ether_like_mib(dev, &ether_like_mib_stats);
+
 	/* If this is a reset xstats is NULL, and we have cleared the
 	 * registers by reading them.
 	 */
 	if (!xstats)
 		return 0;
 
+	count = 0;
+
 	/* Extended stats */
 	for (i = 0; i < IGB_NB_XSTATS; i++) {
-		xstats[i].id = i;
-		xstats[i].value = *(uint64_t *)(((char *)hw_stats) +
+		xstats[count].id = count;
+		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 			rte_igb_stats_strings[i].offset);
+		count++;
+	}
+
+	/* Get stats from IF-MIB objects */
+	for (i = 0; i < IGB_NB_IF_MIB_XSTATS; i++) {
+		xstats[count].value = *(uint64_t *)(((char *)&if_mib_stats) +
+			igb_if_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
 	}
 
-	return IGB_NB_XSTATS;
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < IGB_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		xstats[count].value =
+			*(uint64_t *)(((char *)&ether_like_mib_stats) +
+			igb_ether_like_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
+
+	return count;
 }
 
 static int
@@ -2050,19 +2237,46 @@ igbvf_read_stats_registers(struct e1000_hw *hw, struct e1000_vf_stats *hw_stats)
 	    hw_stats->last_gotlbc, hw_stats->gotlbc);
 }
 
+static unsigned int
+igbvf_xstats_calc_num(void)
+{
+	return IGBVF_NB_XSTATS + IGB_NB_IF_MIB_XSTATS +
+		IGB_NB_ETHER_LIKE_MIB_XSTATS;
+}
+
 static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 				     struct rte_eth_xstat_name *xstats_names,
 				     __rte_unused unsigned limit)
 {
-	unsigned i;
+	unsigned int i, count = 0;
 
-	if (xstats_names != NULL)
-		for (i = 0; i < IGBVF_NB_XSTATS; i++) {
-			snprintf(xstats_names[i].name,
-				sizeof(xstats_names[i].name), "%s",
-				rte_igbvf_stats_strings[i].name);
-		}
-	return IGBVF_NB_XSTATS;
+	if (xstats_names == NULL)
+		return igbvf_xstats_calc_num();
+
+	for (i = 0; i < IGBVF_NB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name), "%s",
+			rte_igbvf_stats_strings[i].name);
+		count++;
+	}
+
+	/* Get stats from IF-MIB objects */
+	for (i = 0; i < IGB_NB_IF_MIB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			 "%s", igb_if_mib_strings[i].name);
+		count++;
+	}
+
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < IGB_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		snprintf(xstats_names[count].name,
+			sizeof(xstats_names[count].name),
+			 "%s", igb_ether_like_mib_strings[i].name);
+		count++;
+	}
+
+	return count;
 }
 
 static int
@@ -2072,23 +2286,49 @@ eth_igbvf_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct e1000_vf_stats *hw_stats = (struct e1000_vf_stats *)
 			E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
-	unsigned i;
+	struct e1000_if_mib_stats if_mib_stats;
+	struct e1000_ether_like_mib_stats ether_like_mib_stats;
+	unsigned int i, count;
 
-	if (n < IGBVF_NB_XSTATS)
-		return IGBVF_NB_XSTATS;
+	count = igbvf_xstats_calc_num();
+	if (n < count)
+		return count;
 
 	igbvf_read_stats_registers(hw, hw_stats);
 
+	igb_read_if_mib(dev, &if_mib_stats);
+	igb_read_ether_like_mib(dev, &ether_like_mib_stats);
+
 	if (!xstats)
 		return 0;
 
+	count = 0;
+
 	for (i = 0; i < IGBVF_NB_XSTATS; i++) {
-		xstats[i].id = i;
-		xstats[i].value = *(uint64_t *)(((char *)hw_stats) +
+		xstats[count].id = count;
+		xstats[count].value = *(uint64_t *)(((char *)hw_stats) +
 			rte_igbvf_stats_strings[i].offset);
+		count++;
 	}
 
-	return IGBVF_NB_XSTATS;
+	/* Get stats from IF-MIB objects */
+	for (i = 0; i < IGB_NB_IF_MIB_XSTATS; i++) {
+		xstats[count].value = *(uint64_t *)(((char *)&if_mib_stats) +
+			igb_if_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
+
+	/* Get stats from Ethernet-like-MIB objects */
+	for (i = 0; i < IGB_NB_ETHER_LIKE_MIB_XSTATS; i++) {
+		xstats[count].value =
+			*(uint64_t *)(((char *)&ether_like_mib_stats) +
+			igb_ether_like_mib_strings[i].offset);
+		xstats[count].id = count;
+		count++;
+	}
+
+	return count;
 }
 
 static void
@@ -2114,6 +2354,8 @@ eth_igbvf_stats_reset(struct rte_eth_dev *dev)
 {
 	struct e1000_vf_stats *hw_stats = (struct e1000_vf_stats*)
 			E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
+	struct e1000_adapter *adapter =
+		E1000_DEV_PRIVATE(dev->data->dev_private);
 
 	/* Sync HW register to the last stats */
 	eth_igbvf_stats_get(dev, NULL);
@@ -2121,6 +2363,9 @@ eth_igbvf_stats_reset(struct rte_eth_dev *dev)
 	/* reset HW current stats*/
 	memset(&hw_stats->gprc, 0, sizeof(*hw_stats) -
 	       offsetof(struct e1000_vf_stats, gprc));
+
+	adapter->if_counter_discontinuity_time =
+			rte_rdtsc() - adapter->sys_up_time_start;
 }
 
 static int
@@ -2366,6 +2611,8 @@ eth_igb_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 {
 	struct e1000_hw *hw =
 		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct e1000_adapter *adapter =
+		E1000_DEV_PRIVATE(dev->data->dev_private);
 	struct rte_eth_link link, old;
 	int link_check, count;
 
@@ -2434,6 +2681,7 @@ eth_igb_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 		return -1;
 
 	/* changed */
+	adapter->if_last_change = rte_rdtsc() - adapter->sys_up_time_start;
 	return 0;
 }
 
-- 
2.7.5

Re: [dpdk-dev] [PATCH v2] drivers/net: add support for IF-MIB and EtherLike-MIB for e1000

[Ferruh Yigit]

On 6/26/2017 10:42 AM, Radu Nicolau wrote:
> From: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
> 
> If-MIB xstats:
> ifNumber
> ifIndex
> ifType
> ifMtu
> ifSpeed
> ifPhysAddress
> ifOperStatus
> ifLastChange
> ifHighSpeed
> ifConnectorPresent
> ifCounterDiscontinuityTime
> 
> EtherLike-MIB xstats:
> dot3PauseOperMode
> dot3StatsDuplexStatus
> dot3StatsRateControlAbility
> dot3StatsRateControlStatus
> dot3ControlFunctionsSupported
> 
> -updated in v2: coding style
> 
> Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
> Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>

<...>

This patch implements MIBs for some Intel drivers using xstats, this is
easy way to get some information from PMDs.

But there was a outstanding comment to make this ethdev layer.

So I believe we have two options:

[1]
Each PMD implements MIBs using xstats, this is pragmatic solution and
each PMD can implement whichever MIBs they want.

[2]
Implement a ethdev layer API and add a new dev_ops to get MIBs, API can
use existing methods to get required information, and if it fails can
call dev_ops which can be similar to the xstats.
Because of API all PMDs can have a small amount of support by default
and they can implement dev_ops for more support.


Although 2) looks more generic and proper, I am not really sure if that
is overkill and if this worth to the effort and to have new API and
dev_ops, comparing current method is easy to implement, any comments?

Thanks,
ferruh

Re: [dpdk-dev] [PATCH v2] drivers/net: add support for IF-MIB and EtherLike-MIB for e1000

[Bruce Richardson]

On Tue, Jun 27, 2017 at 12:08:56PM +0100, Ferruh Yigit wrote:
> On 6/26/2017 10:42 AM, Radu Nicolau wrote:
> > From: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
> > 
> > If-MIB xstats:
> > ifNumber
> > ifIndex
> > ifType
> > ifMtu
> > ifSpeed
> > ifPhysAddress
> > ifOperStatus
> > ifLastChange
> > ifHighSpeed
> > ifConnectorPresent
> > ifCounterDiscontinuityTime
> > 
> > EtherLike-MIB xstats:
> > dot3PauseOperMode
> > dot3StatsDuplexStatus
> > dot3StatsRateControlAbility
> > dot3StatsRateControlStatus
> > dot3ControlFunctionsSupported
> > 
> > -updated in v2: coding style
> > 
> > Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
> > Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
> > Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> 
> <...>
> 
> This patch implements MIBs for some Intel drivers using xstats, this is
> easy way to get some information from PMDs.
> 
> But there was a outstanding comment to make this ethdev layer.
> 
> So I believe we have two options:
> 
> [1]
> Each PMD implements MIBs using xstats, this is pragmatic solution and
> each PMD can implement whichever MIBs they want.
> 
> [2]
> Implement a ethdev layer API and add a new dev_ops to get MIBs, API can
> use existing methods to get required information, and if it fails can
> call dev_ops which can be similar to the xstats.
> Because of API all PMDs can have a small amount of support by default
> and they can implement dev_ops for more support.
> 
> 
> Although 2) looks more generic and proper, I am not really sure if that
> is overkill and if this worth to the effort and to have new API and
> dev_ops, comparing current method is easy to implement, any comments?
> 

For 2, does the "use existing methods" include calling xstats? If so,
then we can just drop the requirement for 2 to have any new functions
implemented in the driver. Instead, have the information provided by
drivers in the normal xstats call, but have a new ethdev API to provide
that information to the app - basically hiding the xstats complexity
underneath.

/Bruce

Re: [dpdk-dev] [PATCH v2] drivers/net: add support for IF-MIB and EtherLike-MIB for e1000

[Ferruh Yigit]

On 6/27/2017 12:21 PM, Bruce Richardson wrote:
> On Tue, Jun 27, 2017 at 12:08:56PM +0100, Ferruh Yigit wrote:
>> On 6/26/2017 10:42 AM, Radu Nicolau wrote:
>>> From: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
>>>
>>> If-MIB xstats:
>>> ifNumber
>>> ifIndex
>>> ifType
>>> ifMtu
>>> ifSpeed
>>> ifPhysAddress
>>> ifOperStatus
>>> ifLastChange
>>> ifHighSpeed
>>> ifConnectorPresent
>>> ifCounterDiscontinuityTime
>>>
>>> EtherLike-MIB xstats:
>>> dot3PauseOperMode
>>> dot3StatsDuplexStatus
>>> dot3StatsRateControlAbility
>>> dot3StatsRateControlStatus
>>> dot3ControlFunctionsSupported
>>>
>>> -updated in v2: coding style
>>>
>>> Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
>>> Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
>>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>>
>> <...>
>>
>> This patch implements MIBs for some Intel drivers using xstats, this is
>> easy way to get some information from PMDs.
>>
>> But there was a outstanding comment to make this ethdev layer.
>>
>> So I believe we have two options:
>>
>> [1]
>> Each PMD implements MIBs using xstats, this is pragmatic solution and
>> each PMD can implement whichever MIBs they want.
>>
>> [2]
>> Implement a ethdev layer API and add a new dev_ops to get MIBs, API can
>> use existing methods to get required information, and if it fails can
>> call dev_ops which can be similar to the xstats.
>> Because of API all PMDs can have a small amount of support by default
>> and they can implement dev_ops for more support.
>>
>>
>> Although 2) looks more generic and proper, I am not really sure if that
>> is overkill and if this worth to the effort and to have new API and
>> dev_ops, comparing current method is easy to implement, any comments?
>>
> 
> For 2, does the "use existing methods" include calling xstats? If so,
> then we can just drop the requirement for 2 to have any new functions
> implemented in the driver. Instead, have the information provided by
> drivers in the normal xstats call, but have a new ethdev API to provide
> that information to the app - basically hiding the xstats complexity
> underneath.

I was thinking another dev_ops similar to xstats, but re-using xstats
can be better idea here.

so option 2 becomes:
[2] An eth_dev API to use existing APIs to get information from PMDs and
wrap MIBs related xstats calls.

But I believe if option 2 is overkill for this task question is still valid.

Thanks,
ferruh

Re: [dpdk-dev] [PATCH v2] drivers/net: add support for IF-MIB and EtherLike-MIB for e1000

[Stephen Hemminger]

On Mon, 26 Jun 2017 10:42:13 +0100
Radu Nicolau <radu.nicolau@intel.com> wrote:

> From: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
> 
> If-MIB xstats:
> ifNumber
> ifIndex
> ifType
> ifMtu
> ifSpeed
> ifPhysAddress
> ifOperStatus
> ifLastChange
> ifHighSpeed
> ifConnectorPresent
> ifCounterDiscontinuityTime
> 
> EtherLike-MIB xstats:
> dot3PauseOperMode
> dot3StatsDuplexStatus
> dot3StatsRateControlAbility
> dot3StatsRateControlStatus
> dot3ControlFunctionsSupported
> 
> -updated in v2: coding style
> 
> Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
> Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>

Although this maybe the easiest way for Intel, and satisfy a specific user's
request. It is not a good way forward for the DPDK project.

This must be generic, not specific to device drivers.
If implementing a MIB variable  requires more information than ethdev API
has now, then extend ethdev API first.

One of the common things in open source projects, is that when you want
to add one new feature for one constrained portion; the maintainers require
you to implement a generic solution. This falls into that category.

Re: [dpdk-dev] [PATCH v2] drivers/net: add support for IF-MIB and EtherLike-MIB for ixgbe

[Stephen Hemminger]

On Mon, 26 Jun 2017 10:39:07 +0100
Radu Nicolau <radu.nicolau@intel.com> wrote:

> +static const struct rte_ixgbe_xstats_name_off ixgbe_if_mib_strings[] = {
> +	{"ifNumber", offsetof(struct ixgbe_if_mib_stats, if_number)},
> +	{"ifIndex", offsetof(struct ixgbe_if_mib_stats, if_index)},
> +	{"ifType", offsetof(struct ixgbe_if_mib_stats, if_type)},
> +	{"ifMtu", offsetof(struct ixgbe_if_mib_stats, if_mtu)},
> +	{"ifSpeed", offsetof(struct ixgbe_if_mib_stats, if_speed)},
> +	{"ifPhysAddress", offsetof(struct ixgbe_if_mib_stats, if_phys_address)},
> +	{"ifOperStatus", offsetof(struct ixgbe_if_mib_stats, if_oper_status)},
> +	{"ifLastChange", offsetof(struct ixgbe_if_mib_stats, if_last_change)},
> +	{"ifHighSpeed", offsetof(struct ixgbe_if_mib_stats, if_high_speed)},
> +	{"ifConnectorPresent", offsetof(struct ixgbe_if_mib_stats,
> +			if_connector_present)},
> +	{"ifCounterDiscontinuityTime", offsetof(struct ixgbe_if_mib_stats,
> +			if_counter_discontinuity_time)},

Most of these should not be xstats. Things like if_index, type and MTU are available
through other API's already.

Re: [dpdk-dev] [PATCH v2] drivers/net: add support for IF-MIB and EtherLike-MIB for e1000

[Pattan, Reshma]

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Tuesday, June 27, 2017 12:36 PM
> > For 2, does the "use existing methods" include calling xstats? If so,
> > then we can just drop the requirement for 2 to have any new functions
> > implemented in the driver. Instead, have the information provided by
> > drivers in the normal xstats call, but have a new ethdev API to
> > provide that information to the app - basically hiding the xstats
> > complexity underneath.
> 
> I was thinking another dev_ops similar to xstats, but re-using xstats can be
> better idea here.
> 
> so option 2 becomes:
> [2] An eth_dev API to use existing APIs to get information from PMDs and
> wrap MIBs related xstats calls.
> 

Instead of getting information/configuration related mib attributes  in xstats_get,  we should  have new eth_dev op to get  all those mib attributes which don’t have any ethdev API. 
Then the new eth_dev API can call this new eth_dev op and other existing ethdev APIs to gather all mib information and expose that to user.

Thanks,
Reshma

Re: [dpdk-dev] [PATCH v2] drivers/net: add support for IF-MIB and EtherLike-MIB for e1000

[Pattan, Reshma]

Hi ,

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Tuesday, June 27, 2017 11:26 PM
> To: Nicolau, Radu <radu.nicolau@intel.com>
> Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Pattan, Reshma
> <reshma.pattan@intel.com>; Jastrzebski, MichalX K
> <michalx.k.jastrzebski@intel.com>; Jain, Deepak K
> <deepak.k.jain@intel.com>; Van Haaren, Harry
> <harry.van.haaren@intel.com>; piotrx.t.azarewicz@intel.com
> Subject: Re: [dpdk-dev] [PATCH v2] drivers/net: add support for IF-MIB and
> EtherLike-MIB for e1000
> 
> On Mon, 26 Jun 2017 10:42:13 +0100
> Radu Nicolau <radu.nicolau@intel.com> wrote:
> 
> > From: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
> >
> > If-MIB xstats:
> > ifNumber
> > ifIndex
> > ifType
> > ifMtu
> > ifSpeed
> > ifPhysAddress
> > ifOperStatus
> > ifLastChange
> > ifHighSpeed
> > ifConnectorPresent
> > ifCounterDiscontinuityTime
> >
> > EtherLike-MIB xstats:
> > dot3PauseOperMode
> > dot3StatsDuplexStatus
> > dot3StatsRateControlAbility
> > dot3StatsRateControlStatus
> > dot3ControlFunctionsSupported
> >
> > -updated in v2: coding style
> >
> > Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
> > Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
> > Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> 
> Although this maybe the easiest way for Intel, and satisfy a specific user's
> request. It is not a good way forward for the DPDK project.
> 
> This must be generic, not specific to device drivers.
> If implementing a MIB variable  requires more information than ethdev API
> has now, then extend ethdev API first.
> 

Yes, most of the MIB attributes can be fetched from rte_eth_dev_data/rte_eth_dev_info. 
Re expressing my opinion (a) below which I did in other mail: 
(a)For the MIB attributes which do not have any ethdev API support, how about getting all of them from PMDs via a new dev_op like xstats_get?. 
   Then add the new eth_dev API, which does call to this new dev_op and other existing ethdev APIs to gather all mib information and expose that 
   to user based on port_id. 

(Or)

(b)Should we expand rte_eth_dev_info or rte_eth_dev_data to add missing mib attributes, fetch them from PMDs using dev_infos_get() and expose to user.
   Adding new mib attributes to existing  structs might need ABI break announcements.  
   
But both cases (a) and (b) does need some PMD changes.

Please let me know what you think about the  points a and b.

Thanks,
Reshma

Re: [dpdk-dev] [PATCH v2] drivers/net: add support for IF-MIB and EtherLike-MIB for e1000

[Ferruh Yigit]

On 6/26/2017 10:42 AM, Radu Nicolau wrote:
> From: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
> 
> If-MIB xstats:
> ifNumber
> ifIndex
> ifType
> ifMtu
> ifSpeed
> ifPhysAddress
> ifOperStatus
> ifLastChange
> ifHighSpeed
> ifConnectorPresent
> ifCounterDiscontinuityTime
> 
> EtherLike-MIB xstats:
> dot3PauseOperMode
> dot3StatsDuplexStatus
> dot3StatsRateControlAbility
> dot3StatsRateControlStatus
> dot3ControlFunctionsSupported
> 
> -updated in v2: coding style
> 
> Signed-off-by: Piotr Azarewicz <piotrx.t.azarewicz@intel.com>
> Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>

This patchset won't able to make for this release. There is already a
new RFC [1] for next release.

[1]
http://dpdk.org/dev/patchwork/patch/27032/