Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH net-next] net: tipc: fix FB_MTU eat two pages
To:     Menglong Dong <menglong8.dong@gmail.com>
Cc:     ying.xue@windriver.com, David Miller <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        netdev <netdev@vger.kernel.org>,
        tipc-discussion@lists.sourceforge.net,
        Menglong Dong <dong.menglong@zte.com.cn>,
        Zeal Robot <zealci@zte.com.cn>
References: <20210604074419.53956-1-dong.menglong@zte.com.cn>
 <e997a058-9f6e-86a0-8591-56b0b89441aa@redhat.com>
 <CADxym3ZostCAY0GwUpTxEHcOPyOj5Lmv4F7xP-Q4=AEAVaEAxw@mail.gmail.com>
 <998cce2c-b18d-59c1-df64-fc62856c63a1@redhat.com> <20210607125120.GA4262@www>
From:   Jon Maloy <jmaloy@redhat.com>
Message-ID: <46d2a694-6a85-0f8e-4156-9bb1c4dbdb69@redhat.com>
Date:   Tue, 8 Jun 2021 18:37:38 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.10.1
MIME-Version: 1.0
In-Reply-To: <20210607125120.GA4262@www>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Precedence: bulk


On 6/7/21 8:51 AM, Menglong Dong wrote:
> On Sat, Jun 05, 2021 at 10:25:53AM -0400, Jon Maloy wrote:
>>
>> On 6/4/21 9:28 PM, Menglong Dong wrote:
>>> Hello Maloy,
>>>
>>> On Sat, Jun 5, 2021 at 3:20 AM Jon Maloy <jmaloy@redhat.com> wrote:

>>>> [...]
>>>
>>> So if I use the non-crypto version, the size allocated will be:
>>>
>>>    PAGE_SIZE - BUF_HEADROOM_non-crypto - BUF_TAILROOM_non-crypt +
>>>      BUF_HEADROOM_crypto + BUF_TAILROOM_crypto
>>>
>>> which is larger than PAGE_SIZE.
>>>
>>> So, I think the simple way is to define FB_MTU in 'crypto.h'. Is this
>>> acceptable?
>>>
>>> Thanks!
>>> Menglong Dong
>>>

I spent a little more time looking into this. I think the best we can do 
is to keep FB_MTU internal to msg.c, and then add an outline function to 
msg.c that can be used by bcast.c. The way it is used is never time 
critical.

I also see that we could need a little cleanup around this. There is a 
redundant align() function that should be removed and replaced with the 
global ALIGN() macro.
Even tipc_buf_acquire() should use this macro instead of the explicit 
method that is used now.
In general, I stongly dislike conditional code, and it is not necessary 
in this function. If we redefine the non-crypto BUF_TAILROOM to 0 
instead of 16 (it is not used anywhere else) we could get rid of this too.

But I leave that to you. If you only fix the FB_MTU macro I am content.

///jon