[ovs-dev] [PATCH 2/2] lib/netlink: Use correct netlink max message size

[Greg Rose]

On 11/01/2017 01:33 PM, Ben Pfaff wrote:
> On Wed, Nov 01, 2017 at 01:17:04PM -0700, Greg Rose wrote:
>> On 11/01/2017 01:01 PM, Ben Pfaff wrote:
>>> On Wed, Nov 01, 2017 at 10:33:25AM -0700, Greg Rose wrote:
>>>> On 11/01/2017 09:38 AM, Ben Pfaff wrote:
>>>>> On Wed, Nov 01, 2017 at 08:28:48AM -0700, Greg Rose wrote:
>>>>>> On 10/31/2017 12:53 PM, Ben Pfaff wrote:
>>>>>>> On Fri, Sep 22, 2017 at 07:44:53AM -0700, Greg Rose wrote:
>>>>>>>> The maximum message size for recent Linux kernels is 32Kb and in older
>>>>>>>> kernels it is 16KB.
>>>>>>>>
>>>>>>>> See http://www.spinics.net/lists/netdev/msg431592.html
>>>>>>>>
>>>>>>>> Adjust the size checked and update a comment.
>>>>>>>>
>>>>>>>> Signed-off-by: Greg Rose <gvrose8192 at gmail.com>
>>>>>>>
>>>>>>> ...
>>>>>>>
>>>>>>>> diff --git a/lib/netlink.c b/lib/netlink.c
>>>>>>>> index de3ebcd..04310ff 100644
>>>>>>>> --- a/lib/netlink.c
>>>>>>>> +++ b/lib/netlink.c
>>>>>>>> @@ -570,7 +570,7 @@ nl_msg_next(struct ofpbuf *buffer, struct ofpbuf *msg)
>>>>>>>>    bool
>>>>>>>>    nl_attr_oversized(size_t payload_size)
>>>>>>>>    {
>>>>>>>> -    return payload_size > UINT16_MAX - NLA_HDRLEN;
>>>>>>>> +    return payload_size > INT16_MAX - NLA_HDRLEN;
>>>>>>>>    }
>>>>>>>
>>>>>>> Thanks for the patch!
>>>>>>>
>>>>>>> I am confused by a difference between the commit message and the code.
>>>>>>> Before this patch, nl_attr_oversized() considered an attribute of about
>>>>>>> 64 kB to be oversize; after this patch, about 32 kB.  Shouldn't the new
>>>>>>> value be about 16 kB?
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Ben.
>>>>>>>
>>>>>>
>>>>>> IIRC this is in user space so we prepare for whatever the maximum size we might
>>>>>> get from a kernel, which is 32KB.
>>>>>>
>>>>>> We could have just left it at 64KB but we don't ever need that much.
>>>>>
>>>>> This response implies that nl_attr_oversized() has something to do with
>>>>> Netlink attributes for messages that userspace receives from the kernel.
>>>>> This is not the case.  OVS userspace is prepared to handle any length
>>>>> attribute in Netlink messages that it receives.  It won't apply
>>>>> nl_attr_oversized() to received attributes, it will just go ahead and
>>>>> process them.
>>>>>
>>>>> On the contrary, userspace uses nl_attr_oversized() to limit the maximum
>>>>> size of a Netlink attribute it *sends to* the kernel.  If the kernel
>>>>> only supports a maximum 16 kB or 32 kB attribute, according to its
>>>>> version, then the function needs to apply this limit.  The current code
>>>>> assumes that the kernel can process anything that can actually be
>>>>> formulated into Netlink.
>>>>>
>>>>> What actual limit does Linux apply to Netlink messages, for sending and
>>>>> receiving?  I am beginning to believe that this function is a red
>>>>> herring and does not need to change.
>>>>>
>>>> So you're right, I didn't recall correctly... that's what I get for quick answers.
>>>>
>>>> In net/netlink/af_netlink.c the core netlink handler will attempt  to copy the
>>>> maximum length of the message and if it is not able to then it will set the
>>>> MSG_TRUNC flag and attempt to process whatever it can.  The netlink
>>>> skb's are processed as datagrams.
>>>>
>>>> Generally the large size messages come from the kernel, not to the kernel
>>>> so it is not much of a concern for any applications I'm aware of.
>>>>
>>>> Does OVS send large messages to the kernel?
>>>
>>> One example is when OVS sends a large number of actions as part of
>>> OVS_FLOW_CMD_NEW or OVS_FLOW_CMD_SET or OVS_PACKET_CMD_EXECUTE.  OVS has
>>> a fallback for that, but it needs to know when to use it, that is, it
>>> needs to know how large is too large.
>>>
>>
>> I see.  Then we should probably use libnl's nl_socket_set_buffer_size() to
>> try setting 32KB buffers and then fall back to 16KB buffers if the call
>> isn't successful.
>>
>> http://www.infradead.org/~tgr/libnl/doc/api/group__socket.html#gaec9b1f3b0fdbf4e6e0fb10a233b5df68
>>
>> I'm in the middle of some other stuff right now, some internal bugs, so
>> when I'm done I'll try out a patch to see if it works.
> 
> I don't see the value of changing the sndbuf value.  The default is more
> than 32 kB.
> 
> As far as I can tell from net/core/af_netlink.c, it should work fine to
> send a 64 kB netlink message to the kernel, so I don't think anything
> needs to change in OVS userspace.
> 

That works for me.

Thanks,