* [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).