Re: [net-next v38] mctp pcc: Implement MCTP over PCC Transport

Jakub Kicinski
2026-04-10T03:29:08Z

On Sun, 5 Apr 2026 14:07:40 -0400 Adam Young wrote:
> + rc = mbox_send_message(mpnd->outbox.chan->mchan, skb);
> + if (rc < 0) {
> + //Remove the header in case it gets sent again

space after //

> + skb_pull(skb, sizeof(*pcc_header));
> + netif_stop_queue(ndev);
> + return NETDEV_TX_BUSY;
> + }
> +
> + return NETDEV_TX_OK;
> +}
> +
> +static void mctp_pcc_tx_prepare(struct mbox_client *cl, void *mssg)
> +{
> + struct mctp_pcc_ndev *mctp_pcc_ndev;
> + struct mctp_pcc_mailbox *outbox;
> + struct sk_buff *skb = mssg;
> +
> + mctp_pcc_ndev = container_of(cl, struct mctp_pcc_ndev, outbox.client);
> + outbox = &mctp_pcc_ndev->outbox;
> +
> + if (!skb)
> + return;

Is this really ever called with a NULL pointer?

> + if (skb->len > outbox->chan->shmem_size) {
> + dev_dstats_tx_dropped(mctp_pcc_ndev->ndev);
> + return;
> + }
> + memcpy_toio(outbox->chan->shmem, skb->data, skb->len);
> +}

> +static int mctp_pcc_ndo_open(struct net_device *ndev)

dont say ndo in function names

> +{
> + struct mctp_pcc_ndev *mctp_pcc_ndev = netdev_priv(ndev);
> + struct mctp_pcc_mailbox *outbox, *inbox;
> +
> + outbox = &mctp_pcc_ndev->outbox;
> + inbox = &mctp_pcc_ndev->inbox;
> +
> + outbox->chan = pcc_mbox_request_channel(&outbox->client, outbox->index);
> + if (IS_ERR(outbox->chan))
> + return PTR_ERR(outbox->chan);
> +
> + inbox->client.rx_callback = mctp_pcc_client_rx_callback;
> + inbox->chan = pcc_mbox_request_channel(&inbox->client, inbox->index);
> + if (IS_ERR(inbox->chan)) {
> + pcc_mbox_free_channel(outbox->chan);
> + return PTR_ERR(inbox->chan);
> + }
> + return 0;

> +static int initialize_MTU(struct net_device *ndev)

don't use capital letter in function names for no good reason

> +{
> + struct mctp_pcc_ndev *mctp_pcc_ndev;
> + struct mctp_pcc_mailbox *outbox;
> + struct pcc_mbox_chan *pchan;
> + int mctp_pcc_max_mtu;
> +
> + mctp_pcc_ndev = netdev_priv(ndev);
> + outbox = &mctp_pcc_ndev->outbox;
> + pchan = pcc_mbox_request_channel(&outbox->client, outbox->index);
> + if (IS_ERR(pchan))
> + return -1;

why not return PTR_ERR(pchan) ?

> + if (pchan->shmem_size < MCTP_MIN_MTU + sizeof(struct acpi_pcct_ext_pcc_shared_memory)) {
> + pcc_mbox_free_channel(pchan);
> + return -1;

EINVAL

We are about to close for the merge window please wait until.. Apr 27th
I think with the next version.
> + }