netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] cxgb4/chcr: nic-tls stats in ethtool
@ 2020-03-21 11:23 Rohit Maheshwari
  2020-03-22 20:57 ` Jakub Kicinski
  0 siblings, 1 reply; 3+ messages in thread
From: Rohit Maheshwari @ 2020-03-21 11:23 UTC (permalink / raw)
  To: davem, netdev, kuba; +Cc: borisp, secdev, Rohit Maheshwari

Included nic tls statistics in ethtool stats.

Signed-off-by: Rohit Maheshwari <rohitm@chelsio.com>
---
 .../ethernet/chelsio/cxgb4/cxgb4_ethtool.c    | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c
index 398ade42476c..4998f1d1e218 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c
@@ -134,6 +134,28 @@ static char loopback_stats_strings[][ETH_GSTRING_LEN] = {
 	"bg3_frames_trunc       ",
 };
 
+#ifdef CONFIG_CHELSIO_TLS_DEVICE
+struct chcr_tls_stats {
+	u64 tx_tls_encrypted_packets;
+	u64 tx_tls_encrypted_bytes;
+	u64 tx_tls_ctx;
+	u64 tx_tls_ooo;
+	u64 tx_tls_skip_no_sync_data;
+	u64 tx_tls_drop_no_sync_data;
+	u64 tx_tls_drop_bypass_req;
+};
+
+static char chcr_tls_stats_strings[][ETH_GSTRING_LEN] = {
+	"tx_tls_encrypted_pkts  ",
+	"tx_tls_encrypted_bytes ",
+	"tx_tls_ctx             ",
+	"tx_tls_ooo             ",
+	"tx_tls_skip_nosync_data",
+	"tx_tls_drop_nosync_data",
+	"tx_tls_drop_bypass_req ",
+};
+#endif
+
 static const char cxgb4_priv_flags_strings[][ETH_GSTRING_LEN] = {
 	[PRIV_FLAG_PORT_TX_VM_BIT] = "port_tx_vm_wr",
 };
@@ -144,6 +166,9 @@ static int get_sset_count(struct net_device *dev, int sset)
 	case ETH_SS_STATS:
 		return ARRAY_SIZE(stats_strings) +
 		       ARRAY_SIZE(adapter_stats_strings) +
+#ifdef CONFIG_CHELSIO_TLS_DEVICE
+		       ARRAY_SIZE(chcr_tls_stats_strings) +
+#endif
 		       ARRAY_SIZE(loopback_stats_strings);
 	case ETH_SS_PRIV_FLAGS:
 		return ARRAY_SIZE(cxgb4_priv_flags_strings);
@@ -204,6 +229,11 @@ static void get_strings(struct net_device *dev, u32 stringset, u8 *data)
 		memcpy(data, adapter_stats_strings,
 		       sizeof(adapter_stats_strings));
 		data += sizeof(adapter_stats_strings);
+#ifdef CONFIG_CHELSIO_TLS_DEVICE
+		memcpy(data, chcr_tls_stats_strings,
+		       sizeof(chcr_tls_stats_strings));
+		data += sizeof(chcr_tls_stats_strings);
+#endif
 		memcpy(data, loopback_stats_strings,
 		       sizeof(loopback_stats_strings));
 	} else if (stringset == ETH_SS_PRIV_FLAGS) {
@@ -289,6 +319,29 @@ static void collect_adapter_stats(struct adapter *adap, struct adapter_stats *s)
 	}
 }
 
+#ifdef CONFIG_CHELSIO_TLS_DEVICE
+static void collect_chcr_tls_stats(struct adapter *adap,
+				   struct chcr_tls_stats *s)
+{
+	struct chcr_stats_debug *stats = &adap->chcr_stats;
+
+	memset(s, 0, sizeof(*s));
+
+	s->tx_tls_encrypted_packets =
+		atomic64_read(&stats->ktls_tx_encrypted_packets);
+	s->tx_tls_encrypted_bytes =
+		atomic64_read(&stats->ktls_tx_encrypted_bytes);
+	s->tx_tls_ctx = atomic64_read(&stats->ktls_tx_ctx);
+	s->tx_tls_ooo = atomic64_read(&stats->ktls_tx_ooo);
+	s->tx_tls_skip_no_sync_data =
+		atomic64_read(&stats->ktls_tx_skip_no_sync_data);
+	s->tx_tls_drop_no_sync_data =
+		atomic64_read(&stats->ktls_tx_drop_no_sync_data);
+	s->tx_tls_drop_bypass_req =
+		atomic64_read(&stats->ktls_tx_drop_bypass_req);
+}
+#endif
+
 static void get_stats(struct net_device *dev, struct ethtool_stats *stats,
 		      u64 *data)
 {
@@ -307,6 +360,10 @@ static void get_stats(struct net_device *dev, struct ethtool_stats *stats,
 	data += sizeof(struct queue_port_stats) / sizeof(u64);
 	collect_adapter_stats(adapter, (struct adapter_stats *)data);
 	data += sizeof(struct adapter_stats) / sizeof(u64);
+#ifdef CONFIG_CHELSIO_TLS_DEVICE
+	collect_chcr_tls_stats(adapter, (struct chcr_tls_stats *)data);
+	data += sizeof(struct chcr_tls_stats) / sizeof(u64);
+#endif
 
 	*data++ = (u64)pi->port_id;
 	memset(&s, 0, sizeof(s));
-- 
2.18.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH net-next] cxgb4/chcr: nic-tls stats in ethtool
  2020-03-21 11:23 [PATCH net-next] cxgb4/chcr: nic-tls stats in ethtool Rohit Maheshwari
@ 2020-03-22 20:57 ` Jakub Kicinski
  2020-03-31 14:26   ` rohit maheshwari
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Kicinski @ 2020-03-22 20:57 UTC (permalink / raw)
  To: Rohit Maheshwari; +Cc: davem, netdev, borisp, secdev

On Sat, 21 Mar 2020 16:53:36 +0530 Rohit Maheshwari wrote:
> Included nic tls statistics in ethtool stats.
> 
> Signed-off-by: Rohit Maheshwari <rohitm@chelsio.com>
> ---
>  .../ethernet/chelsio/cxgb4/cxgb4_ethtool.c    | 57 +++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c
> index 398ade42476c..4998f1d1e218 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c
> @@ -134,6 +134,28 @@ static char loopback_stats_strings[][ETH_GSTRING_LEN] = {
>  	"bg3_frames_trunc       ",
>  };
>  
> +#ifdef CONFIG_CHELSIO_TLS_DEVICE
> +struct chcr_tls_stats {
> +	u64 tx_tls_encrypted_packets;
> +	u64 tx_tls_encrypted_bytes;
> +	u64 tx_tls_ctx;
> +	u64 tx_tls_ooo;
> +	u64 tx_tls_skip_no_sync_data;
> +	u64 tx_tls_drop_no_sync_data;
> +	u64 tx_tls_drop_bypass_req;

I don't understand why you need to have a structure for this, and then
you memset it to 0, unnecessarily, but I guess that's just a matter of
taste.

> +};
> +
> +static char chcr_tls_stats_strings[][ETH_GSTRING_LEN] = {
> +	"tx_tls_encrypted_pkts  ",
> +	"tx_tls_encrypted_bytes ",
> +	"tx_tls_ctx             ",
> +	"tx_tls_ooo             ",
> +	"tx_tls_skip_nosync_data",
> +	"tx_tls_drop_nosync_data",
> +	"tx_tls_drop_bypass_req ",

These, however, are not correct - please remove the spaces at the end,
otherwise your names are different than for other vendors. And there is
an underscore in the middle of "no_sync".

When you're told to adhere to API recommendation, please adhere to it
exactly.

> +};
> +#endif
> +
>  static const char cxgb4_priv_flags_strings[][ETH_GSTRING_LEN] = {
>  	[PRIV_FLAG_PORT_TX_VM_BIT] = "port_tx_vm_wr",
>  };
> @@ -144,6 +166,9 @@ static int get_sset_count(struct net_device *dev, int sset)
>  	case ETH_SS_STATS:
>  		return ARRAY_SIZE(stats_strings) +
>  		       ARRAY_SIZE(adapter_stats_strings) +
> +#ifdef CONFIG_CHELSIO_TLS_DEVICE
> +		       ARRAY_SIZE(chcr_tls_stats_strings) +
> +#endif
>  		       ARRAY_SIZE(loopback_stats_strings);
>  	case ETH_SS_PRIV_FLAGS:
>  		return ARRAY_SIZE(cxgb4_priv_flags_strings);
> @@ -204,6 +229,11 @@ static void get_strings(struct net_device *dev, u32 stringset, u8 *data)
>  		memcpy(data, adapter_stats_strings,
>  		       sizeof(adapter_stats_strings));
>  		data += sizeof(adapter_stats_strings);
> +#ifdef CONFIG_CHELSIO_TLS_DEVICE
> +		memcpy(data, chcr_tls_stats_strings,
> +		       sizeof(chcr_tls_stats_strings));
> +		data += sizeof(chcr_tls_stats_strings);
> +#endif
>  		memcpy(data, loopback_stats_strings,
>  		       sizeof(loopback_stats_strings));
>  	} else if (stringset == ETH_SS_PRIV_FLAGS) {
> @@ -289,6 +319,29 @@ static void collect_adapter_stats(struct adapter *adap, struct adapter_stats *s)
>  	}
>  }
>  
> +#ifdef CONFIG_CHELSIO_TLS_DEVICE
> +static void collect_chcr_tls_stats(struct adapter *adap,
> +				   struct chcr_tls_stats *s)
> +{
> +	struct chcr_stats_debug *stats = &adap->chcr_stats;
> +
> +	memset(s, 0, sizeof(*s));
> +
> +	s->tx_tls_encrypted_packets =
> +		atomic64_read(&stats->ktls_tx_encrypted_packets);
> +	s->tx_tls_encrypted_bytes =
> +		atomic64_read(&stats->ktls_tx_encrypted_bytes);
> +	s->tx_tls_ctx = atomic64_read(&stats->ktls_tx_ctx);
> +	s->tx_tls_ooo = atomic64_read(&stats->ktls_tx_ooo);
> +	s->tx_tls_skip_no_sync_data =
> +		atomic64_read(&stats->ktls_tx_skip_no_sync_data);
> +	s->tx_tls_drop_no_sync_data =
> +		atomic64_read(&stats->ktls_tx_drop_no_sync_data);
> +	s->tx_tls_drop_bypass_req =
> +		atomic64_read(&stats->ktls_tx_drop_bypass_req);
> +}
> +#endif
> +
>  static void get_stats(struct net_device *dev, struct ethtool_stats *stats,
>  		      u64 *data)
>  {
> @@ -307,6 +360,10 @@ static void get_stats(struct net_device *dev, struct ethtool_stats *stats,
>  	data += sizeof(struct queue_port_stats) / sizeof(u64);
>  	collect_adapter_stats(adapter, (struct adapter_stats *)data);
>  	data += sizeof(struct adapter_stats) / sizeof(u64);
> +#ifdef CONFIG_CHELSIO_TLS_DEVICE
> +	collect_chcr_tls_stats(adapter, (struct chcr_tls_stats *)data);
> +	data += sizeof(struct chcr_tls_stats) / sizeof(u64);
> +#endif
>  
>  	*data++ = (u64)pi->port_id;
>  	memset(&s, 0, sizeof(s));


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH net-next] cxgb4/chcr: nic-tls stats in ethtool
  2020-03-22 20:57 ` Jakub Kicinski
