* [PATCH net-next 0/4] sfc: EF100 TSO enhancements
@ 2020-10-28 20:41 Edward Cree
2020-10-28 20:43 ` [PATCH net-next 1/4] sfc: extend bitfield macros to 17 fields Edward Cree
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Edward Cree @ 2020-10-28 20:41 UTC (permalink / raw)
To: linux-net-drivers, kuba; +Cc: davem, netdev
Support TSO over encapsulation (with GSO_PARTIAL), and over VLANs
(which the code already handled but we didn't advertise). Also
correct our handling of IPID mangling.
I couldn't find documentation of exactly what shaped SKBs we can
get given, so patch #2 is slightly guesswork, but when I tested
TSO over both underlay and (VxLAN) overlay, the checksums came
out correctly, so at least in those cases the edits we're making
must be the right ones.
Similarly, I'm not 100% sure I've correctly understood how FIXEDID
and MANGLEID are supposed to work in patch #3.
Edward Cree (4):
sfc: extend bitfield macros to 17 fields
sfc: implement encap TSO on EF100
sfc: only use fixed-id if the skb asks for it
sfc: advertise our vlan features
drivers/net/ethernet/sfc/bitfield.h | 42 +++++++++++++++++---
drivers/net/ethernet/sfc/ef100_nic.c | 17 ++++++--
drivers/net/ethernet/sfc/ef100_tx.c | 58 ++++++++++++++++------------
3 files changed, 84 insertions(+), 33 deletions(-)
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next 1/4] sfc: extend bitfield macros to 17 fields
2020-10-28 20:41 [PATCH net-next 0/4] sfc: EF100 TSO enhancements Edward Cree
@ 2020-10-28 20:43 ` Edward Cree
2020-10-28 20:43 ` [PATCH net-next 2/4] sfc: implement encap TSO on EF100 Edward Cree
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Edward Cree @ 2020-10-28 20:43 UTC (permalink / raw)
To: linux-net-drivers, kuba; +Cc: davem, netdev
We need EFX_POPULATE_OWORD_17 for an encap TSO descriptor on EF100.
Signed-off-by: Edward Cree <ecree@solarflare.com>
---
drivers/net/ethernet/sfc/bitfield.h | 42 +++++++++++++++++++++++++----
1 file changed, 37 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/sfc/bitfield.h b/drivers/net/ethernet/sfc/bitfield.h
index 2590cab53e54..64731eb5dd56 100644
--- a/drivers/net/ethernet/sfc/bitfield.h
+++ b/drivers/net/ethernet/sfc/bitfield.h
@@ -285,7 +285,11 @@ typedef union efx_oword {
field10, value10, \
field11, value11, \
field12, value12, \
- field13, value13) \
+ field13, value13, \
+ field14, value14, \
+ field15, value15, \
+ field16, value16, \
+ field17, value17) \
(EFX_INSERT_FIELD_NATIVE((min), (max), field1, (value1)) | \
EFX_INSERT_FIELD_NATIVE((min), (max), field2, (value2)) | \
EFX_INSERT_FIELD_NATIVE((min), (max), field3, (value3)) | \
@@ -298,7 +302,11 @@ typedef union efx_oword {
EFX_INSERT_FIELD_NATIVE((min), (max), field10, (value10)) | \
EFX_INSERT_FIELD_NATIVE((min), (max), field11, (value11)) | \
EFX_INSERT_FIELD_NATIVE((min), (max), field12, (value12)) | \
- EFX_INSERT_FIELD_NATIVE((min), (max), field13, (value13)))
+ EFX_INSERT_FIELD_NATIVE((min), (max), field13, (value13)) | \
+ EFX_INSERT_FIELD_NATIVE((min), (max), field14, (value14)) | \
+ EFX_INSERT_FIELD_NATIVE((min), (max), field15, (value15)) | \
+ EFX_INSERT_FIELD_NATIVE((min), (max), field16, (value16)) | \
+ EFX_INSERT_FIELD_NATIVE((min), (max), field17, (value17)))
#define EFX_INSERT_FIELDS64(...) \
cpu_to_le64(EFX_INSERT_FIELDS_NATIVE(__VA_ARGS__))
@@ -340,7 +348,15 @@ typedef union efx_oword {
#endif
/* Populate an octword field with various numbers of arguments */
-#define EFX_POPULATE_OWORD_13 EFX_POPULATE_OWORD
+#define EFX_POPULATE_OWORD_17 EFX_POPULATE_OWORD
+#define EFX_POPULATE_OWORD_16(oword, ...) \
+ EFX_POPULATE_OWORD_17(oword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
+#define EFX_POPULATE_OWORD_15(oword, ...) \
+ EFX_POPULATE_OWORD_16(oword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
+#define EFX_POPULATE_OWORD_14(oword, ...) \
+ EFX_POPULATE_OWORD_15(oword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
+#define EFX_POPULATE_OWORD_13(oword, ...) \
+ EFX_POPULATE_OWORD_14(oword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
#define EFX_POPULATE_OWORD_12(oword, ...) \
EFX_POPULATE_OWORD_13(oword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
#define EFX_POPULATE_OWORD_11(oword, ...) \
@@ -375,7 +391,15 @@ typedef union efx_oword {
EFX_DWORD_3, 0xffffffff)
/* Populate a quadword field with various numbers of arguments */
-#define EFX_POPULATE_QWORD_13 EFX_POPULATE_QWORD
+#define EFX_POPULATE_QWORD_17 EFX_POPULATE_QWORD
+#define EFX_POPULATE_QWORD_16(qword, ...) \
+ EFX_POPULATE_QWORD_17(qword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
+#define EFX_POPULATE_QWORD_15(qword, ...) \
+ EFX_POPULATE_QWORD_16(qword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
+#define EFX_POPULATE_QWORD_14(qword, ...) \
+ EFX_POPULATE_QWORD_15(qword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
+#define EFX_POPULATE_QWORD_13(qword, ...) \
+ EFX_POPULATE_QWORD_14(qword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
#define EFX_POPULATE_QWORD_12(qword, ...) \
EFX_POPULATE_QWORD_13(qword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
#define EFX_POPULATE_QWORD_11(qword, ...) \
@@ -408,7 +432,15 @@ typedef union efx_oword {
EFX_DWORD_1, 0xffffffff)
/* Populate a dword field with various numbers of arguments */
-#define EFX_POPULATE_DWORD_13 EFX_POPULATE_DWORD
+#define EFX_POPULATE_DWORD_17 EFX_POPULATE_DWORD
+#define EFX_POPULATE_DWORD_16(dword, ...) \
+ EFX_POPULATE_DWORD_17(dword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
+#define EFX_POPULATE_DWORD_15(dword, ...) \
+ EFX_POPULATE_DWORD_16(dword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
+#define EFX_POPULATE_DWORD_14(dword, ...) \
+ EFX_POPULATE_DWORD_15(dword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
+#define EFX_POPULATE_DWORD_13(dword, ...) \
+ EFX_POPULATE_DWORD_14(dword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
#define EFX_POPULATE_DWORD_12(dword, ...) \
EFX_POPULATE_DWORD_13(dword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
#define EFX_POPULATE_DWORD_11(dword, ...) \
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 2/4] sfc: implement encap TSO on EF100
2020-10-28 20:41 [PATCH net-next 0/4] sfc: EF100 TSO enhancements Edward Cree
2020-10-28 20:43 ` [PATCH net-next 1/4] sfc: extend bitfield macros to 17 fields Edward Cree
@ 2020-10-28 20:43 ` Edward Cree
2020-10-30 15:49 ` Willem de Bruijn
2020-10-28 20:43 ` [PATCH net-next 3/4] sfc: only use fixed-id if the skb asks for it Edward Cree
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Edward Cree @ 2020-10-28 20:43 UTC (permalink / raw)
To: linux-net-drivers, kuba; +Cc: davem, netdev
The NIC only needs to know where the headers it has to edit (TCP and
inner and outer IPv4) are, which fits GSO_PARTIAL nicely.
It also supports non-PARTIAL offload of UDP tunnels, again just
needing to be told the outer transport offset so that it can edit
the UDP length field.
(It's not clear to me whether the stack will ever use the non-PARTIAL
version with the netdev feature flags we're setting here.)
Signed-off-by: Edward Cree <ecree@solarflare.com>
---
drivers/net/ethernet/sfc/ef100_nic.c | 13 ++++++--
drivers/net/ethernet/sfc/ef100_tx.c | 45 ++++++++++++++++------------
2 files changed, 37 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/sfc/ef100_nic.c b/drivers/net/ethernet/sfc/ef100_nic.c
index 3148fe770356..bf92cdc60cda 100644
--- a/drivers/net/ethernet/sfc/ef100_nic.c
+++ b/drivers/net/ethernet/sfc/ef100_nic.c
@@ -182,8 +182,16 @@ static int efx_ef100_init_datapath_caps(struct efx_nic *efx)
if (rc)
return rc;
- if (efx_ef100_has_cap(nic_data->datapath_caps2, TX_TSO_V3))
- efx->net_dev->features |= NETIF_F_TSO | NETIF_F_TSO6;
+ if (efx_ef100_has_cap(nic_data->datapath_caps2, TX_TSO_V3)) {
+ struct net_device *net_dev = efx->net_dev;
+ netdev_features_t tso = NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_GSO_PARTIAL |
+ NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_UDP_TUNNEL_CSUM;
+
+ net_dev->features |= tso;
+ net_dev->hw_features |= tso;
+ net_dev->hw_enc_features |= tso;
+ net_dev->gso_partial_features |= NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_UDP_TUNNEL_CSUM;
+ }
efx->num_mac_stats = MCDI_WORD(outbuf,
GET_CAPABILITIES_V4_OUT_MAC_STATS_NUM_STATS);
netif_dbg(efx, probe, efx->net_dev,
@@ -1101,6 +1109,7 @@ static int ef100_probe_main(struct efx_nic *efx)
nic_data->efx = efx;
net_dev->features |= efx->type->offload_features;
net_dev->hw_features |= efx->type->offload_features;
+ net_dev->hw_enc_features |= efx->type->offload_features;
/* Populate design-parameter defaults */
nic_data->tso_max_hdr_len = ESE_EF100_DP_GZ_TSO_MAX_HDR_LEN_DEFAULT;
diff --git a/drivers/net/ethernet/sfc/ef100_tx.c b/drivers/net/ethernet/sfc/ef100_tx.c
index a90e5a9d2a37..d267b12bdaa0 100644
--- a/drivers/net/ethernet/sfc/ef100_tx.c
+++ b/drivers/net/ethernet/sfc/ef100_tx.c
@@ -54,8 +54,6 @@ static bool ef100_tx_can_tso(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
struct efx_nic *efx = tx_queue->efx;
struct ef100_nic_data *nic_data;
struct efx_tx_buffer *buffer;
- struct tcphdr *tcphdr;
- struct iphdr *iphdr;
size_t header_len;
u32 mss;
@@ -98,20 +96,6 @@ static bool ef100_tx_can_tso(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
buffer->unmap_len = 0;
buffer->skb = skb;
++tx_queue->insert_count;
-
- /* Adjust the TCP checksum to exclude the total length, since we set
- * ED_INNER_IP_LEN in the descriptor.
- */
- tcphdr = tcp_hdr(skb);
- if (skb_is_gso_v6(skb)) {
- tcphdr->check = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
- &ipv6_hdr(skb)->daddr,
- 0, IPPROTO_TCP, 0);
- } else {
- iphdr = ip_hdr(skb);
- tcphdr->check = ~csum_tcpudp_magic(iphdr->saddr, iphdr->daddr,
- 0, IPPROTO_TCP, 0);
- }
return true;
}
@@ -209,17 +193,35 @@ static void ef100_make_tso_desc(struct efx_nic *efx,
ESE_GZ_TX_DESC_IP4_ID_INC_MOD16;
u16 vlan_enable = efx->net_dev->features & NETIF_F_HW_VLAN_CTAG_TX ?
skb_vlan_tag_present(skb) : 0;
+ bool gso_partial = skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL;
unsigned int len, ip_offset, tcp_offset, payload_segs;
+ unsigned int outer_ip_offset, outer_l4_offset;
u16 vlan_tci = skb_vlan_tag_get(skb);
u32 mss = skb_shinfo(skb)->gso_size;
+ bool encap = skb->encapsulation;
+ struct tcphdr *tcp;
+ u32 paylen;
len = skb->len - buffer->len;
/* We use 1 for the TSO descriptor and 1 for the header */
payload_segs = segment_count - 2;
- ip_offset = skb_network_offset(skb);
- tcp_offset = skb_transport_offset(skb);
+ if (encap) {
+ outer_ip_offset = skb_network_offset(skb);
+ outer_l4_offset = skb_transport_offset(skb);
+ ip_offset = skb_inner_network_offset(skb);
+ tcp_offset = skb_inner_transport_offset(skb);
+ } else {
+ ip_offset = skb_network_offset(skb);
+ tcp_offset = skb_transport_offset(skb);
+ outer_ip_offset = outer_l4_offset = 0;
+ }
+
+ /* subtract TCP payload length from inner checksum */
+ tcp = (void *)skb->data + tcp_offset;
+ paylen = skb->len - tcp_offset;
+ csum_replace_by_diff(&tcp->check, (__force __wsum)htonl(paylen));
- EFX_POPULATE_OWORD_13(*txd,
+ EFX_POPULATE_OWORD_17(*txd,
ESF_GZ_TX_DESC_TYPE, ESE_GZ_TX_DESC_TYPE_TSO,
ESF_GZ_TX_TSO_MSS, mss,
ESF_GZ_TX_TSO_HDR_NUM_SEGS, 1,
@@ -231,6 +233,11 @@ static void ef100_make_tso_desc(struct efx_nic *efx,
ESF_GZ_TX_TSO_INNER_L4_OFF_W, tcp_offset >> 1,
ESF_GZ_TX_TSO_ED_INNER_IP4_ID, mangleid,
ESF_GZ_TX_TSO_ED_INNER_IP_LEN, 1,
+ ESF_GZ_TX_TSO_OUTER_L3_OFF_W, outer_ip_offset >> 1,
+ ESF_GZ_TX_TSO_OUTER_L4_OFF_W, outer_l4_offset >> 1,
+ ESF_GZ_TX_TSO_ED_OUTER_UDP_LEN, encap && !gso_partial,
+ ESF_GZ_TX_TSO_ED_OUTER_IP4_ID, encap ? mangleid :
+ ESE_GZ_TX_DESC_IP4_ID_NO_OP,
ESF_GZ_TX_TSO_VLAN_INSERT_EN, vlan_enable,
ESF_GZ_TX_TSO_VLAN_INSERT_TCI, vlan_tci
);
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 3/4] sfc: only use fixed-id if the skb asks for it
2020-10-28 20:41 [PATCH net-next 0/4] sfc: EF100 TSO enhancements Edward Cree
2020-10-28 20:43 ` [PATCH net-next 1/4] sfc: extend bitfield macros to 17 fields Edward Cree
2020-10-28 20:43 ` [PATCH net-next 2/4] sfc: implement encap TSO on EF100 Edward Cree
@ 2020-10-28 20:43 ` Edward Cree
2020-10-28 20:44 ` [PATCH net-next 4/4] sfc: advertise our vlan features Edward Cree
2020-10-31 0:45 ` [PATCH net-next 0/4] sfc: EF100 TSO enhancements Jakub Kicinski
4 siblings, 0 replies; 13+ messages in thread
From: Edward Cree @ 2020-10-28 20:43 UTC (permalink / raw)
To: linux-net-drivers, kuba; +Cc: davem, netdev
AIUI, the NETIF_F_TSO_MANGLEID flag is a signal to the stack that a
driver may _need_ to mangle IDs in order to do TSO, and conversely
a signal from the stack that the driver is permitted to do so.
Since we support both fixed and incrementing IPIDs, we should rely
on the SKB_GSO_FIXEDID flag on a per-skb basis, rather than using
the MANGLEID feature to make all TSOs fixed-id.
Includes other minor cleanups of ef100_make_tso_desc() coding style.
Signed-off-by: Edward Cree <ecree@solarflare.com>
---
drivers/net/ethernet/sfc/ef100_nic.c | 2 +-
drivers/net/ethernet/sfc/ef100_tx.c | 13 +++++++------
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/sfc/ef100_nic.c b/drivers/net/ethernet/sfc/ef100_nic.c
index bf92cdc60cda..8a187a16ac89 100644
--- a/drivers/net/ethernet/sfc/ef100_nic.c
+++ b/drivers/net/ethernet/sfc/ef100_nic.c
@@ -694,7 +694,7 @@ static unsigned int ef100_check_caps(const struct efx_nic *efx,
#define EF100_OFFLOAD_FEATURES (NETIF_F_HW_CSUM | NETIF_F_RXCSUM | \
NETIF_F_HIGHDMA | NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_NTUPLE | \
NETIF_F_RXHASH | NETIF_F_RXFCS | NETIF_F_TSO_ECN | NETIF_F_RXALL | \
- NETIF_F_TSO_MANGLEID | NETIF_F_HW_VLAN_CTAG_TX)
+ NETIF_F_HW_VLAN_CTAG_TX)
const struct efx_nic_type ef100_pf_nic_type = {
.revision = EFX_REV_EF100,
diff --git a/drivers/net/ethernet/sfc/ef100_tx.c b/drivers/net/ethernet/sfc/ef100_tx.c
index d267b12bdaa0..ad0ad9bad423 100644
--- a/drivers/net/ethernet/sfc/ef100_tx.c
+++ b/drivers/net/ethernet/sfc/ef100_tx.c
@@ -187,21 +187,22 @@ static void ef100_make_tso_desc(struct efx_nic *efx,
struct efx_tx_buffer *buffer, efx_oword_t *txd,
unsigned int segment_count)
{
- u32 mangleid = (efx->net_dev->features & NETIF_F_TSO_MANGLEID) ||
- skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID ?
- ESE_GZ_TX_DESC_IP4_ID_NO_OP :
- ESE_GZ_TX_DESC_IP4_ID_INC_MOD16;
- u16 vlan_enable = efx->net_dev->features & NETIF_F_HW_VLAN_CTAG_TX ?
- skb_vlan_tag_present(skb) : 0;
bool gso_partial = skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL;
unsigned int len, ip_offset, tcp_offset, payload_segs;
+ u32 mangleid = ESE_GZ_TX_DESC_IP4_ID_INC_MOD16;
unsigned int outer_ip_offset, outer_l4_offset;
u16 vlan_tci = skb_vlan_tag_get(skb);
u32 mss = skb_shinfo(skb)->gso_size;
bool encap = skb->encapsulation;
+ u16 vlan_enable = 0;
struct tcphdr *tcp;
u32 paylen;
+ if (skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID)
+ mangleid = ESE_GZ_TX_DESC_IP4_ID_NO_OP;
+ if (efx->net_dev->features & NETIF_F_HW_VLAN_CTAG_TX)
+ vlan_enable = skb_vlan_tag_present(skb);
+
len = skb->len - buffer->len;
/* We use 1 for the TSO descriptor and 1 for the header */
payload_segs = segment_count - 2;
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 4/4] sfc: advertise our vlan features
2020-10-28 20:41 [PATCH net-next 0/4] sfc: EF100 TSO enhancements Edward Cree
` (2 preceding siblings ...)
2020-10-28 20:43 ` [PATCH net-next 3/4] sfc: only use fixed-id if the skb asks for it Edward Cree
@ 2020-10-28 20:44 ` Edward Cree
2020-10-31 0:45 ` [PATCH net-next 0/4] sfc: EF100 TSO enhancements Jakub Kicinski
4 siblings, 0 replies; 13+ messages in thread
From: Edward Cree @ 2020-10-28 20:44 UTC (permalink / raw)
To: linux-net-drivers, kuba; +Cc: davem, netdev
Signed-off-by: Edward Cree <ecree@solarflare.com>
---
drivers/net/ethernet/sfc/ef100_nic.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/sfc/ef100_nic.c b/drivers/net/ethernet/sfc/ef100_nic.c
index 8a187a16ac89..cd93c5ffd45c 100644
--- a/drivers/net/ethernet/sfc/ef100_nic.c
+++ b/drivers/net/ethernet/sfc/ef100_nic.c
@@ -1110,6 +1110,8 @@ static int ef100_probe_main(struct efx_nic *efx)
net_dev->features |= efx->type->offload_features;
net_dev->hw_features |= efx->type->offload_features;
net_dev->hw_enc_features |= efx->type->offload_features;
+ net_dev->vlan_features |= NETIF_F_HW_CSUM | NETIF_F_SG |
+ NETIF_F_HIGHDMA | NETIF_F_ALL_TSO;
/* Populate design-parameter defaults */
nic_data->tso_max_hdr_len = ESE_EF100_DP_GZ_TSO_MAX_HDR_LEN_DEFAULT;
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 2/4] sfc: implement encap TSO on EF100
2020-10-28 20:43 ` [PATCH net-next 2/4] sfc: implement encap TSO on EF100 Edward Cree
@ 2020-10-30 15:49 ` Willem de Bruijn
2020-10-30 16:13 ` Edward Cree
0 siblings, 1 reply; 13+ messages in thread
From: Willem de Bruijn @ 2020-10-30 15:49 UTC (permalink / raw)
To: Edward Cree
Cc: linux-net-drivers, Jakub Kicinski, David Miller, Network Development
On Wed, Oct 28, 2020 at 9:39 PM Edward Cree <ecree@solarflare.com> wrote:
>
> The NIC only needs to know where the headers it has to edit (TCP and
> inner and outer IPv4) are, which fits GSO_PARTIAL nicely.
> It also supports non-PARTIAL offload of UDP tunnels, again just
> needing to be told the outer transport offset so that it can edit
> the UDP length field.
> (It's not clear to me whether the stack will ever use the non-PARTIAL
> version with the netdev feature flags we're setting here.)
>
> Signed-off-by: Edward Cree <ecree@solarflare.com>
> ---
> drivers/net/ethernet/sfc/ef100_nic.c | 13 ++++++--
> drivers/net/ethernet/sfc/ef100_tx.c | 45 ++++++++++++++++------------
> 2 files changed, 37 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/ethernet/sfc/ef100_nic.c b/drivers/net/ethernet/sfc/ef100_nic.c
> index 3148fe770356..bf92cdc60cda 100644
> --- a/drivers/net/ethernet/sfc/ef100_nic.c
> +++ b/drivers/net/ethernet/sfc/ef100_nic.c
> @@ -182,8 +182,16 @@ static int efx_ef100_init_datapath_caps(struct efx_nic *efx)
> if (rc)
> return rc;
>
> - if (efx_ef100_has_cap(nic_data->datapath_caps2, TX_TSO_V3))
> - efx->net_dev->features |= NETIF_F_TSO | NETIF_F_TSO6;
> + if (efx_ef100_has_cap(nic_data->datapath_caps2, TX_TSO_V3)) {
> + struct net_device *net_dev = efx->net_dev;
> + netdev_features_t tso = NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_GSO_PARTIAL |
> + NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_UDP_TUNNEL_CSUM;
> +
> + net_dev->features |= tso;
> + net_dev->hw_features |= tso;
> + net_dev->hw_enc_features |= tso;
> + net_dev->gso_partial_features |= NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_UDP_TUNNEL_CSUM;
> + }
> efx->num_mac_stats = MCDI_WORD(outbuf,
> GET_CAPABILITIES_V4_OUT_MAC_STATS_NUM_STATS);
> netif_dbg(efx, probe, efx->net_dev,
> @@ -1101,6 +1109,7 @@ static int ef100_probe_main(struct efx_nic *efx)
> nic_data->efx = efx;
> net_dev->features |= efx->type->offload_features;
> net_dev->hw_features |= efx->type->offload_features;
> + net_dev->hw_enc_features |= efx->type->offload_features;
>
> /* Populate design-parameter defaults */
> nic_data->tso_max_hdr_len = ESE_EF100_DP_GZ_TSO_MAX_HDR_LEN_DEFAULT;
> diff --git a/drivers/net/ethernet/sfc/ef100_tx.c b/drivers/net/ethernet/sfc/ef100_tx.c
> index a90e5a9d2a37..d267b12bdaa0 100644
> --- a/drivers/net/ethernet/sfc/ef100_tx.c
> +++ b/drivers/net/ethernet/sfc/ef100_tx.c
> @@ -54,8 +54,6 @@ static bool ef100_tx_can_tso(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
> struct efx_nic *efx = tx_queue->efx;
> struct ef100_nic_data *nic_data;
> struct efx_tx_buffer *buffer;
> - struct tcphdr *tcphdr;
> - struct iphdr *iphdr;
> size_t header_len;
> u32 mss;
>
> @@ -98,20 +96,6 @@ static bool ef100_tx_can_tso(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
> buffer->unmap_len = 0;
> buffer->skb = skb;
> ++tx_queue->insert_count;
> -
> - /* Adjust the TCP checksum to exclude the total length, since we set
> - * ED_INNER_IP_LEN in the descriptor.
> - */
> - tcphdr = tcp_hdr(skb);
> - if (skb_is_gso_v6(skb)) {
> - tcphdr->check = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
> - &ipv6_hdr(skb)->daddr,
> - 0, IPPROTO_TCP, 0);
> - } else {
> - iphdr = ip_hdr(skb);
> - tcphdr->check = ~csum_tcpudp_magic(iphdr->saddr, iphdr->daddr,
> - 0, IPPROTO_TCP, 0);
> - }
> return true;
> }
>
> @@ -209,17 +193,35 @@ static void ef100_make_tso_desc(struct efx_nic *efx,
> ESE_GZ_TX_DESC_IP4_ID_INC_MOD16;
> u16 vlan_enable = efx->net_dev->features & NETIF_F_HW_VLAN_CTAG_TX ?
> skb_vlan_tag_present(skb) : 0;
> + bool gso_partial = skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL;
> unsigned int len, ip_offset, tcp_offset, payload_segs;
> + unsigned int outer_ip_offset, outer_l4_offset;
> u16 vlan_tci = skb_vlan_tag_get(skb);
> u32 mss = skb_shinfo(skb)->gso_size;
> + bool encap = skb->encapsulation;
> + struct tcphdr *tcp;
> + u32 paylen;
>
> len = skb->len - buffer->len;
> /* We use 1 for the TSO descriptor and 1 for the header */
> payload_segs = segment_count - 2;
> - ip_offset = skb_network_offset(skb);
> - tcp_offset = skb_transport_offset(skb);
> + if (encap) {
> + outer_ip_offset = skb_network_offset(skb);
> + outer_l4_offset = skb_transport_offset(skb);
> + ip_offset = skb_inner_network_offset(skb);
> + tcp_offset = skb_inner_transport_offset(skb);
> + } else {
> + ip_offset = skb_network_offset(skb);
> + tcp_offset = skb_transport_offset(skb);
> + outer_ip_offset = outer_l4_offset = 0;
> + }
> +
> + /* subtract TCP payload length from inner checksum */
> + tcp = (void *)skb->data + tcp_offset;
> + paylen = skb->len - tcp_offset;
> + csum_replace_by_diff(&tcp->check, (__force __wsum)htonl(paylen));
>
> - EFX_POPULATE_OWORD_13(*txd,
> + EFX_POPULATE_OWORD_17(*txd,
> ESF_GZ_TX_DESC_TYPE, ESE_GZ_TX_DESC_TYPE_TSO,
> ESF_GZ_TX_TSO_MSS, mss,
> ESF_GZ_TX_TSO_HDR_NUM_SEGS, 1,
> @@ -231,6 +233,11 @@ static void ef100_make_tso_desc(struct efx_nic *efx,
> ESF_GZ_TX_TSO_INNER_L4_OFF_W, tcp_offset >> 1,
> ESF_GZ_TX_TSO_ED_INNER_IP4_ID, mangleid,
> ESF_GZ_TX_TSO_ED_INNER_IP_LEN, 1,
> + ESF_GZ_TX_TSO_OUTER_L3_OFF_W, outer_ip_offset >> 1,
> + ESF_GZ_TX_TSO_OUTER_L4_OFF_W, outer_l4_offset >> 1,
> + ESF_GZ_TX_TSO_ED_OUTER_UDP_LEN, encap && !gso_partial,
This is a boolean field to signal whether the NIC needs to fix up the
udp length field ?
Which in the case of GSO_PARTIAL has already been resolved by the gso
layer (in __skb_udp_tunnel_segment).
Just curious, is this ever expected to be true? Not based on current
advertised features, right?
> + ESF_GZ_TX_TSO_ED_OUTER_IP4_ID, encap ? mangleid :
> + ESE_GZ_TX_DESC_IP4_ID_NO_OP,
> ESF_GZ_TX_TSO_VLAN_INSERT_EN, vlan_enable,
> ESF_GZ_TX_TSO_VLAN_INSERT_TCI, vlan_tci
> );
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 2/4] sfc: implement encap TSO on EF100
2020-10-30 15:49 ` Willem de Bruijn
@ 2020-10-30 16:13 ` Edward Cree
2020-10-30 16:26 ` Willem de Bruijn
0 siblings, 1 reply; 13+ messages in thread
From: Edward Cree @ 2020-10-30 16:13 UTC (permalink / raw)
To: Willem de Bruijn
Cc: linux-net-drivers, Jakub Kicinski, David Miller, Network Development
On 30/10/2020 15:49, Willem de Bruijn wrote:
> On Wed, Oct 28, 2020 at 9:39 PM Edward Cree <ecree@solarflare.com> wrote:
>> + ESF_GZ_TX_TSO_ED_OUTER_UDP_LEN, encap && !gso_partial,
>
> This is a boolean field to signal whether the NIC needs to fix up the
> udp length field ?
Yes.
> Which in the case of GSO_PARTIAL has already been resolved by the gso
> layer (in __skb_udp_tunnel_segment).
Indeed.
> Just curious, is this ever expected to be true? Not based on current
> advertised features, right?
As I mentioned in the patch description and cover letter, I'm not
entirely certain. I don't _think_ the stack will ever give us an
encap skb without GSO_PARTIAL with the features we've advertised,
but since the hardware supports it I thought it better to handle
that case anyway, just in case I'm mistaken.
-ed
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 2/4] sfc: implement encap TSO on EF100
2020-10-30 16:13 ` Edward Cree
@ 2020-10-30 16:26 ` Willem de Bruijn
2020-10-30 16:42 ` Edward Cree
0 siblings, 1 reply; 13+ messages in thread
From: Willem de Bruijn @ 2020-10-30 16:26 UTC (permalink / raw)
To: Edward Cree
Cc: Willem de Bruijn, linux-net-drivers, Jakub Kicinski,
David Miller, Network Development
On Fri, Oct 30, 2020 at 12:16 PM Edward Cree <ecree@solarflare.com> wrote:
>
> On 30/10/2020 15:49, Willem de Bruijn wrote:
> > On Wed, Oct 28, 2020 at 9:39 PM Edward Cree <ecree@solarflare.com> wrote:
> >> + ESF_GZ_TX_TSO_ED_OUTER_UDP_LEN, encap && !gso_partial,
> >
> > This is a boolean field to signal whether the NIC needs to fix up the
> > udp length field ?
> Yes.
Thanks
> > Which in the case of GSO_PARTIAL has already been resolved by the gso
> > layer (in __skb_udp_tunnel_segment).
> Indeed.
>
> > Just curious, is this ever expected to be true? Not based on current
> > advertised features, right?
> As I mentioned in the patch description and cover letter, I'm not
> entirely certain. I don't _think_ the stack will ever give us an
> encap skb without GSO_PARTIAL with the features we've advertised,
That's my understanding too.
> but since the hardware supports it I thought it better to handle
> that case anyway, just in case I'm mistaken.
Then you could (as follow-up) advertise without GSO_PARTIAL and avoid
the whole transition through the gso layer?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 2/4] sfc: implement encap TSO on EF100
2020-10-30 16:26 ` Willem de Bruijn
@ 2020-10-30 16:42 ` Edward Cree
2020-10-30 17:33 ` Willem de Bruijn
0 siblings, 1 reply; 13+ messages in thread
From: Edward Cree @ 2020-10-30 16:42 UTC (permalink / raw)
To: Willem de Bruijn
Cc: linux-net-drivers, Jakub Kicinski, David Miller, Network Development
On 30/10/2020 16:26, Willem de Bruijn wrote:
> Then you could (as follow-up) advertise without GSO_PARTIAL and avoid
> the whole transition through the gso layer?
The thing is, non-PARTIAL offload only supports tunnels that the NIC
understands (single-layer UDP tunnels), but AIUI GSO_PARTIAL can
support all sorts of other things, such as gretaps (though we'd need
some more advertised features, I haven't figured out quite which
ones yet but when I do and can test it I'll send a follow-up) and
nested tunnels (as long as we don't need to edit the 'middle' IP ID,
e.g. if it's DF or IPv6). So we definitely want to advertise
GSO_PARTIAL support.
But possibly I don't need to have NETIF_F_GSO_UDP_TUNNEL[_CSUM] in
net_dev->gso_partial_features?
-ed
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 2/4] sfc: implement encap TSO on EF100
2020-10-30 16:42 ` Edward Cree
@ 2020-10-30 17:33 ` Willem de Bruijn
2020-10-30 18:14 ` Edward Cree
0 siblings, 1 reply; 13+ messages in thread
From: Willem de Bruijn @ 2020-10-30 17:33 UTC (permalink / raw)
To: Edward Cree
Cc: Willem de Bruijn, linux-net-drivers, Jakub Kicinski,
David Miller, Network Development
On Fri, Oct 30, 2020 at 12:43 PM Edward Cree <ecree@solarflare.com> wrote:
>
> On 30/10/2020 16:26, Willem de Bruijn wrote:
> > Then you could (as follow-up) advertise without GSO_PARTIAL and avoid
> > the whole transition through the gso layer?
>
> The thing is, non-PARTIAL offload only supports tunnels that the NIC
> understands (single-layer UDP tunnels), but AIUI GSO_PARTIAL can
> support all sorts of other things, such as gretaps (though we'd need
> some more advertised features, I haven't figured out quite which
> ones yet but when I do and can test it I'll send a follow-up) and
> nested tunnels (as long as we don't need to edit the 'middle' IP ID,
> e.g. if it's DF or IPv6). So we definitely want to advertise
> GSO_PARTIAL support.
> But possibly I don't need to have NETIF_F_GSO_UDP_TUNNEL[_CSUM] in
> net_dev->gso_partial_features?
If the device can handle fixing the odd last segment length, indeed.
If you remove them from gso_partial_flags, then gso_features_check
won't mask them out
/* Support for GSO partial features requires software
* intervention before we can actually process the packets
* so we need to strip support for any partial features now
* and we can pull them back in after we have partially
* segmented the frame.
*/
if (!(skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL))
features &= ~dev->gso_partial_features;
as called in validate_xmit_skb prior to testing whether the packet can
be passed to the nic as is in netif_needs_gso.
Until adding other tunnel types like NETIF_F_GSO_GRE_CSUM, for this
device gso_partial_features would then be 0 and thus
NETIF_F_GSO_PARTIAL is not needed at all?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 2/4] sfc: implement encap TSO on EF100
2020-10-30 17:33 ` Willem de Bruijn
@ 2020-10-30 18:14 ` Edward Cree
2020-10-30 20:55 ` Willem de Bruijn
0 siblings, 1 reply; 13+ messages in thread
From: Edward Cree @ 2020-10-30 18:14 UTC (permalink / raw)
To: Willem de Bruijn
Cc: linux-net-drivers, Jakub Kicinski, David Miller, Network Development
On 30/10/2020 17:33, Willem de Bruijn wrote:
> On Fri, Oct 30, 2020 at 12:43 PM Edward Cree <ecree@solarflare.com> wrote:
>> But possibly I don't need to have NETIF_F_GSO_UDP_TUNNEL[_CSUM] in
>> net_dev->gso_partial_features?
> If the device can handle fixing the odd last segment length, indeed.
It can, but...
I thought Linux didn't give drivers odd last segments any more. At
least I'm fairly sure that in the (non-PARTIAL) non-encap TSO tests
I've done, the GSO skbs we've been given have all been a whole
number of mss long.
Which means I haven't been able to test that the device gets it right.
> Until adding other tunnel types like NETIF_F_GSO_GRE_CSUM, for this
> device gso_partial_features would then be 0 and thus
> NETIF_F_GSO_PARTIAL is not needed at all?
Unless the kernel supports GSO_PARTIAL of nested tunnels. The device
will handle (say) VxLAN-in-VxLAN just fine, as long as you don't want
it to update the middle IPID; and being able to cope with crazy
things like that was (I thought) part of the point of GSO_PARTIAL.
Indeed, given that GSO_PARTIAL is supposed to be a non-protocol-
ossified design, it seems a bit silly to me that we have to specify
a list of other NETIF_F_GSO_foo, rather than just being able to say
"I can handle anything of the form ETH-IP-gubbins-IP-TCP" and let
the kernel put whatever it likes — VxLAN, GRE, fou-over-geneve with
cherry and sprinkles — in the 'gubbins'. Wasn't that what 'less is
more' was supposed to be all about?
-ed
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 2/4] sfc: implement encap TSO on EF100
2020-10-30 18:14 ` Edward Cree
@ 2020-10-30 20:55 ` Willem de Bruijn
0 siblings, 0 replies; 13+ messages in thread
From: Willem de Bruijn @ 2020-10-30 20:55 UTC (permalink / raw)
To: Edward Cree
Cc: Willem de Bruijn, linux-net-drivers, Jakub Kicinski,
David Miller, Network Development
On Fri, Oct 30, 2020 at 2:14 PM Edward Cree <ecree@solarflare.com> wrote:
>
> On 30/10/2020 17:33, Willem de Bruijn wrote:
> > On Fri, Oct 30, 2020 at 12:43 PM Edward Cree <ecree@solarflare.com> wrote:
> >> But possibly I don't need to have NETIF_F_GSO_UDP_TUNNEL[_CSUM] in
> >> net_dev->gso_partial_features?
> > If the device can handle fixing the odd last segment length, indeed.
> It can, but...
> I thought Linux didn't give drivers odd last segments any more. At
> least I'm fairly sure that in the (non-PARTIAL) non-encap TSO tests
> I've done, the GSO skbs we've been given have all been a whole
> number of mss long.
> Which means I haven't been able to test that the device gets it right.
Perhaps the local TCP stack tries to avoid it. Off the top of my head
not sure if this is assured in all edge cases.
The forwarding path is another wildcard.
> > Until adding other tunnel types like NETIF_F_GSO_GRE_CSUM, for this
> > device gso_partial_features would then be 0 and thus
> > NETIF_F_GSO_PARTIAL is not needed at all?
> Unless the kernel supports GSO_PARTIAL of nested tunnels. The device
> will handle (say) VxLAN-in-VxLAN just fine, as long as you don't want
> it to update the middle IPID; and being able to cope with crazy
> things like that was (I thought) part of the point of GSO_PARTIAL.
Oh right.
> Indeed, given that GSO_PARTIAL is supposed to be a non-protocol-
> ossified design, it seems a bit silly to me that we have to specify
> a list of other NETIF_F_GSO_foo, rather than just being able to say
> "I can handle anything of the form ETH-IP-gubbins-IP-TCP" and let
> the kernel put whatever it likes — VxLAN, GRE, fou-over-geneve with
> cherry and sprinkles — in the 'gubbins'. Wasn't that what 'less is
> more' was supposed to be all about?
>
> -ed
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 0/4] sfc: EF100 TSO enhancements
2020-10-28 20:41 [PATCH net-next 0/4] sfc: EF100 TSO enhancements Edward Cree
` (3 preceding siblings ...)
2020-10-28 20:44 ` [PATCH net-next 4/4] sfc: advertise our vlan features Edward Cree
@ 2020-10-31 0:45 ` Jakub Kicinski
4 siblings, 0 replies; 13+ messages in thread
From: Jakub Kicinski @ 2020-10-31 0:45 UTC (permalink / raw)
To: Edward Cree; +Cc: linux-net-drivers, davem, netdev
On Wed, 28 Oct 2020 20:41:21 +0000 Edward Cree wrote:
> Support TSO over encapsulation (with GSO_PARTIAL), and over VLANs
> (which the code already handled but we didn't advertise). Also
> correct our handling of IPID mangling.
>
> I couldn't find documentation of exactly what shaped SKBs we can
> get given, so patch #2 is slightly guesswork, but when I tested
> TSO over both underlay and (VxLAN) overlay, the checksums came
> out correctly, so at least in those cases the edits we're making
> must be the right ones.
> Similarly, I'm not 100% sure I've correctly understood how FIXEDID
> and MANGLEID are supposed to work in patch #3.
Applied, thanks everyone!
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-10-31 0:45 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-28 20:41 [PATCH net-next 0/4] sfc: EF100 TSO enhancements Edward Cree
2020-10-28 20:43 ` [PATCH net-next 1/4] sfc: extend bitfield macros to 17 fields Edward Cree
2020-10-28 20:43 ` [PATCH net-next 2/4] sfc: implement encap TSO on EF100 Edward Cree
2020-10-30 15:49 ` Willem de Bruijn
2020-10-30 16:13 ` Edward Cree
2020-10-30 16:26 ` Willem de Bruijn
2020-10-30 16:42 ` Edward Cree
2020-10-30 17:33 ` Willem de Bruijn
2020-10-30 18:14 ` Edward Cree
2020-10-30 20:55 ` Willem de Bruijn
2020-10-28 20:43 ` [PATCH net-next 3/4] sfc: only use fixed-id if the skb asks for it Edward Cree
2020-10-28 20:44 ` [PATCH net-next 4/4] sfc: advertise our vlan features Edward Cree
2020-10-31 0:45 ` [PATCH net-next 0/4] sfc: EF100 TSO enhancements Jakub Kicinski
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).