Menu
▾
▴
tipc-discussion
|
From: Jon M. <jm...@re...> - 2021-06-04 19:20:45
|
On 6/4/21 3:44 AM, men...@gm... wrote:
> From: Menglong Dong <don...@zt...>
>
> FB_MTU is used in 'tipc_msg_build()' to alloc smaller skb when memory
> allocation fails, which can avoid unnecessary sending failures.
>
> The value of FB_MTU now is 3744, and the data size will be:
>
> (3744 + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) + \
> SKB_DATA_ALIGN(BUF_HEADROOM + BUF_TAILROOM + 3))
>
> which is larger than one page(4096), and two pages will be allocated.
>
> To avoid it, replace '3744' with a calculation:
>
> FB_MTU = (PAGE_SIZE - SKB_DATA_ALIGN(sizeof(struct skb_shared_info))
> - SKB_DATA_ALIGN(BUF_HEADROOM + BUF_TAILROOM + 3))
>
> Fixes: 4c94cc2d3d57 ("tipc: fall back to smaller MTU if allocation of local send skb fails")
>
> Reported-by: Zeal Robot <ze...@zt...>
> Signed-off-by: Menglong Dong <don...@zt...>
> ---
> net/tipc/bcast.c | 1 +
> net/tipc/msg.c | 8 +------
> net/tipc/msg.h | 1 -
> net/tipc/mtu.h | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 57 insertions(+), 8 deletions(-)
> create mode 100644 net/tipc/mtu.h
>
> diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
> index d4beca895992..c641b68e0812 100644
> --- a/net/tipc/bcast.c
> +++ b/net/tipc/bcast.c
> @@ -41,6 +41,7 @@
> #include "bcast.h"
> #include "link.h"
> #include "name_table.h"
> +#include "mtu.h"
>
> #define BCLINK_WIN_DEFAULT 50 /* bcast link window size (default) */
> #define BCLINK_WIN_MIN 32 /* bcast minimum link window size */
> diff --git a/net/tipc/msg.c b/net/tipc/msg.c
> index ce6ab54822d8..ec70d271c2da 100644
> --- a/net/tipc/msg.c
> +++ b/net/tipc/msg.c
> @@ -40,15 +40,9 @@
> #include "addr.h"
> #include "name_table.h"
> #include "crypto.h"
> +#include "mtu.h"
>
> #define MAX_FORWARD_SIZE 1024
> -#ifdef CONFIG_TIPC_CRYPTO
> -#define BUF_HEADROOM ALIGN(((LL_MAX_HEADER + 48) + EHDR_MAX_SIZE), 16)
> -#define BUF_TAILROOM (TIPC_AES_GCM_TAG_SIZE)
> -#else
> -#define BUF_HEADROOM (LL_MAX_HEADER + 48)
> -#define BUF_TAILROOM 16
> -#endif
>
> static unsigned int align(unsigned int i)
> {
> diff --git a/net/tipc/msg.h b/net/tipc/msg.h
> index 5d64596ba987..e83689d0f0f6 100644
> --- a/net/tipc/msg.h
> +++ b/net/tipc/msg.h
> @@ -99,7 +99,6 @@ struct plist;
> #define MAX_H_SIZE 60 /* Largest possible TIPC header size */
>
> #define MAX_MSG_SIZE (MAX_H_SIZE + TIPC_MAX_USER_MSG_SIZE)
> -#define FB_MTU 3744
> #define TIPC_MEDIA_INFO_OFFSET 5
>
> struct tipc_skb_cb {
> diff --git a/net/tipc/mtu.h b/net/tipc/mtu.h
> new file mode 100644
> index 000000000000..033f0b178f9d
> --- /dev/null
> +++ b/net/tipc/mtu.h
Please don't add any extra file just for this little fix. We have enough
files.
Keep the macros in msg.h/c where they used to be. You can still add
your copyright line to those files.
Regarding the macros kept inside msg.c, they are there because we design
by the principle of minimal exposure, even among our module internal files.
Otherwise it is ok.
Thanks
///jon
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright 2021 ZTE Corporation.
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. Neither the names of the copyright holders nor the names of its
> + * contributors may be used to endorse or promote products derived from
> + * this software without specific prior written permission.
> + *
> + * Alternatively, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") version 2 as published by the Free
> + * Software Foundation.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef _TIPC_MTU_H
> +#define _TIPC_MTU_H
> +
> +#include <linux/tipc.h>
> +#include "crypto.h"
> +
> +#ifdef CONFIG_TIPC_CRYPTO
> +#define BUF_HEADROOM ALIGN(((LL_MAX_HEADER + 48) + EHDR_MAX_SIZE), 16)
> +#define BUF_TAILROOM (TIPC_AES_GCM_TAG_SIZE)
> +#define FB_MTU (PAGE_SIZE - \
> + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) - \
> + SKB_DATA_ALIGN(BUF_HEADROOM + BUF_TAILROOM + 3))
> +#else
> +#define BUF_HEADROOM (LL_MAX_HEADER + 48)
> +#define BUF_TAILROOM 16
> +#define FB_MTU (PAGE_SIZE - \
> + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) - \
> + SKB_DATA_ALIGN(BUF_HEADROOM + 3))
> +#endif
> +
> +#endif
|
|
From: Jon M. <jm...@re...> - 2021-06-07 02:41:08
|
On 6/4/21 9:28 PM, Menglong Dong wrote:
> Hello Maloy,
>
> On Sat, Jun 5, 2021 at 3:20 AM Jon Maloy <jm...@re...> wrote:
>>
> [...]
>> Please don't add any extra file just for this little fix. We have enough
>> files.
>> Keep the macros in msg.h/c where they used to be. You can still add
>> your copyright line to those files.
>> Regarding the macros kept inside msg.c, they are there because we design
>> by the principle of minimal exposure, even among our module internal files.
>> Otherwise it is ok.
>>
> I don't want to add a new file too, but I found it's hard to define FB_MTU. I
> tried to define it in msg.h, and 'crypto.h' should be included, which
> 'BUF_HEADROOM' is defined in. However, 'msg.h' is already included in
> 'crypto.h', so it doesn't work.
>
> I tried to define FB_MTU in 'crypto.h', but it feels weird to define
> it here. And
> FB_MTU is also used in 'bcast.c', so it can't be defined in 'msg.c'.
>
> I will see if there is a better solution.
I think we can leverage the fact that this by definition is a node local
message, and those are never encrypted.
So, if you base FB_MTU on the non-crypto versions of BUF_HEADROOM and
BUF_TAILROOM we should be safe.
That will even give us better utilization of the space available.
///jon
>
> Thanks!
> Menglong Dong
>
>>> @@ -0,0 +1,55 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * Copyright 2021 ZTE Corporation.
>>> + * All rights reserved.
>>> + *
>>> + * Redistribution and use in source and binary forms, with or without
>>> + * modification, are permitted provided that the following conditions are met:
>>> + *
>>> + * 1. Redistributions of source code must retain the above copyright
>>> + * notice, this list of conditions and the following disclaimer.
>>> + * 2. Redistributions in binary form must reproduce the above copyright
>>> + * notice, this list of conditions and the following disclaimer in the
>>> + * documentation and/or other materials provided with the distribution.
>>> + * 3. Neither the names of the copyright holders nor the names of its
>>> + * contributors may be used to endorse or promote products derived from
>>> + * this software without specific prior written permission.
>>> + *
>>> + * Alternatively, this software may be distributed under the terms of the
>>> + * GNU General Public License ("GPL") version 2 as published by the Free
>>> + * Software Foundation.
>>> + *
>>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
>>> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>>> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
>>> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
>>> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>>> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>>> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
>>> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>>> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>>> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
>>> + * POSSIBILITY OF SUCH DAMAGE.
>>> + */
>>> +
>>> +#ifndef _TIPC_MTU_H
>>> +#define _TIPC_MTU_H
>>> +
>>> +#include <linux/tipc.h>
>>> +#include "crypto.h"
>>> +
>>> +#ifdef CONFIG_TIPC_CRYPTO
>>> +#define BUF_HEADROOM ALIGN(((LL_MAX_HEADER + 48) + EHDR_MAX_SIZE), 16)
>>> +#define BUF_TAILROOM (TIPC_AES_GCM_TAG_SIZE)
>>> +#define FB_MTU (PAGE_SIZE - \
>>> + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) - \
>>> + SKB_DATA_ALIGN(BUF_HEADROOM + BUF_TAILROOM + 3))
>>> +#else
>>> +#define BUF_HEADROOM (LL_MAX_HEADER + 48)
>>> +#define BUF_TAILROOM 16
>>> +#define FB_MTU (PAGE_SIZE - \
>>> + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) - \
>>> + SKB_DATA_ALIGN(BUF_HEADROOM + 3))
>>> +#endif
>>> +
>>> +#endif
|
|
From: Jon M. <jm...@re...> - 2021-06-08 22:37:56
|
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 <jm...@re...> 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 |
|
From: Jon M. <jm...@re...> - 2021-06-09 07:34:50
|
On 6/8/21 10:54 PM, Menglong Dong wrote: > On Tue, Jun 08, 2021 at 06:37:38PM -0400, Jon Maloy wrote: >> > [...] >> 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. >> > Yeah, I think I can handle it, just leave it to me. > > (finger heart :/) > Menglong DongI It seems like I have been misleading you. It turns out that these messages *will* be sent out over the nework in some cases, i.e. at multicast/broadcast over an UDP bearer. So, what we need is two macros, one with the conditional crypto head/tailroom defined as you first suggested, and one that only use the non-crypto head/tailroom as we have been discussing now. The first one can be defined inside bcast.c, the latter inside msg.c. It might also be a good idea to give the macros more descriptive names, such as ONEPAGE_MTU in the broadcast version, and ONEPAGE_SKB in the node local version. Does that make sense? ///jon > |
|
From: Jon M. <jm...@re...> - 2021-06-16 17:20:11
|
On 6/15/21 5:45 AM, men...@gm... wrote:
> From: Menglong Dong <don...@zt...>
>
> FB_MTU is used in 'tipc_msg_build()' to alloc smaller skb when memory
> allocation fails, which can avoid unnecessary sending failures.
>
> The value of FB_MTU now is 3744, and the data size will be:
>
> (3744 + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) + \
> SKB_DATA_ALIGN(BUF_HEADROOM + BUF_TAILROOM + 3))
>
> which is larger than one page(4096), and two pages will be allocated.
>
> To avoid it, replace '3744' with a calculation:
>
> FB_MTU=(PAGE_SIZE - \
> SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) - \
> SKB_DATA_ALIGN(BUF_HEADROOM + BUF_TAILROOM + \
> EHDR_MAX_SIZE) \
> )
>
> which is for crypto skb, and:
>
> FB_MTU_LOCAL=(PAGE_SIZE - SKB_DATA_ALIGN(BUF_HEADROOM) - \
> SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) \
> )
> which is for local message.
>
> And BUF_HEADROOM is defined as non-crypto version.
>
> What's more, alloc_skb_fclone() will call SKB_DATA_ALIGN for data size,
> and it's not necessary to make alignment for buf_size in
> tipc_buf_acquire(). So, just remove it.
>
> Fixes: 4c94cc2d3d57 ("tipc: fall back to smaller MTU if allocation of local send skb fails")
>
> Signed-off-by: Menglong Dong <don...@zt...>
> ---
> V3:
> - split tipc_msg_build to tipc_msg_build and tipc_msg_frag
> - introduce tipc_buf_acquire_flex, which is able to alloc skb for
> local message
> - add the variate 'local' in tipc_msg_build to check if this is a
> local message.
>
> V2:
> - define FB_MTU in msg.c instead of introduce a new file
> - remove align for buf_size in tipc_buf_acquire()
> ---
> net/tipc/bcast.c | 2 +-
> net/tipc/msg.c | 168 +++++++++++++++++++++++++++++------------------
> net/tipc/msg.h | 3 +-
> 3 files changed, 108 insertions(+), 65 deletions(-)
>
> diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
> index d4beca895992..9daace9542f4 100644
> --- a/net/tipc/bcast.c
> +++ b/net/tipc/bcast.c
> @@ -699,7 +699,7 @@ int tipc_bcast_init(struct net *net)
> spin_lock_init(&tipc_net(net)->bclock);
>
> if (!tipc_link_bc_create(net, 0, 0, NULL,
> - FB_MTU,
> + fb_mtu,
> BCLINK_WIN_DEFAULT,
> BCLINK_WIN_DEFAULT,
> 0,
> diff --git a/net/tipc/msg.c b/net/tipc/msg.c
> index ce6ab54822d8..349107e08d6f 100644
> --- a/net/tipc/msg.c
> +++ b/net/tipc/msg.c
> @@ -42,19 +42,46 @@
> #include "crypto.h"
>
> #define MAX_FORWARD_SIZE 1024
> +#define BUF_HEADROOM ALIGN(LL_MAX_HEADER + 48, 16)
> +
> #ifdef CONFIG_TIPC_CRYPTO
> -#define BUF_HEADROOM ALIGN(((LL_MAX_HEADER + 48) + EHDR_MAX_SIZE), 16)
> -#define BUF_TAILROOM (TIPC_AES_GCM_TAG_SIZE)
> +#define BUF_TAILROOM TIPC_AES_GCM_TAG_SIZE
> #else
> -#define BUF_HEADROOM (LL_MAX_HEADER + 48)
> -#define BUF_TAILROOM 16
> +#define EHDR_MAX_SIZE 0
> +#define BUF_TAILROOM 0
> #endif
>
We need either a comment or a naming that explains why we are doing all
this, i.e., that we want a buffer that fits within one page.
> +#define FB_MTU (PAGE_SIZE - \
> + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) - \
> + SKB_DATA_ALIGN(BUF_HEADROOM + BUF_TAILROOM + \
> + EHDR_MAX_SIZE) \
> + )
> +
> +#define FB_MTU_LOCAL (PAGE_SIZE - SKB_DATA_ALIGN(BUF_HEADROOM) - \
> + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) \
> + )
> +
> +const int fb_mtu = FB_MTU;
> +
> static unsigned int align(unsigned int i)
> {
> return (i + 3) & ~3u;
> }
This one should be completely replaced with the ALIGN macro, or maybe we
should define our own ALIGN4() macro based on that, unnless there
already is some.
But that should be a separate patch.
>
> +static inline struct sk_buff *tipc_alloc_skb(int headroom, int tailroom,
> + int size, gfp_t gfp)
> +{
> + struct sk_buff *skb;
> +
> + skb = alloc_skb_fclone(size + headroom + tailroom, gfp);
> + if (skb) {
> + skb_reserve(skb, headroom);
> + skb_put(skb, size);
> + skb->next = NULL;
> + }
> + return skb;
> +}
> +
> /**
> * tipc_buf_acquire - creates a TIPC message buffer
> * @size: message size (including TIPC header)
> @@ -68,20 +95,17 @@ static unsigned int align(unsigned int i)
> */
> struct sk_buff *tipc_buf_acquire(u32 size, gfp_t gfp)
> {
> - struct sk_buff *skb;
> -#ifdef CONFIG_TIPC_CRYPTO
> - unsigned int buf_size = (BUF_HEADROOM + size + BUF_TAILROOM + 3) & ~3u;
> -#else
> - unsigned int buf_size = (BUF_HEADROOM + size + 3) & ~3u;
> -#endif
> + return tipc_alloc_skb(BUF_HEADROOM + EHDR_MAX_SIZE,
> + BUF_TAILROOM, size, gfp);
> +}
>
So far, so good.
> - skb = alloc_skb_fclone(buf_size, gfp);
> - if (skb) {
> - skb_reserve(skb, BUF_HEADROOM);
> - skb_put(skb, size);
> - skb->next = NULL;
> - }
> - return skb
> ;
> +static struct sk_buff *tipc_buf_acquire_flex(u32 size, bool local,
> + gfp_t gfp)
I feel this function is overkill and way too intrusive for my comfort,
especially for such a marginal fix this is meant to be.
We sure may save a few bytes when allocating local buffers, but most
buffers are not so close to a page limit a that it will make any difference.
And even if we happen to save a page now and then, it is not worth it.
Stick to tipc_buf_acquire() where it is used now, so we don´t have to
change any more code.
BR
///jon
> +{
> + if (local)
> + return tipc_alloc_skb(BUF_HEADROOM, 0, size, gfp);
> + else
> + return tipc_buf_acquire(size, gfp);
> }
>
> void tipc_msg_init(u32 own_node, struct tipc_msg *m, u32 user, u32 type,
> @@ -357,26 +381,12 @@ int tipc_msg_fragment(struct sk_buff *skb, const struct tipc_msg *hdr,
> return -ENOMEM;
> }
>
> -/**
> - * tipc_msg_build - create buffer chain containing specified header and data
> - * @mhdr: Message header, to be prepended to data
> - * @m: User message
> - * @offset: buffer offset for fragmented messages (FIXME)
> - * @dsz: Total length of user data
> - * @pktmax: Max packet size that can be used
> - * @list: Buffer or chain of buffers to be returned to caller
> - *
> - * Note that the recursive call we are making here is safe, since it can
> - * logically go only one further level down.
> - *
> - * Return: message data size or errno: -ENOMEM, -EFAULT
> - */
> -int tipc_msg_build(struct tipc_msg *mhdr, struct msghdr *m, int offset,
> - int dsz, int pktmax, struct sk_buff_head *list)
> +static int tipc_msg_frag(struct tipc_msg *mhdr, struct msghdr *m, int dsz,
> + int pktmax, struct sk_buff_head *list,
> + bool local)
Same comment as above.
> {
> int mhsz = msg_hdr_sz(mhdr);
> struct tipc_msg pkthdr;
> - int msz = mhsz + dsz;
> int pktrem = pktmax;
> struct sk_buff *skb;
> int drem = dsz;
> @@ -385,33 +395,6 @@ int tipc_msg_build(struct tipc_msg *mhdr, struct msghdr *m, int offset,
> int pktsz;
> int rc;
>
> - msg_set_size(mhdr, msz);
> -
> - /* No fragmentation needed? */
> - if (likely(msz <= pktmax)) {
> - skb = tipc_buf_acquire(msz, GFP_KERNEL);
> -
> - /* Fall back to smaller MTU if node local message */
> - if (unlikely(!skb)) {
> - if (pktmax != MAX_MSG_SIZE)
> - return -ENOMEM;
> - rc = tipc_msg_build(mhdr, m, offset, dsz, FB_MTU, list);
> - if (rc != dsz)
> - return rc;
> - if (tipc_msg_assemble(list))
> - return dsz;
> - return -ENOMEM;
> - }
> - skb_orphan(skb);
> - __skb_queue_tail(list, skb);
> - skb_copy_to_linear_data(skb, mhdr, mhsz);
> - pktpos = skb->data + mhsz;
> - if (copy_from_iter_full(pktpos, dsz, &m->msg_iter))
> - return dsz;
> - rc = -EFAULT;
> - goto error;
> - }
> -
> /* Prepare reusable fragment header */
> tipc_msg_init(msg_prevnode(mhdr), &pkthdr, MSG_FRAGMENTER,
> FIRST_FRAGMENT, INT_H_SIZE, msg_destnode(mhdr));
> @@ -420,7 +403,7 @@ int tipc_msg_build(struct tipc_msg *mhdr, struct msghdr *m, int offset,
> msg_set_importance(&pkthdr, msg_importance(mhdr));
>
> /* Prepare first fragment */
> - skb = tipc_buf_acquire(pktmax, GFP_KERNEL);
> + skb = tipc_buf_acquire_flex(pktmax, local, GFP_KERNEL);
> if (!skb)
> return -ENOMEM;
> skb_orphan(skb);
> @@ -451,7 +434,7 @@ int tipc_msg_build(struct tipc_msg *mhdr, struct msghdr *m, int offset,
> pktsz = drem + INT_H_SIZE;
> else
> pktsz = pktmax;
> - skb = tipc_buf_acquire(pktsz, GFP_KERNEL);
> + skb = tipc_buf_acquire_flex(pktsz, local, GFP_KERNEL);
> if (!skb) {
> rc = -ENOMEM;
> goto error;
> @@ -474,6 +457,65 @@ int tipc_msg_build(struct tipc_msg *mhdr, struct msghdr *m, int offset,
> return rc;
> }
>
> +/**
> + * tipc_msg_build - create buffer chain containing specified header and data
> + * @mhdr: Message header, to be prepended to data
> + * @m: User message
> + * @offset: buffer offset for fragmented messages (FIXME)
> + * @dsz: Total length of user data
> + * @pktmax: Max packet size that can be used
> + * @list: Buffer or chain of buffers to be returned to caller
> + *
> + * Note that the recursive call we are making here is safe, since it can
> + * logically go only one further level down.
> + *
> + * Return: message data size or errno: -ENOMEM, -EFAULT
> + */
> +int tipc_msg_build(struct tipc_msg *mhdr, struct msghdr *m, int offset,
> + int dsz, int pktmax, struct sk_buff_head *list)
> +{
> + int mhsz = msg_hdr_sz(mhdr);
> + int msz = mhsz + dsz;
> + struct sk_buff *skb;
> + bool local = false;
> + char *pktpos;
> + int rc;
> +
> + msg_set_size(mhdr, msz);
> + if (pktmax == MAX_MSG_SIZE)
> + local = true;
> +
> + /* Fragmentation needed */
> + if (unlikely(msz <= pktmax))
> + return tipc_msg_frag(mhdr, m, dsz, pktmax, list, local);
> +
> + skb = tipc_buf_acquire_flex(msz, local, GFP_KERNEL);
> +
> + /* Fall back to smaller MTU if node local message */
> + if (unlikely(!skb))
> + goto try_frag;
> +
> + skb_orphan(skb);
> + skb_copy_to_linear_data(skb, mhdr, mhsz);
> + pktpos = skb->data + mhsz;
> + if (copy_from_iter_full(pktpos, dsz, &m->msg_iter)) {
> + __skb_queue_tail(list, skb);
> + return dsz;
> + }
> + __skb_queue_head_init(list);
> + return -EFAULT;
> +
> +try_frag:
> + if (!local)
> + return -ENOMEM;
> + rc = tipc_msg_frag(mhdr, m, dsz, FB_MTU_LOCAL, list, true);
> + if (rc != dsz)
> + return rc;
> + if (tipc_msg_assemble(list))
> + return dsz;
> + return -ENOMEM;
> +}
> +
> /**
> * tipc_msg_bundle - Append contents of a buffer to tail of an existing one
> * @bskb: the bundle buffer to append to
> diff --git a/net/tipc/msg.h b/net/tipc/msg.h
> index 5d64596ba987..2c214691037c 100644
> --- a/net/tipc/msg.h
> +++ b/net/tipc/msg.h
> @@ -99,9 +99,10 @@ struct plist;
> #define MAX_H_SIZE 60 /* Largest possible TIPC header size */
>
> #define MAX_MSG_SIZE (MAX_H_SIZE + TIPC_MAX_USER_MSG_SIZE)
> -#define FB_MTU 3744
> #define TIPC_MEDIA_INFO_OFFSET 5
>
> +extern const int fb_mtu;
> +
> struct tipc_skb_cb {
> union {
> struct {
|
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X