[1/8] net/txgbe: fix Rx buffer size in configure register
Checks
Commit Message
When round up buffer size to 1K, to configure the register, hardware will
receive packets exceeding the buffer size in LRO mode. It will cause a
segment fault in the receive function.
Fixes: be797cbf4582 ("net/txgbe: add Rx and Tx init")
Cc: stable@dpdk.org
Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
drivers/net/txgbe/txgbe_rxtx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On 1/18/2023 6:00 AM, Jiawen Wu wrote:
> When round up buffer size to 1K, to configure the register, hardware will
> receive packets exceeding the buffer size in LRO mode. It will cause a
> segment fault in the receive function.
>
> Fixes: be797cbf4582 ("net/txgbe: add Rx and Tx init")
> Cc: stable@dpdk.org
>
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> ---
> drivers/net/txgbe/txgbe_rxtx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/txgbe/txgbe_rxtx.c b/drivers/net/txgbe/txgbe_rxtx.c
> index ac1bba08a3..ae70ca3beb 100644
> --- a/drivers/net/txgbe/txgbe_rxtx.c
> +++ b/drivers/net/txgbe/txgbe_rxtx.c
> @@ -4382,7 +4382,7 @@ txgbe_dev_rx_init(struct rte_eth_dev *dev)
> */
> buf_size = (uint16_t)(rte_pktmbuf_data_room_size(rxq->mb_pool) -
> RTE_PKTMBUF_HEADROOM);
> - buf_size = ROUND_UP(buf_size, 0x1 << 10);
> + buf_size = ROUND_DOWN(buf_size, 0x1 << 10);
> srrctl |= TXGBE_RXCFG_PKTLEN(buf_size);
>
What if 'buf_size' is less than 1K, won't 'ROUND_DOWN' make it 0, and I
assume setting 0 to register may cause problem.
And according to the code comment for 'buf_size' [1], buffer size can't
be more than 16K, but technically 'buf_size' can be more than 16K.
Does the HW constrain values larger than 16K? If not the 'buf_size'
value needs to be checked against the 16K limit.
[1]
/*
* Configure the RX buffer size in the PKTLEN field of
* the RXCFG register of the queue.
* The value is in 1 KB resolution. Valid values can be from
* 1 KB to 16 KB.
*/
On Friday, January 27, 2023 11:36 PM, Ferruh Yigit wrote:
> On 1/18/2023 6:00 AM, Jiawen Wu wrote:
> > When round up buffer size to 1K, to configure the register, hardware
> > will receive packets exceeding the buffer size in LRO mode. It will
> > cause a segment fault in the receive function.
> >
> > Fixes: be797cbf4582 ("net/txgbe: add Rx and Tx init")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> > ---
> > drivers/net/txgbe/txgbe_rxtx.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/txgbe/txgbe_rxtx.c
> > b/drivers/net/txgbe/txgbe_rxtx.c index ac1bba08a3..ae70ca3beb 100644
> > --- a/drivers/net/txgbe/txgbe_rxtx.c
> > +++ b/drivers/net/txgbe/txgbe_rxtx.c
> > @@ -4382,7 +4382,7 @@ txgbe_dev_rx_init(struct rte_eth_dev *dev)
> > */
> > buf_size = (uint16_t)(rte_pktmbuf_data_room_size(rxq->mb_pool) -
> > RTE_PKTMBUF_HEADROOM);
> > - buf_size = ROUND_UP(buf_size, 0x1 << 10);
> > + buf_size = ROUND_DOWN(buf_size, 0x1 << 10);
> > srrctl |= TXGBE_RXCFG_PKTLEN(buf_size);
> >
>
> What if 'buf_size' is less than 1K, won't 'ROUND_DOWN' make it 0, and I assume
> setting 0 to register may cause problem.
>
It is indeed a problem, I will fix it.
> And according to the code comment for 'buf_size' [1], buffer size can't be more than
> 16K, but technically 'buf_size' can be more than 16K.
> Does the HW constrain values larger than 16K? If not the 'buf_size'
> value needs to be checked against the 16K limit.
>
There is a macro that defines the conversion to limit 16K.
#define PKTLEN(v) ROUND_OVER(v, 14, 10)
#define ROUND_OVER(x, maxbits, unitbits) \
((x) >= 1 << (maxbits) ? 0 : (x) >> (unitbits))
>
>
>
> [1]
> /*
> * Configure the RX buffer size in the PKTLEN field of
> * the RXCFG register of the queue.
> * The value is in 1 KB resolution. Valid values can be from
> * 1 KB to 16 KB.
> */
>
On 2/1/2023 2:34 AM, Jiawen Wu wrote:
> On Friday, January 27, 2023 11:36 PM, Ferruh Yigit wrote:
>> On 1/18/2023 6:00 AM, Jiawen Wu wrote:
>>> When round up buffer size to 1K, to configure the register, hardware
>>> will receive packets exceeding the buffer size in LRO mode. It will
>>> cause a segment fault in the receive function.
>>>
>>> Fixes: be797cbf4582 ("net/txgbe: add Rx and Tx init")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
>>> ---
>>> drivers/net/txgbe/txgbe_rxtx.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/txgbe/txgbe_rxtx.c
>>> b/drivers/net/txgbe/txgbe_rxtx.c index ac1bba08a3..ae70ca3beb 100644
>>> --- a/drivers/net/txgbe/txgbe_rxtx.c
>>> +++ b/drivers/net/txgbe/txgbe_rxtx.c
>>> @@ -4382,7 +4382,7 @@ txgbe_dev_rx_init(struct rte_eth_dev *dev)
>>> */
>>> buf_size = (uint16_t)(rte_pktmbuf_data_room_size(rxq->mb_pool) -
>>> RTE_PKTMBUF_HEADROOM);
>>> - buf_size = ROUND_UP(buf_size, 0x1 << 10);
>>> + buf_size = ROUND_DOWN(buf_size, 0x1 << 10);
>>> srrctl |= TXGBE_RXCFG_PKTLEN(buf_size);
>>>
>>
>> What if 'buf_size' is less than 1K, won't 'ROUND_DOWN' make it 0, and I assume
>> setting 0 to register may cause problem.
>>
>
> It is indeed a problem, I will fix it.
>
>> And according to the code comment for 'buf_size' [1], buffer size can't be more than
>> 16K, but technically 'buf_size' can be more than 16K.
>> Does the HW constrain values larger than 16K? If not the 'buf_size'
>> value needs to be checked against the 16K limit.
>>
>
> There is a macro that defines the conversion to limit 16K.
>
> #define PKTLEN(v) ROUND_OVER(v, 14, 10)
> #define ROUND_OVER(x, maxbits, unitbits) \
> ((x) >= 1 << (maxbits) ? 0 : (x) >> (unitbits))
>
ack
@@ -4382,7 +4382,7 @@ txgbe_dev_rx_init(struct rte_eth_dev *dev)
*/
buf_size = (uint16_t)(rte_pktmbuf_data_room_size(rxq->mb_pool) -
RTE_PKTMBUF_HEADROOM);
- buf_size = ROUND_UP(buf_size, 0x1 << 10);
+ buf_size = ROUND_DOWN(buf_size, 0x1 << 10);
srrctl |= TXGBE_RXCFG_PKTLEN(buf_size);
wr32(hw, TXGBE_RXCFG(rxq->reg_idx), srrctl);