| Message ID | 20160419190615.11723.53966.stgit@ahduyck-xeon-server |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Apr 19, 2016 at 12:06 PM, Alexander Duyck <aduyck@mirantis.com> wrote: > This patch assumes that the bnxt hardware will ignore existing IPv4/v6 > header fields for length and checksum as well as the length and checksum > fields for outer UDP and GRE headers. > > I have no means of testing this as I do not have any bnx2x hardware but > thought I would submit it as an RFC to see if anyone out there wants to > test this and see if this does in fact enable this functionality allowing > us to to segment tunneled frames that have an outer checksum. > > Signed-off-by: Alexander Duyck <aduyck@mirantis.com> Hi Alex, I just did a very quick test of this patch on our bnxt hardware and it seemed to work. I created a vxlan endpoint with udpcsum enabled and I saw TSO packets getting through. I've verified that our hardware can be programmed to either ignore outer UDP checksum or to calculate it. Current default is to ignore ipv4 UDP checksum and calculate ipv6 UDP checksum. Thanks. > --- > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) >
On Tue, Apr 26, 2016 at 10:55 PM, Michael Chan <michael.chan@broadcom.com> wrote: > On Tue, Apr 19, 2016 at 12:06 PM, Alexander Duyck <aduyck@mirantis.com> wrote: >> This patch assumes that the bnxt hardware will ignore existing IPv4/v6 >> header fields for length and checksum as well as the length and checksum >> fields for outer UDP and GRE headers. >> >> I have no means of testing this as I do not have any bnx2x hardware but >> thought I would submit it as an RFC to see if anyone out there wants to >> test this and see if this does in fact enable this functionality allowing >> us to to segment tunneled frames that have an outer checksum. >> >> Signed-off-by: Alexander Duyck <aduyck@mirantis.com> > > Hi Alex, I just did a very quick test of this patch on our bnxt > hardware and it seemed to work. > > I created a vxlan endpoint with udpcsum enabled and I saw TSO packets > getting through. I've verified that our hardware can be programmed to > either ignore outer UDP checksum or to calculate it. Current default > is to ignore ipv4 UDP checksum and calculate ipv6 UDP checksum. > Thanks. Are you saying you can natively support UDP tunnel with outer checksum offload then? I'm just trying to sort out if you actually need to have the partial segmentation offload support or if we can handle it in hardware. Also is there any documentation you could point me to that might help to clarify what the hardware does/doesn't support so that I could improve upon this patch in order to make sure we are getting the most bang for the buck in terms of the features that can be offloaded by hardware? Thanks. - Alex
On Wed, Apr 27, 2016 at 8:21 AM, Alexander Duyck <alexander.duyck@gmail.com> wrote: > On Tue, Apr 26, 2016 at 10:55 PM, Michael Chan > <michael.chan@broadcom.com> wrote: >> On Tue, Apr 19, 2016 at 12:06 PM, Alexander Duyck <aduyck@mirantis.com> wrote: >>> This patch assumes that the bnxt hardware will ignore existing IPv4/v6 >>> header fields for length and checksum as well as the length and checksum >>> fields for outer UDP and GRE headers. >>> >>> I have no means of testing this as I do not have any bnx2x hardware but >>> thought I would submit it as an RFC to see if anyone out there wants to >>> test this and see if this does in fact enable this functionality allowing >>> us to to segment tunneled frames that have an outer checksum. >>> >>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com> >> >> Hi Alex, I just did a very quick test of this patch on our bnxt >> hardware and it seemed to work. >> >> I created a vxlan endpoint with udpcsum enabled and I saw TSO packets >> getting through. I've verified that our hardware can be programmed to >> either ignore outer UDP checksum or to calculate it. Current default >> is to ignore ipv4 UDP checksum and calculate ipv6 UDP checksum. >> Thanks. > > Are you saying you can natively support UDP tunnel with outer checksum > offload then? Yes. Calculate or ignore the outer UDP checksum. > > I'm just trying to sort out if you actually need to have the partial > segmentation offload support or if we can handle it in hardware. Also > is there any documentation you could point me to that might help to > clarify what the hardware does/doesn't support so that I could improve > upon this patch in order to make sure we are getting the most bang for > the buck in terms of the features that can be offloaded by hardware? No public documentation yet. I think the plan is to publish the programmer's reference on our website at some point in the future.
On Wed, Apr 27, 2016 at 9:32 PM, Michael Chan <michael.chan@broadcom.com> wrote: > On Wed, Apr 27, 2016 at 8:21 AM, Alexander Duyck > <alexander.duyck@gmail.com> wrote: >> On Tue, Apr 26, 2016 at 10:55 PM, Michael Chan >> <michael.chan@broadcom.com> wrote: >>> On Tue, Apr 19, 2016 at 12:06 PM, Alexander Duyck <aduyck@mirantis.com> wrote: >>>> This patch assumes that the bnxt hardware will ignore existing IPv4/v6 >>>> header fields for length and checksum as well as the length and checksum >>>> fields for outer UDP and GRE headers. >>>> >>>> I have no means of testing this as I do not have any bnx2x hardware but >>>> thought I would submit it as an RFC to see if anyone out there wants to >>>> test this and see if this does in fact enable this functionality allowing >>>> us to to segment tunneled frames that have an outer checksum. >>>> >>>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com> >>> >>> Hi Alex, I just did a very quick test of this patch on our bnxt >>> hardware and it seemed to work. >>> >>> I created a vxlan endpoint with udpcsum enabled and I saw TSO packets >>> getting through. I've verified that our hardware can be programmed to >>> either ignore outer UDP checksum or to calculate it. Current default >>> is to ignore ipv4 UDP checksum and calculate ipv6 UDP checksum. >>> Thanks. >> >> Are you saying you can natively support UDP tunnel with outer checksum >> offload then? > > Yes. Calculate or ignore the outer UDP checksum. I was just thinking about this. When you say you compute the IPv6 checksum how is it you are specifying to the hardware that you want to do that? Is it something you can configure per packet or is it something that is configured for the VXLAN flow? I just want to make sure you aren't adding checksums to IPv6 tunnels that specify that they don't want a checksum, or stripping them from v4 tunnels that do want a checksum. Thanks. - Alex
On Fri, Apr 29, 2016 at 2:17 PM, Alexander Duyck <alexander.duyck@gmail.com> wrote: > On Wed, Apr 27, 2016 at 9:32 PM, Michael Chan <michael.chan@broadcom.com> wrote: >> On Wed, Apr 27, 2016 at 8:21 AM, Alexander Duyck >> <alexander.duyck@gmail.com> wrote: >>> On Tue, Apr 26, 2016 at 10:55 PM, Michael Chan >>> <michael.chan@broadcom.com> wrote: >>>> On Tue, Apr 19, 2016 at 12:06 PM, Alexander Duyck <aduyck@mirantis.com> wrote: >>>>> This patch assumes that the bnxt hardware will ignore existing IPv4/v6 >>>>> header fields for length and checksum as well as the length and checksum >>>>> fields for outer UDP and GRE headers. >>>>> >>>>> I have no means of testing this as I do not have any bnx2x hardware but >>>>> thought I would submit it as an RFC to see if anyone out there wants to >>>>> test this and see if this does in fact enable this functionality allowing >>>>> us to to segment tunneled frames that have an outer checksum. >>>>> >>>>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com> >>>> >>>> Hi Alex, I just did a very quick test of this patch on our bnxt >>>> hardware and it seemed to work. >>>> >>>> I created a vxlan endpoint with udpcsum enabled and I saw TSO packets >>>> getting through. I've verified that our hardware can be programmed to >>>> either ignore outer UDP checksum or to calculate it. Current default >>>> is to ignore ipv4 UDP checksum and calculate ipv6 UDP checksum. >>>> Thanks. >>> >>> Are you saying you can natively support UDP tunnel with outer checksum >>> offload then? >> >> Yes. Calculate or ignore the outer UDP checksum. > > I was just thinking about this. When you say you compute the IPv6 > checksum how is it you are specifying to the hardware that you want to > do that? Is it something you can configure per packet or is it > something that is configured for the VXLAN flow? In the current version of the hardware, it is a global (chip-wide) setting. 1 bit to control outer ipv4 vxlan and 1 bit for outer ipv6 vxlan. > > I just want to make sure you aren't adding checksums to IPv6 tunnels > that specify that they don't want a checksum, or stripping them from > v4 tunnels that do want a checksum. If the global setting has outer UDP checksum enabled, it will be calculated no matter what. If the setting is disabled, it will just ignore it without overwriting it. Thanks.
On Fri, Apr 29, 2016 at 2:29 PM, Michael Chan <michael.chan@broadcom.com> wrote: > On Fri, Apr 29, 2016 at 2:17 PM, Alexander Duyck > <alexander.duyck@gmail.com> wrote: >> On Wed, Apr 27, 2016 at 9:32 PM, Michael Chan <michael.chan@broadcom.com> wrote: >>> On Wed, Apr 27, 2016 at 8:21 AM, Alexander Duyck >>> <alexander.duyck@gmail.com> wrote: >>>> On Tue, Apr 26, 2016 at 10:55 PM, Michael Chan >>>> <michael.chan@broadcom.com> wrote: >>>>> On Tue, Apr 19, 2016 at 12:06 PM, Alexander Duyck <aduyck@mirantis.com> wrote: >>>>>> This patch assumes that the bnxt hardware will ignore existing IPv4/v6 >>>>>> header fields for length and checksum as well as the length and checksum >>>>>> fields for outer UDP and GRE headers. >>>>>> >>>>>> I have no means of testing this as I do not have any bnx2x hardware but >>>>>> thought I would submit it as an RFC to see if anyone out there wants to >>>>>> test this and see if this does in fact enable this functionality allowing >>>>>> us to to segment tunneled frames that have an outer checksum. >>>>>> >>>>>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com> >>>>> >>>>> Hi Alex, I just did a very quick test of this patch on our bnxt >>>>> hardware and it seemed to work. >>>>> >>>>> I created a vxlan endpoint with udpcsum enabled and I saw TSO packets >>>>> getting through. I've verified that our hardware can be programmed to >>>>> either ignore outer UDP checksum or to calculate it. Current default >>>>> is to ignore ipv4 UDP checksum and calculate ipv6 UDP checksum. >>>>> Thanks. >>>> >>>> Are you saying you can natively support UDP tunnel with outer checksum >>>> offload then? >>> >>> Yes. Calculate or ignore the outer UDP checksum. >> >> I was just thinking about this. When you say you compute the IPv6 >> checksum how is it you are specifying to the hardware that you want to >> do that? Is it something you can configure per packet or is it >> something that is configured for the VXLAN flow? > > In the current version of the hardware, it is a global (chip-wide) > setting. 1 bit to control outer ipv4 vxlan and 1 bit for outer ipv6 > vxlan. > >> >> I just want to make sure you aren't adding checksums to IPv6 tunnels >> that specify that they don't want a checksum, or stripping them from >> v4 tunnels that do want a checksum. > > If the global setting has outer UDP checksum enabled, it will be > calculated no matter what. If the setting is disabled, it will just > ignore it without overwriting it. Okay so if that is the case we may want to make it so that we ignore checksum for both IPv4 and IPv6 and then we can just provide it via GSO_PARTIAL in the case we want it. Otherwise you are technically mangling the frames by inserting a checksum on the outer header even though the tunnel was not configured for it. If you can point me toward the point in the code where that is happening I can probably make it a part of this patch. Thanks. - Alex
On Fri, Apr 29, 2016 at 2:31 PM, Alexander Duyck <alexander.duyck@gmail.com> wrote: > Okay so if that is the case we may want to make it so that we ignore > checksum for both IPv4 and IPv6 and then we can just provide it via > GSO_PARTIAL in the case we want it. Otherwise you are technically > mangling the frames by inserting a checksum on the outer header even > though the tunnel was not configured for it. If you can point me > toward the point in the code where that is happening I can probably > make it a part of this patch. > All the chip settings are controlled by firmware. I will check with the firmware team to disable them if they are not already disabled. When first developing the driver, before all the recent proposals, the intention was to not advertise NETIF_F_GSO_UDP_TUNNEL_CSUM and not support TSO with outer UDP checksum enabled. Thanks.
On Fri, Apr 29, 2016 at 4:29 PM, Michael Chan <michael.chan@broadcom.com> wrote: > On Fri, Apr 29, 2016 at 2:31 PM, Alexander Duyck > <alexander.duyck@gmail.com> wrote: > >> Okay so if that is the case we may want to make it so that we ignore >> checksum for both IPv4 and IPv6 and then we can just provide it via >> GSO_PARTIAL in the case we want it. Otherwise you are technically >> mangling the frames by inserting a checksum on the outer header even >> though the tunnel was not configured for it. If you can point me >> toward the point in the code where that is happening I can probably >> make it a part of this patch. >> > > All the chip settings are controlled by firmware. I will check with > the firmware team to disable them if they are not already disabled. > When first developing the driver, before all the recent proposals, the > intention was to not advertise NETIF_F_GSO_UDP_TUNNEL_CSUM and not > support TSO with outer UDP checksum enabled. Thanks. Right. But adding a checksum where there wasn't one could theoretically be problematic if there was a implementation out there where somebody was mandating that the tunnel checksum must be 0. The i40e driver was enabling the same thing in the driver for an upcoming device that supported an outer checksum offload until I went in and fixed it. Generally if we can match what is expected that is preferred so we don't have any unexpected conflicts in the event that a VTEP expects the packets to come in with checksums or without. I have submitted the patch and when the firmware gets updated the behavior will be cleared up so you can have it either way depending on what the tunnel itself requested. Thanks. - Alex
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index 4645c44e7c15..ae668476fff0 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -6194,14 +6194,19 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_GRE | NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT | - NETIF_F_RXHASH | + NETIF_F_GSO_UDP_TUNNEL_CSUM | NETIF_F_GSO_GRE_CSUM | + NETIF_F_GSO_PARTIAL | NETIF_F_RXHASH | NETIF_F_RXCSUM | NETIF_F_LRO | NETIF_F_GRO; dev->hw_enc_features = NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_SG | NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_GRE | - NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT; + NETIF_F_GSO_UDP_TUNNEL_CSUM | NETIF_F_GSO_GRE_CSUM | + NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT | + NETIF_F_GSO_PARTIAL; + dev->gso_partial_features = NETIF_F_GSO_UDP_TUNNEL_CSUM | + NETIF_F_GSO_GRE_CSUM; dev->vlan_features = dev->hw_features | NETIF_F_HIGHDMA; dev->hw_features |= NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_RX | NETIF_F_HW_VLAN_STAG_TX;
This patch assumes that the bnxt hardware will ignore existing IPv4/v6 header fields for length and checksum as well as the length and checksum fields for outer UDP and GRE headers. I have no means of testing this as I do not have any bnx2x hardware but thought I would submit it as an RFC to see if anyone out there wants to test this and see if this does in fact enable this functionality allowing us to to segment tunneled frames that have an outer checksum. Signed-off-by: Alexander Duyck <aduyck@mirantis.com> --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)