| Message ID | 20231013131812.873331-1-manishc@marvell.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2f3389c73832ad90b63208c0fc281ad080114c7a |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] qed: fix LL2 RX buffer allocation | expand |
Hello: This patch was applied to netdev/net.git (main) by David S. Miller <davem@davemloft.net>: On Fri, 13 Oct 2023 18:48:12 +0530 you wrote: > Driver allocates the LL2 rx buffers from kmalloc() > area to construct the skb using slab_build_skb() > > The required size allocation seems to have overlooked > for accounting both skb_shared_info size and device > placement padding bytes which results into the below > panic when doing skb_put() for a standard MTU sized frame. > > [...] Here is the summary with links: - [net] qed: fix LL2 RX buffer allocation https://git.kernel.org/netdev/net/c/2f3389c73832 You are awesome, thank you!
On Fri, Oct 13, 2023 at 06:48:12PM +0530, Manish Chopra wrote: > Driver allocates the LL2 rx buffers from kmalloc() > area to construct the skb using slab_build_skb() > > The required size allocation seems to have overlooked > for accounting both skb_shared_info size and device > placement padding bytes which results into the below > panic when doing skb_put() for a standard MTU sized frame. > > skbuff: skb_over_panic: text:ffffffffc0b0225f len:1514 put:1514 > head:ff3dabceaf39c000 data:ff3dabceaf39c042 tail:0x62c end:0x566 > dev:<NULL> > … > skb_panic+0x48/0x4a > skb_put.cold+0x10/0x10 > qed_ll2b_complete_rx_packet+0x14f/0x260 [qed] > qed_ll2_rxq_handle_completion.constprop.0+0x169/0x200 [qed] > qed_ll2_rxq_completion+0xba/0x320 [qed] > qed_int_sp_dpc+0x1a7/0x1e0 [qed] > > This patch fixes this by accouting skb_shared_info and device > placement padding size bytes when allocating the buffers. > > Cc: David S. Miller <davem@davemloft.net> > Fixes: 0a7fb11c23c0 ("qed: Add Light L2 support") > Signed-off-by: Manish Chopra <manishc@marvell.com> > --- Ack. While normally build_skb will use the full slab object size anyway, this skb_over_panic is seen when the buffer is allocated from a kfence pool which changes the return value of ksize. Reviewed-by: Chris Leech <cleech@redhat.com> > drivers/net/ethernet/qlogic/qed/qed_ll2.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c b/drivers/net/ethernet/qlogic/qed/qed_ll2.c > index 717a0b3f89bd..ab5ef254a748 100644 > --- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c > +++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c > @@ -113,7 +113,10 @@ static void qed_ll2b_complete_tx_packet(void *cxt, > static int qed_ll2_alloc_buffer(struct qed_dev *cdev, > u8 **data, dma_addr_t *phys_addr) > { > - *data = kmalloc(cdev->ll2->rx_size, GFP_ATOMIC); > + size_t size = cdev->ll2->rx_size + NET_SKB_PAD + > + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); > + > + *data = kmalloc(size, GFP_ATOMIC); > if (!(*data)) { > DP_INFO(cdev, "Failed to allocate LL2 buffer data\n"); > return -ENOMEM; > @@ -2589,7 +2592,7 @@ static int qed_ll2_start(struct qed_dev *cdev, struct qed_ll2_params *params) > INIT_LIST_HEAD(&cdev->ll2->list); > spin_lock_init(&cdev->ll2->lock); > > - cdev->ll2->rx_size = NET_SKB_PAD + ETH_HLEN + > + cdev->ll2->rx_size = PRM_DMA_PAD_BYTES_NUM + ETH_HLEN + > L1_CACHE_BYTES + params->mtu; > > /* Allocate memory for LL2. > -- > 2.27.0 >
diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c b/drivers/net/ethernet/qlogic/qed/qed_ll2.c index 717a0b3f89bd..ab5ef254a748 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c +++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c @@ -113,7 +113,10 @@ static void qed_ll2b_complete_tx_packet(void *cxt, static int qed_ll2_alloc_buffer(struct qed_dev *cdev, u8 **data, dma_addr_t *phys_addr) { - *data = kmalloc(cdev->ll2->rx_size, GFP_ATOMIC); + size_t size = cdev->ll2->rx_size + NET_SKB_PAD + + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); + + *data = kmalloc(size, GFP_ATOMIC); if (!(*data)) { DP_INFO(cdev, "Failed to allocate LL2 buffer data\n"); return -ENOMEM; @@ -2589,7 +2592,7 @@ static int qed_ll2_start(struct qed_dev *cdev, struct qed_ll2_params *params) INIT_LIST_HEAD(&cdev->ll2->list); spin_lock_init(&cdev->ll2->lock); - cdev->ll2->rx_size = NET_SKB_PAD + ETH_HLEN + + cdev->ll2->rx_size = PRM_DMA_PAD_BYTES_NUM + ETH_HLEN + L1_CACHE_BYTES + params->mtu; /* Allocate memory for LL2.
Driver allocates the LL2 rx buffers from kmalloc() area to construct the skb using slab_build_skb() The required size allocation seems to have overlooked for accounting both skb_shared_info size and device placement padding bytes which results into the below panic when doing skb_put() for a standard MTU sized frame. skbuff: skb_over_panic: text:ffffffffc0b0225f len:1514 put:1514 head:ff3dabceaf39c000 data:ff3dabceaf39c042 tail:0x62c end:0x566 dev:<NULL> … skb_panic+0x48/0x4a skb_put.cold+0x10/0x10 qed_ll2b_complete_rx_packet+0x14f/0x260 [qed] qed_ll2_rxq_handle_completion.constprop.0+0x169/0x200 [qed] qed_ll2_rxq_completion+0xba/0x320 [qed] qed_int_sp_dpc+0x1a7/0x1e0 [qed] This patch fixes this by accouting skb_shared_info and device placement padding size bytes when allocating the buffers. Cc: David S. Miller <davem@davemloft.net> Fixes: 0a7fb11c23c0 ("qed: Add Light L2 support") Signed-off-by: Manish Chopra <manishc@marvell.com> --- drivers/net/ethernet/qlogic/qed/qed_ll2.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)