Re: [PATCH v2] Add payload to be 32-bit aligned to fix dropped packets

From: Kumar Thangavel <kumarthangavel.hcl@gmail.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: linux-aspeed <linux-aspeed@lists.ozlabs.org>,
	Networking <netdev@vger.kernel.org>,
	OpenBMC Maillist <openbmc@lists.ozlabs.org>,
	Amithash Prasad <amithash@fb.com>,
	patrickw3@fb.com, Samuel Mendoza-Jonas <sam@mendozajonas.com>,
	velumanit@hcl.com, "David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH v2] Add payload to be 32-bit aligned to fix dropped packets
Date: Wed, 20 Oct 2021 13:18:49 +0530	[thread overview]
Message-ID: <CAA7TbcsXCLhzDRuzNgzHFMVqEW3aP7BremDVbedO2eJMG9HFyA@mail.gmail.com> (raw)
In-Reply-To: <20211019075241.7ba9fd0a@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]

Thanks Jakub for your comments.

Please find my response inline below.

On Tue, Oct 19, 2021 at 8:22 PM Jakub Kicinski <kuba@kernel.org> wrote:

> On Tue, 19 Oct 2021 20:11:27 +0530 Kumar Thangavel wrote:
> >       len += sizeof(struct ncsi_cmd_pkt_hdr) + 4;
> > -     if (nca->payload < 26)
> > +     payload = ALIGN(nca->payload, 4)
> > +     if (payload < 26)
> >               len += 26;
> >       else
> > -             len += nca->payload;
> > +             len += payload;
>
> You round up to 4 and then add 26 if the result is smaller.  26 is not
> a multiple of 4. Is this intentional?
>
>     Kumar :  This is intentional. The total number of bytes should be 64.
This 64 bytes includes Ethernet header, NC-SI header and payload, pldm
header and payload.
                   Some pldm commands payload is less than 26.  So we added
remaining bytes to match with 64 and which is 4 bytes aligned.

Also you can write this on one line:
>
>         len += max(payload, 26);
>

    Kumar : Ack. Will update in the next patch set.

[-- Attachment #2: Type: text/html, Size: 1701 bytes --]