Re: [PATCH linux dev-6.6 3/3] net: mctp: usb: Port for kernel 6.6

[Santosh Puranik <santosh.puranik.ibm@gmail.com>]

Hi Jeremy,

Thank you for the review.

On 28/04/25 7:13 AM, Jeremy Kerr wrote:
> Hi Santosh,
> 
>> --- a/drivers/net/mctp/mctp-usb.c
>> +++ b/drivers/net/mctp/mctp-usb.c
>> @@ -48,16 +48,17 @@ static void mctp_usb_out_complete(struct urb *urb)
>>          case -ECONNRESET:
>>          case -ESHUTDOWN:
>>          case -EPROTO:
>> -               dev_dstats_tx_dropped(netdev);
>> +        netdev->stats.tx_dropped++;
> 
> Some weird indenting happening here (and with most of the other
> additions too).

Will fix in v2. Thanks.

> 
> If we don't want to use dstats, we should also remove
> 
>      dev->pcpu_stat_type = NETDEV_PCPU_STAT_DSTATS;
> 
> Have you checked that the stats look correct in your backport?

Yes, they did, for ex:

```
mctpusb0  Link encap:UNSPEC  HWaddr 
00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00
           UP RUNNING NOARP  MTU:68  Metric:1
           RX packets:1 errors:0 dropped:0 overruns:0 frame:0
           TX packets:1 errors:0 dropped:0 overruns:0 carrier:0
           collisions:0 txqueuelen:1000
           RX bytes:11 (11.0 B)  TX bytes:13 (13.0 B)
```
I will remove the stat_type assignment and re-run the test before 
submitting v2.

> 
>> @@ -340,7 +342,7 @@ static int mctp_usb_probe(struct usb_interface *intf,
>>   
>>          INIT_DELAYED_WORK(&dev->rx_retry_work, mctp_usb_rx_retry_work);
>>   
>> -       rc = mctp_register_netdev(netdev, NULL, MCTP_PHYS_BINDING_USB);
>> +       rc = mctp_register_netdev(netdev, NULL);
>>          if (rc)
>>                  goto err_free_urbs;
> 
> Alternatively, we could consider backporting 580db513b4a9 ("net: mctp:
> Expose transport binding identifier via IFLA attribute") too. This would
> be user-visible, as we would now have an new IFLA_MCTP_PHYS_BINDING
> attribute on netlink update messages, but that should be entirely
> backward-compatible for applications.

Ack. Please Let me know if you have a preference.

> 
> Cheers,
> 
> 
> Jeremy