| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <7f8adf40-af34-4140-a01f-caf255b51306@amd.com>
Date: Thu, 17 Apr 2025 14:02:38 +0530
From: "Badole, Vishal" <vishal.badole@....com>
To: "Lendacky, Thomas" <Thomas.Lendacky@....com>,
"S-k, Shyam-sundar" <Shyam-sundar.S-k@....com>,
"andrew+netdev@...n.ch" <andrew+netdev@...n.ch>,
"davem@...emloft.net" <davem@...emloft.net>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com"
<pabeni@...hat.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Cc: "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH net 1/2] amd-xgbe: Fix data loss when RX checksum
offloading is disabled
On 4/14/2025 8:04 PM, Lendacky, Thomas wrote:
> On 4/14/25 07:02, Vishal Badole wrote:
>> To properly disable checksum offloading, the split header mode must also
>> be disabled. When split header mode is disabled, the network device stores
>> received packets (with size <= 1536 bytes) entirely in buffer1, leaving
>> buffer2 empty. However, with the current DMA configuration, only 256 bytes
>> from buffer1 are copied from the network device to system memory,
>> resulting in the loss of the remaining packet data.
>>
>> Address the issue by programming the ARBS field to 256 bytes, which aligns
>> with the socket buffer size, and setting the SPH bit in the control
>> register to disable split header mode. With this configuration, the
>> network device stores the first 256 bytes of the received packet in
>> buffer1 and the remaining data in buffer2. The DMA is then able to
>> transfer the full packet from the network device to system memory without
>> any data loss.
>>
>> Cc: stable@...r.kernel.org
>> Fixes: c5aa9e3b8156 ("amd-xgbe: Initial AMD 10GbE platform driver")
>
> Arguably, this patch doesn't fix anything as all you've done is defined
> some routines that aren't called by anything yet. You can probably
> combine this with the actual fix in the next patch and make this a
> single patch.
>
Sure, Thomas, we will combine these patches into a single patch and
resend it with version V2.
> Split-header support wasn't added until commit 174fd2597b0b ("amd-xgbe:
> Implement split header receive support") and receive side scaling wasn't
> added until commit 5b9dfe299e55 ("amd-xgbe: Provide support for receive
> side scaling"), so you'll want to double check your Fixes: tag.
>
Thank you for your observation. We will recheck the Fixes: tag and
update it in the V2 patch version.
> Also, the ARBS field wasn't always present, so you might need to
> investigate that this is truly backwards compatible and earlier versions
> don't require a different fix.
We will update the patch and resend it with the necessary changes to
support earlier versions of the hardware as well.
>
> Thanks,
> Tom
>
>> Signed-off-by: Vishal Badole <Vishal.Badole@....com>
>> ---
>> drivers/net/ethernet/amd/xgbe/xgbe-common.h | 2 ++
>> drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 18 ++++++++++++++++++
>> drivers/net/ethernet/amd/xgbe/xgbe.h | 5 +++++
>> 3 files changed, 25 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-common.h b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
>> index bcb221f74875..d92453ee2505 100644
>> --- a/drivers/net/ethernet/amd/xgbe/xgbe-common.h
>> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
>> @@ -232,6 +232,8 @@
>> #define DMA_CH_IER_TIE_WIDTH 1
>> #define DMA_CH_IER_TXSE_INDEX 1
>> #define DMA_CH_IER_TXSE_WIDTH 1
>> +#define DMA_CH_RCR_ARBS_INDEX 28
>> +#define DMA_CH_RCR_ARBS_WIDTH 3
>> #define DMA_CH_RCR_PBL_INDEX 16
>> #define DMA_CH_RCR_PBL_WIDTH 6
>> #define DMA_CH_RCR_RBSZ_INDEX 1
>> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
>> index 7a923b6e83df..429c5e1444d8 100644
>> --- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
>> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
>> @@ -292,6 +292,8 @@ static void xgbe_config_rx_buffer_size(struct xgbe_prv_data *pdata)
>>
>> XGMAC_DMA_IOWRITE_BITS(pdata->channel[i], DMA_CH_RCR, RBSZ,
>> pdata->rx_buf_size);
>> + XGMAC_DMA_IOWRITE_BITS(pdata->channel[i], DMA_CH_RCR, ARBS,
>> + XGBE_ARBS_SIZE);
>> }
>> }
>>
>> @@ -321,6 +323,18 @@ static void xgbe_config_sph_mode(struct xgbe_prv_data *pdata)
>> XGMAC_IOWRITE_BITS(pdata, MAC_RCR, HDSMS, XGBE_SPH_HDSMS_SIZE);
>> }
>>
>> +static void xgbe_disable_sph_mode(struct xgbe_prv_data *pdata)
>> +{
>> + unsigned int i;
>> +
>> + for (i = 0; i < pdata->channel_count; i++) {
>> + if (!pdata->channel[i]->rx_ring)
>> + break;
>> +
>> + XGMAC_DMA_IOWRITE_BITS(pdata->channel[i], DMA_CH_CR, SPH, 0);
>> + }
>> +}
>> +
>> static int xgbe_write_rss_reg(struct xgbe_prv_data *pdata, unsigned int type,
>> unsigned int index, unsigned int val)
>> {
>> @@ -3910,5 +3924,9 @@ void xgbe_init_function_ptrs_dev(struct xgbe_hw_if *hw_if)
>> hw_if->disable_vxlan = xgbe_disable_vxlan;
>> hw_if->set_vxlan_id = xgbe_set_vxlan_id;
>>
>> + /* For Split Header*/
>> + hw_if->enable_sph = xgbe_config_sph_mode;
>> + hw_if->disable_sph = xgbe_disable_sph_mode;
>> +
>> DBGPR("<--xgbe_init_function_ptrs\n");
>> }
>> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h
>> index db73c8f8b139..1b9c679453fb 100755
>> --- a/drivers/net/ethernet/amd/xgbe/xgbe.h
>> +++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
>> @@ -166,6 +166,7 @@
>> #define XGBE_RX_BUF_ALIGN 64
>> #define XGBE_SKB_ALLOC_SIZE 256
>> #define XGBE_SPH_HDSMS_SIZE 2 /* Keep in sync with SKB_ALLOC_SIZE */
>> +#define XGBE_ARBS_SIZE 3
>>
>> #define XGBE_MAX_DMA_CHANNELS 16
>> #define XGBE_MAX_QUEUES 16
>> @@ -902,6 +903,10 @@ struct xgbe_hw_if {
>> void (*enable_vxlan)(struct xgbe_prv_data *);
>> void (*disable_vxlan)(struct xgbe_prv_data *);
>> void (*set_vxlan_id)(struct xgbe_prv_data *);
>> +
>> + /* For Split Header */
>> + void (*enable_sph)(struct xgbe_prv_data *pdata);
>> + void (*disable_sph)(struct xgbe_prv_data *pdata);
>> };
>>
>> /* This structure represents implementation specific routines for an
Powered by blists - more mailing lists