Hi Andrew, On 26/04/24 11:43 pm, Andrew Lunn wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Fri, Apr 26, 2024 at 01:45:20PM +0000, Parthiban.Veerasooran@xxxxxxxxxxxxx wrote: >> Hi Andrew, >> >> On 24/04/24 5:38 am, Andrew Lunn wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>>> +static int oa_tc6_allocate_rx_skb(struct oa_tc6 *tc6) >>>> +{ >>>> + tc6->rx_skb = netdev_alloc_skb(tc6->netdev, tc6->netdev->mtu + ETH_HLEN + >>>> + ETH_FCS_LEN + NET_IP_ALIGN); >>>> + if (!tc6->rx_skb) { >>>> + tc6->netdev->stats.rx_dropped++; >>>> + return -ENOMEM; >>>> + } >>>> + skb_reserve(tc6->rx_skb, NET_IP_ALIGN); >>> >>> I think you can use netdev_alloc_skb_ip_align() here. >> Ah OK, then do you mean we can rewrite the function >> oa_tc6_allocate_rx_skb() as below? >> >> static int oa_tc6_allocate_rx_skb(struct oa_tc6 *tc6) >> { >> tc6->rx_skb = netdev_alloc_skb_ip_align(tc6->netdev, tc6->netdev->mtu + >> ETH_HLEN + ETH_FCS_LEN); >> if (tc6->rx_skb) >> return 0; >> >> tc6->netdev->stats.rx_dropped++; >> return -ENOMEM; >> } > > Looks about right. But i did say 'I think', meaning i'm not too sure > about this. > > I generally don't review code actually moving packets around. It is > what developers focus on, test heavily, and so is generally O.K. It is > the code around the edges which often needs improvements prompted by > review, ethtool, PHY handling, statistics. You are right. But I tested implementing your proposal with iperf3 and it works as expected. So will keep this implementation in the next version. Best regards, Parthiban V > > Andrew