diff mbox series

[v2,02/13] net: Add a 'do_not_pad" to NetClientState

Message ID 20210315075718.5402-3-bmeng.cn@gmail.com (mailing list archive)
State New, archived
Headers show
Series net: Pad short frames for network backends | expand

Commit Message

Bin Meng March 15, 2021, 7:57 a.m. UTC
This adds a flag in NetClientState, so that a net client can tell
its peer that the packets do not need to be padded to the minimum
size of an Ethernet frame (60 bytes) before sending to it.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 include/net/net.h | 1 +
 1 file changed, 1 insertion(+)

Comments

Philippe Mathieu-Daudé March 15, 2021, 9:17 a.m. UTC | #1
On 3/15/21 8:57 AM, Bin Meng wrote:
> This adds a flag in NetClientState, so that a net client can tell
> its peer that the packets do not need to be padded to the minimum
> size of an Ethernet frame (60 bytes) before sending to it.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>  include/net/net.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/net/net.h b/include/net/net.h
> index 919facaad2..6fab1f83f5 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -100,6 +100,7 @@ struct NetClientState {
>      int vring_enable;
>      int vnet_hdr_len;
>      bool is_netdev;
> +    bool do_not_pad;
>      QTAILQ_HEAD(, NetFilterState) filters;
>  };

This is a bit pointless without the next patch, why
not squash it there?
Philippe Mathieu-Daudé March 15, 2021, 9:18 a.m. UTC | #2
On 3/15/21 10:17 AM, Philippe Mathieu-Daudé wrote:
> On 3/15/21 8:57 AM, Bin Meng wrote:
>> This adds a flag in NetClientState, so that a net client can tell
>> its peer that the packets do not need to be padded to the minimum
>> size of an Ethernet frame (60 bytes) before sending to it.
>>
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> ---
>>
>>  include/net/net.h | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/include/net/net.h b/include/net/net.h
>> index 919facaad2..6fab1f83f5 100644
>> --- a/include/net/net.h
>> +++ b/include/net/net.h
>> @@ -100,6 +100,7 @@ struct NetClientState {
>>      int vring_enable;
>>      int vnet_hdr_len;
>>      bool is_netdev;
>> +    bool do_not_pad;
>>      QTAILQ_HEAD(, NetFilterState) filters;
>>  };
> 
> This is a bit pointless without the next patch, why
> not squash it there?

Ah one is SLiRP and the other is tap. OK then.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Philippe Mathieu-Daudé March 15, 2021, 9:22 a.m. UTC | #3
On 3/15/21 10:18 AM, Philippe Mathieu-Daudé wrote:
> On 3/15/21 10:17 AM, Philippe Mathieu-Daudé wrote:
>> On 3/15/21 8:57 AM, Bin Meng wrote:
>>> This adds a flag in NetClientState, so that a net client can tell
>>> its peer that the packets do not need to be padded to the minimum
>>> size of an Ethernet frame (60 bytes) before sending to it.
>>>
>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>> ---
>>>
>>>  include/net/net.h | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/include/net/net.h b/include/net/net.h
>>> index 919facaad2..6fab1f83f5 100644
>>> --- a/include/net/net.h
>>> +++ b/include/net/net.h
>>> @@ -100,6 +100,7 @@ struct NetClientState {
>>>      int vring_enable;
>>>      int vnet_hdr_len;
>>>      bool is_netdev;
>>> +    bool do_not_pad;

Maybe 'do_not_pad_to_min_eth_frame_len' to avoid
wondering what padding is it.

>>>      QTAILQ_HEAD(, NetFilterState) filters;
>>>  };
>>
>> This is a bit pointless without the next patch, why
>> not squash it there?
> 
> Ah one is SLiRP and the other is tap. OK then.
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
Bin Meng March 15, 2021, 10:17 a.m. UTC | #4
Hi Philippe,

On Mon, Mar 15, 2021 at 5:22 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 3/15/21 10:18 AM, Philippe Mathieu-Daudé wrote:
> > On 3/15/21 10:17 AM, Philippe Mathieu-Daudé wrote:
> >> On 3/15/21 8:57 AM, Bin Meng wrote:
> >>> This adds a flag in NetClientState, so that a net client can tell
> >>> its peer that the packets do not need to be padded to the minimum
> >>> size of an Ethernet frame (60 bytes) before sending to it.
> >>>
> >>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> >>> ---
> >>>
> >>>  include/net/net.h | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/include/net/net.h b/include/net/net.h
> >>> index 919facaad2..6fab1f83f5 100644
> >>> --- a/include/net/net.h
> >>> +++ b/include/net/net.h
> >>> @@ -100,6 +100,7 @@ struct NetClientState {
> >>>      int vring_enable;
> >>>      int vnet_hdr_len;
> >>>      bool is_netdev;
> >>> +    bool do_not_pad;
>
> Maybe 'do_not_pad_to_min_eth_frame_len' to avoid
> wondering what padding is it.

I felt the same when I added this :) But I wonder if that name is too long ..

Regards,
Bin
Peter Maydell March 15, 2021, 10:21 a.m. UTC | #5
On Mon, 15 Mar 2021 at 10:17, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Philippe,
>
> On Mon, Mar 15, 2021 at 5:22 PM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
> >
> > On 3/15/21 10:18 AM, Philippe Mathieu-Daudé wrote:
> > > On 3/15/21 10:17 AM, Philippe Mathieu-Daudé wrote:
> > >> On 3/15/21 8:57 AM, Bin Meng wrote:
> > >>> This adds a flag in NetClientState, so that a net client can tell
> > >>> its peer that the packets do not need to be padded to the minimum
> > >>> size of an Ethernet frame (60 bytes) before sending to it.
> > >>>
> > >>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > >>> ---
> > >>>
> > >>>  include/net/net.h | 1 +
> > >>>  1 file changed, 1 insertion(+)
> > >>>
> > >>> diff --git a/include/net/net.h b/include/net/net.h
> > >>> index 919facaad2..6fab1f83f5 100644
> > >>> --- a/include/net/net.h
> > >>> +++ b/include/net/net.h
> > >>> @@ -100,6 +100,7 @@ struct NetClientState {
> > >>>      int vring_enable;
> > >>>      int vnet_hdr_len;
> > >>>      bool is_netdev;
> > >>> +    bool do_not_pad;
> >
> > Maybe 'do_not_pad_to_min_eth_frame_len' to avoid
> > wondering what padding is it.
>
> I felt the same when I added this :) But I wonder if that name is too long ..

If you don't change the name, you could at least add a short
comment in the structure definition describing what the flag does.

thanks
-- PMM
diff mbox series

Patch

diff --git a/include/net/net.h b/include/net/net.h
index 919facaad2..6fab1f83f5 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -100,6 +100,7 @@  struct NetClientState {
     int vring_enable;
     int vnet_hdr_len;
     bool is_netdev;
+    bool do_not_pad;
     QTAILQ_HEAD(, NetFilterState) filters;
 };