@ 2020-03-31 14:26   ` rohit maheshwari
  0 siblings, 0 replies; 3+ messages in thread
From: rohit maheshwari @ 2020-03-31 14:26 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, netdev, borisp, secdev


On 23/03/20 2:27 AM, Jakub Kicinski wrote:
> On Sat, 21 Mar 2020 16:53:36 +0530 Rohit Maheshwari wrote:
>> Included nic tls statistics in ethtool stats.
>>
>> Signed-off-by: Rohit Maheshwari <rohitm@chelsio.com>
>> ---
>>   .../ethernet/chelsio/cxgb4/cxgb4_ethtool.c    | 57 +++++++++++++++++++
>>   1 file changed, 57 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c
>> index 398ade42476c..4998f1d1e218 100644
>> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c
>> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c
>> @@ -134,6 +134,28 @@ static char loopback_stats_strings[][ETH_GSTRING_LEN] = {
>>   	"bg3_frames_trunc       ",
>>   };
>>   
>> +#ifdef CONFIG_CHELSIO_TLS_DEVICE
>> +struct chcr_tls_stats {
>> +	u64 tx_tls_encrypted_packets;
>> +	u64 tx_tls_encrypted_bytes;
>> +	u64 tx_tls_ctx;
>> +	u64 tx_tls_ooo;
>> +	u64 tx_tls_skip_no_sync_data;
>> +	u64 tx_tls_drop_no_sync_data;
>> +	u64 tx_tls_drop_bypass_req;
> I don't understand why you need to have a structure for this, and then
> you memset it to 0, unnecessarily, but I guess that's just a matter of
> taste.
Yeah, this memset is pure stupidity, I'll remove it and this new
structure as well. Thanks for the suggestion.
>> +};
>> +
>> +static char chcr_tls_stats_strings[][ETH_GSTRING_LEN] = {
>> +	"tx_tls_encrypted_pkts  ",
>> +	"tx_tls_encrypted_bytes ",
>> +	"tx_tls_ctx             ",
>> +	"tx_tls_ooo             ",
>> +	"tx_tls_skip_nosync_data",
>> +	"tx_tls_drop_nosync_data",
>> +	"tx_tls_drop_bypass_req ",
> These, however, are not correct - please remove the spaces at the end,
> otherwise your names are different than for other vendors. And there is
> an underscore in the middle of "no_sync".
>
> When you're told to adhere to API recommendation, please adhere to it
> exactly.
These spaces are used for alignment, so the statistics will be readable
for end user. As far as I understood, these are minimum set of TLS
related statistics, with or without space it will remain same for end
user. Please let me know if I am interpreting it wrong.
  However I agree about the no_sync, I'll change it in my
next patch.



^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-31 14:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-21 11:23 [PATCH net-next] cxgb4/chcr: nic-tls stats in ethtool Rohit Maheshwari
2020-03-22 20:57 ` Jakub Kicinski
2020-03-31 14:26   ` rohit maheshwari

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).