| Message ID | 20210303191205.1656980-3-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | net: Handle short frames for SLiRP/TAP interfaces | expand |
On 2021/3/4 3:11 上午, Philippe Mathieu-Daudé wrote: > From: Bin Meng <bin.meng@windriver.com> > > The minimum Ethernet frame length is 60 bytes. For short frames with > smaller length like ARP packets (only 42 bytes), on a real world NIC > it can choose either padding its length to the minimum required 60 > bytes, or sending it out directly to the wire. Such behavior can be > hardcoded or controled by a register bit. Similarly on the receive > path, NICs can choose either dropping such short frames directly or > handing them over to software to handle. > > On the other hand, for the network backends SLiRP/TAP, they don't > expose a way to control the short frame behavior. As of today they > just send/receive data from/to the other end connected to them, > which means any sized packet is acceptable. So they can send and > receive short frames without any problem. It is observed that ARP > packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send > these ARP packets to the other end which might be a NIC model that > does not allow short frames to pass through. Do we need to care about other type of networking backends? E.g socket. Or at least we should keep the padding logic if we can't audit all of the backends. Thanks > > To provide better compatibility, for packets sent from SLiRP/TAP, we > change to pad short frames before sending it out to the other end. > This ensures SLiRP/TAP as an Ethernet sender do not violate the spec. > But with this change, the behavior of dropping short frames in the > NIC model cannot be emulated because it always receives a packet that > is spec complaint. The capability of sending short frames from NIC > models are still supported and short frames can still pass through > SLiRP/TAP interfaces. > > This commit should be able to fix the issue as reported with some > NIC models before, that ARP requests get dropped, preventing the > guest from becoming visible on the network. It was workarounded in > these NIC models on the receive path, that when a short frame is > received, it is padded up to 60 bytes. > > The following 2 commits seem to be the one to workaround this issue > in e1000 and vmxenet3 before, and should probably be reverted. > > commit 78aeb23eded2 ("e1000: Pad short frames to minimum size (60 bytes)") > commit 40a87c6c9b11 ("vmxnet3: Pad short frames to minimum size (60 bytes)") > > Signed-off-by: Bin Meng <bin.meng@windriver.com> > Message-Id: <1614763306-18026-2-git-send-email-bmeng.cn@gmail.com> > [PMD: Use struct iovec for zero-copy] > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > include/net/eth.h | 1 + > net/net.c | 14 ++++++++++++++ > 2 files changed, 15 insertions(+) > > diff --git a/include/net/eth.h b/include/net/eth.h > index 0671be69165..7c825ecb2f7 100644 > --- a/include/net/eth.h > +++ b/include/net/eth.h > @@ -31,6 +31,7 @@ > > #define ETH_ALEN 6 > #define ETH_HLEN 14 > +#define ETH_ZLEN 60 /* Min. octets in frame sans FCS */ > > struct eth_header { > uint8_t h_dest[ETH_ALEN]; /* destination eth addr */ > diff --git a/net/net.c b/net/net.c > index 159e4d0ec25..d42ac9365eb 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -620,6 +620,7 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender, > const uint8_t *buf, int size, > NetPacketSent *sent_cb) > { > + static const uint8_t null_buf[ETH_ZLEN] = { }; > NetQueue *queue; > int ret; > int iovcnt = 1; > @@ -628,6 +629,10 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender, > .iov_base = (void *)buf, > .iov_len = size, > }, > + [1] = { > + .iov_base = (void *)null_buf, > + .iov_len = ETH_ZLEN, > + }, > }; > > #ifdef DEBUG_NET > @@ -639,6 +644,15 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender, > return size; > } > > + /* Pad to minimum Ethernet frame length for SLiRP and TAP */ > + if (sender->info->type == NET_CLIENT_DRIVER_USER || > + sender->info->type == NET_CLIENT_DRIVER_TAP) { > + if (size < ETH_ZLEN) { > + iov[1].iov_len = ETH_ZLEN - size; > + iovcnt = 2; > + } > + } > + > /* Let filters handle the packet first */ > ret = filter_receive_iov(sender, NET_FILTER_DIRECTION_TX, > sender, flags, iov, iovcnt, sent_cb);
On Mon, Mar 8, 2021 at 11:48 AM Jason Wang <jasowang@redhat.com> wrote: > > > On 2021/3/4 3:11 上午, Philippe Mathieu-Daudé wrote: > > From: Bin Meng <bin.meng@windriver.com> > > > > The minimum Ethernet frame length is 60 bytes. For short frames with > > smaller length like ARP packets (only 42 bytes), on a real world NIC > > it can choose either padding its length to the minimum required 60 > > bytes, or sending it out directly to the wire. Such behavior can be > > hardcoded or controled by a register bit. Similarly on the receive > > path, NICs can choose either dropping such short frames directly or > > handing them over to software to handle. > > > > On the other hand, for the network backends SLiRP/TAP, they don't > > expose a way to control the short frame behavior. As of today they > > just send/receive data from/to the other end connected to them, > > which means any sized packet is acceptable. So they can send and > > receive short frames without any problem. It is observed that ARP > > packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send > > these ARP packets to the other end which might be a NIC model that > > does not allow short frames to pass through. > > > Do we need to care about other type of networking backends? E.g socket. > I am not sure as I never used other backends. If someone who is more familiar with the network codes better than me can confirm other backends are also needed, we might do: if (sender->info->type != NET_CLIENT_DRIVER_NIC) > Or at least we should keep the padding logic if we can't audit all of > the backends. Regards, Bin
On Mon, 8 Mar 2021 at 03:48, Jason Wang <jasowang@redhat.com> wrote: > Do we need to care about other type of networking backends? E.g socket. > > Or at least we should keep the padding logic if we can't audit all of > the backends. I think the key thing we need to do here is make a decision and be clear about what we're doing. There are three options I can see: (1) we say that the net API demands that backends pad packets they emit to the minimum ethernet frame length unless they specifically are intending to emit a short frame, and we fix any backends that don't comply (or equivalently, add support in the core code for a backend to mark itself as "I don't pad; please do it for me"). (2) we say that the networking subsystem doesn't support short packets, and just have the common code always enforce padding short frames to the minimum length somewhere between when it receives a packet from a backend and passes it to a NIC model. (3) we say that it's the job of the NIC models to pad short frames as they see them coming in. I think (3) is pretty clearly the worst of these, since it requires every NIC model to handle it; it has no advantages over (2) that I can see. I don't have a strong take on whether we'd rather have (1) or (2): it's a tradeoff between whether we support modelling of short frames vs simplicity of code. I'd just like us to be clear about what point or points in the code have the responsibility for padding short frames. On + if (sender->info->type == NET_CLIENT_DRIVER_USER || + sender->info->type == NET_CLIENT_DRIVER_TAP) { vs > if (sender->info->type != NET_CLIENT_DRIVER_NIC) another option would be to add a 'bool pad_short_tx_frames' to the NetClientInfo struct, and have those things which don't pad set it to true. thanks -- PMM
On 2021/3/8 6:22 下午, Peter Maydell wrote: > On Mon, 8 Mar 2021 at 03:48, Jason Wang <jasowang@redhat.com> wrote: >> Do we need to care about other type of networking backends? E.g socket. >> >> Or at least we should keep the padding logic if we can't audit all of >> the backends. > I think the key thing we need to do here is make a decision > and be clear about what we're doing. There are three options > I can see: > > (1) we say that the net API demands that backends pad > packets they emit to the minimum ethernet frame length > unless they specifically are intending to emit a short frame, > and we fix any backends that don't comply (or equivalently, > add support in the core code for a backend to mark itself > as "I don't pad; please do it for me"). > > (2) we say that the networking subsystem doesn't support > short packets, and just have the common code always enforce > padding short frames to the minimum length somewhere between > when it receives a packet from a backend and passes it to > a NIC model. > > (3) we say that it's the job of the NIC models to pad > short frames as they see them coming in. > > I think (3) is pretty clearly the worst of these, since it > requires every NIC model to handle it; it has no advantages > over (2) that I can see. I don't have a strong take on whether > we'd rather have (1) or (2): it's a tradeoff between whether > we support modelling of short frames vs simplicity of code. > I'd just like us to be clear about what point or points in > the code have the responsibility for padding short frames. I'm not sure how much value we can gain from (1). So (2) looks better to me. Bin or Philippe, want to send a new version? Thanks > > On > + if (sender->info->type == NET_CLIENT_DRIVER_USER || > + sender->info->type == NET_CLIENT_DRIVER_TAP) { > > vs >> if (sender->info->type != NET_CLIENT_DRIVER_NIC) > another option would be to add a 'bool pad_short_tx_frames' to > the NetClientInfo struct, and have those things which don't > pad set it to true. > > thanks > -- PMM >
Hi Jason, On Tue, Mar 9, 2021 at 4:23 PM Jason Wang <jasowang@redhat.com> wrote: > > > On 2021/3/8 6:22 下午, Peter Maydell wrote: > > On Mon, 8 Mar 2021 at 03:48, Jason Wang <jasowang@redhat.com> wrote: > >> Do we need to care about other type of networking backends? E.g socket. > >> > >> Or at least we should keep the padding logic if we can't audit all of > >> the backends. > > I think the key thing we need to do here is make a decision > > and be clear about what we're doing. There are three options > > I can see: > > > > (1) we say that the net API demands that backends pad > > packets they emit to the minimum ethernet frame length > > unless they specifically are intending to emit a short frame, > > and we fix any backends that don't comply (or equivalently, > > add support in the core code for a backend to mark itself > > as "I don't pad; please do it for me"). > > > > (2) we say that the networking subsystem doesn't support > > short packets, and just have the common code always enforce > > padding short frames to the minimum length somewhere between > > when it receives a packet from a backend and passes it to > > a NIC model. > > > > (3) we say that it's the job of the NIC models to pad > > short frames as they see them coming in. > > > > I think (3) is pretty clearly the worst of these, since it > > requires every NIC model to handle it; it has no advantages > > over (2) that I can see. I don't have a strong take on whether > > we'd rather have (1) or (2): it's a tradeoff between whether > > we support modelling of short frames vs simplicity of code. > > I'd just like us to be clear about what point or points in > > the code have the responsibility for padding short frames. > > > I'm not sure how much value we can gain from (1). So (2) looks better to me. > > Bin or Philippe, want to send a new version? > I think this series does what (2) asks for. Or am I missing anything? Regards, Bin
On 2021/3/9 4:35 下午, Bin Meng wrote: > Hi Jason, > > On Tue, Mar 9, 2021 at 4:23 PM Jason Wang <jasowang@redhat.com> wrote: >> >> On 2021/3/8 6:22 下午, Peter Maydell wrote: >>> On Mon, 8 Mar 2021 at 03:48, Jason Wang <jasowang@redhat.com> wrote: >>>> Do we need to care about other type of networking backends? E.g socket. >>>> >>>> Or at least we should keep the padding logic if we can't audit all of >>>> the backends. >>> I think the key thing we need to do here is make a decision >>> and be clear about what we're doing. There are three options >>> I can see: >>> >>> (1) we say that the net API demands that backends pad >>> packets they emit to the minimum ethernet frame length >>> unless they specifically are intending to emit a short frame, >>> and we fix any backends that don't comply (or equivalently, >>> add support in the core code for a backend to mark itself >>> as "I don't pad; please do it for me"). >>> >>> (2) we say that the networking subsystem doesn't support >>> short packets, and just have the common code always enforce >>> padding short frames to the minimum length somewhere between >>> when it receives a packet from a backend and passes it to >>> a NIC model. >>> >>> (3) we say that it's the job of the NIC models to pad >>> short frames as they see them coming in. >>> >>> I think (3) is pretty clearly the worst of these, since it >>> requires every NIC model to handle it; it has no advantages >>> over (2) that I can see. I don't have a strong take on whether >>> we'd rather have (1) or (2): it's a tradeoff between whether >>> we support modelling of short frames vs simplicity of code. >>> I'd just like us to be clear about what point or points in >>> the code have the responsibility for padding short frames. >> >> I'm not sure how much value we can gain from (1). So (2) looks better to me. >> >> Bin or Philippe, want to send a new version? >> > I think this series does what (2) asks for. Or am I missing anything? It only did the padding for user/TAP. Thanks > > Regards, > Bin >
Hi Jason, On Tue, Mar 9, 2021 at 4:57 PM Jason Wang <jasowang@redhat.com> wrote: > > > On 2021/3/9 4:35 下午, Bin Meng wrote: > > Hi Jason, > > > > On Tue, Mar 9, 2021 at 4:23 PM Jason Wang <jasowang@redhat.com> wrote: > >> > >> On 2021/3/8 6:22 下午, Peter Maydell wrote: > >>> On Mon, 8 Mar 2021 at 03:48, Jason Wang <jasowang@redhat.com> wrote: > >>>> Do we need to care about other type of networking backends? E.g socket. > >>>> > >>>> Or at least we should keep the padding logic if we can't audit all of > >>>> the backends. > >>> I think the key thing we need to do here is make a decision > >>> and be clear about what we're doing. There are three options > >>> I can see: > >>> > >>> (1) we say that the net API demands that backends pad > >>> packets they emit to the minimum ethernet frame length > >>> unless they specifically are intending to emit a short frame, > >>> and we fix any backends that don't comply (or equivalently, > >>> add support in the core code for a backend to mark itself > >>> as "I don't pad; please do it for me"). > >>> > >>> (2) we say that the networking subsystem doesn't support > >>> short packets, and just have the common code always enforce > >>> padding short frames to the minimum length somewhere between > >>> when it receives a packet from a backend and passes it to > >>> a NIC model. > >>> > >>> (3) we say that it's the job of the NIC models to pad > >>> short frames as they see them coming in. > >>> > >>> I think (3) is pretty clearly the worst of these, since it > >>> requires every NIC model to handle it; it has no advantages > >>> over (2) that I can see. I don't have a strong take on whether > >>> we'd rather have (1) or (2): it's a tradeoff between whether > >>> we support modelling of short frames vs simplicity of code. > >>> I'd just like us to be clear about what point or points in > >>> the code have the responsibility for padding short frames. > >> > >> I'm not sure how much value we can gain from (1). So (2) looks better to me. > >> > >> Bin or Philippe, want to send a new version? > >> > > I think this series does what (2) asks for. Or am I missing anything? > > > It only did the padding for user/TAP. Ah, so we want this:
Hi Jason, On Tue, Mar 9, 2021 at 5:00 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Jason, > > On Tue, Mar 9, 2021 at 4:57 PM Jason Wang <jasowang@redhat.com> wrote: > > > > > > On 2021/3/9 4:35 下午, Bin Meng wrote: > > > Hi Jason, > > > > > > On Tue, Mar 9, 2021 at 4:23 PM Jason Wang <jasowang@redhat.com> wrote: > > >> > > >> On 2021/3/8 6:22 下午, Peter Maydell wrote: > > >>> On Mon, 8 Mar 2021 at 03:48, Jason Wang <jasowang@redhat.com> wrote: > > >>>> Do we need to care about other type of networking backends? E.g socket. > > >>>> > > >>>> Or at least we should keep the padding logic if we can't audit all of > > >>>> the backends. > > >>> I think the key thing we need to do here is make a decision > > >>> and be clear about what we're doing. There are three options > > >>> I can see: > > >>> > > >>> (1) we say that the net API demands that backends pad > > >>> packets they emit to the minimum ethernet frame length > > >>> unless they specifically are intending to emit a short frame, > > >>> and we fix any backends that don't comply (or equivalently, > > >>> add support in the core code for a backend to mark itself > > >>> as "I don't pad; please do it for me"). > > >>> > > >>> (2) we say that the networking subsystem doesn't support > > >>> short packets, and just have the common code always enforce > > >>> padding short frames to the minimum length somewhere between > > >>> when it receives a packet from a backend and passes it to > > >>> a NIC model. > > >>> > > >>> (3) we say that it's the job of the NIC models to pad > > >>> short frames as they see them coming in. > > >>> > > >>> I think (3) is pretty clearly the worst of these, since it > > >>> requires every NIC model to handle it; it has no advantages > > >>> over (2) that I can see. I don't have a strong take on whether > > >>> we'd rather have (1) or (2): it's a tradeoff between whether > > >>> we support modelling of short frames vs simplicity of code. > > >>> I'd just like us to be clear about what point or points in > > >>> the code have the responsibility for padding short frames. > > >> > > >> I'm not sure how much value we can gain from (1). So (2) looks better to me. > > >> > > >> Bin or Philippe, want to send a new version? > > >> > > > I think this series does what (2) asks for. Or am I missing anything? > > > > > > It only did the padding for user/TAP. > (hit send too soon ...) Ah, so we want this: if (sender->info->type != NET_CLIENT_DRIVER_NIC) correct? Regards, Bin
On Tue, 9 Mar 2021 at 09:01, Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Jason, > > On Tue, Mar 9, 2021 at 5:00 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Jason, > > > > On Tue, Mar 9, 2021 at 4:57 PM Jason Wang <jasowang@redhat.com> wrote: > > > > > > > > > On 2021/3/9 4:35 下午, Bin Meng wrote: > > > > Hi Jason, > > > > > > > > On Tue, Mar 9, 2021 at 4:23 PM Jason Wang <jasowang@redhat.com> wrote: > > > >> > > > >> On 2021/3/8 6:22 下午, Peter Maydell wrote: > > > >>> I think the key thing we need to do here is make a decision > > > >>> and be clear about what we're doing. There are three options > > > >>> I can see: > > > >>> > > > >>> (1) we say that the net API demands that backends pad > > > >>> packets they emit to the minimum ethernet frame length > > > >>> unless they specifically are intending to emit a short frame, > > > >>> and we fix any backends that don't comply (or equivalently, > > > >>> add support in the core code for a backend to mark itself > > > >>> as "I don't pad; please do it for me"). > > > >>> > > > >>> (2) we say that the networking subsystem doesn't support > > > >>> short packets, and just have the common code always enforce > > > >>> padding short frames to the minimum length somewhere between > > > >>> when it receives a packet from a backend and passes it to > > > >>> a NIC model. > > > >>> > > > >>> (3) we say that it's the job of the NIC models to pad > > > >>> short frames as they see them coming in. > > > >> I'm not sure how much value we can gain from (1). So (2) looks better to me. > > > >> > > > >> Bin or Philippe, want to send a new version? > > > >> > > > > I think this series does what (2) asks for. Or am I missing anything? > > > > > > > > > It only did the padding for user/TAP. > > > > (hit send too soon ...) > > Ah, so we want this: > > if (sender->info->type != NET_CLIENT_DRIVER_NIC) > > correct? No, option (2) is "always pad short packets regardless of sender->info->type". Even if a NIC driver sends out a short packet, we want to pad it, because we might be feeding it to something that assumes it does not see short packets. thanks -- PMM
Hi Peter, On Tue, Mar 9, 2021 at 6:13 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Tue, 9 Mar 2021 at 09:01, Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Jason, > > > > On Tue, Mar 9, 2021 at 5:00 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > Hi Jason, > > > > > > On Tue, Mar 9, 2021 at 4:57 PM Jason Wang <jasowang@redhat.com> wrote: > > > > > > > > > > > > On 2021/3/9 4:35 下午, Bin Meng wrote: > > > > > Hi Jason, > > > > > > > > > > On Tue, Mar 9, 2021 at 4:23 PM Jason Wang <jasowang@redhat.com> wrote: > > > > >> > > > > >> On 2021/3/8 6:22 下午, Peter Maydell wrote: > > > > >>> I think the key thing we need to do here is make a decision > > > > >>> and be clear about what we're doing. There are three options > > > > >>> I can see: > > > > >>> > > > > >>> (1) we say that the net API demands that backends pad > > > > >>> packets they emit to the minimum ethernet frame length > > > > >>> unless they specifically are intending to emit a short frame, > > > > >>> and we fix any backends that don't comply (or equivalently, > > > > >>> add support in the core code for a backend to mark itself > > > > >>> as "I don't pad; please do it for me"). > > > > >>> > > > > >>> (2) we say that the networking subsystem doesn't support > > > > >>> short packets, and just have the common code always enforce > > > > >>> padding short frames to the minimum length somewhere between > > > > >>> when it receives a packet from a backend and passes it to > > > > >>> a NIC model. > > > > >>> > > > > >>> (3) we say that it's the job of the NIC models to pad > > > > >>> short frames as they see them coming in. > > > > > >> I'm not sure how much value we can gain from (1). So (2) looks better to me. > > > > >> > > > > >> Bin or Philippe, want to send a new version? > > > > >> > > > > > I think this series does what (2) asks for. Or am I missing anything? > > > > > > > > > > > > It only did the padding for user/TAP. > > > > > > > (hit send too soon ...) > > > > Ah, so we want this: > > > > if (sender->info->type != NET_CLIENT_DRIVER_NIC) > > > > correct? > > No, option (2) is "always pad short packets regardless of > sender->info->type". Even if a NIC driver sends out a short > packet, we want to pad it, because we might be feeding it to > something that assumes it does not see short packets. Then this is v1: http://patchwork.ozlabs.org/project/qemu-devel/patch/1614333786-74258-2-git-send-email-bmeng.cn@gmail.com/ If yes, I will respin v3. Regards, Bin
> On 9 Mar 2021, at 12:13 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Tue, 9 Mar 2021 at 09:01, Bin Meng <bmeng.cn@gmail.com <mailto:bmeng.cn@gmail.com>> wrote: >> >> Hi Jason, >> >> On Tue, Mar 9, 2021 at 5:00 PM Bin Meng <bmeng.cn@gmail.com> wrote: >>> >>> Hi Jason, >>> >>> On Tue, Mar 9, 2021 at 4:57 PM Jason Wang <jasowang@redhat.com> wrote: >>>> >>>> >>>> On 2021/3/9 4:35 下午, Bin Meng wrote: >>>>> Hi Jason, >>>>> >>>>> On Tue, Mar 9, 2021 at 4:23 PM Jason Wang <jasowang@redhat.com> wrote: >>>>>> >>>>>> On 2021/3/8 6:22 下午, Peter Maydell wrote: >>>>>>> I think the key thing we need to do here is make a decision >>>>>>> and be clear about what we're doing. There are three options >>>>>>> I can see: >>>>>>> >>>>>>> (1) we say that the net API demands that backends pad >>>>>>> packets they emit to the minimum ethernet frame length >>>>>>> unless they specifically are intending to emit a short frame, >>>>>>> and we fix any backends that don't comply (or equivalently, >>>>>>> add support in the core code for a backend to mark itself >>>>>>> as "I don't pad; please do it for me"). >>>>>>> >>>>>>> (2) we say that the networking subsystem doesn't support >>>>>>> short packets, and just have the common code always enforce >>>>>>> padding short frames to the minimum length somewhere between >>>>>>> when it receives a packet from a backend and passes it to >>>>>>> a NIC model. >>>>>>> >>>>>>> (3) we say that it's the job of the NIC models to pad >>>>>>> short frames as they see them coming in. > >>>>>> I'm not sure how much value we can gain from (1). So (2) looks better to me. >>>>>> >>>>>> Bin or Philippe, want to send a new version? >>>>>> >>>>> I think this series does what (2) asks for. Or am I missing anything? >>>> >>>> >>>> It only did the padding for user/TAP. >>> >> >> (hit send too soon ...) >> >> Ah, so we want this: >> >> if (sender->info->type != NET_CLIENT_DRIVER_NIC) >> >> correct? > > No, option (2) is "always pad short packets regardless of > sender->info->type". Even if a NIC driver sends out a short > packet, we want to pad it, because we might be feeding it to > something that assumes it does not see short packets. Some thought on this option - in such case with virtio-net, can we also get an indication from the device that the packet will be padded? Currently we are padding short packets in Windows driver (this is MS certification requirement), and it will be nice not do to this in the guest if device will announce such capability. Best regards, Yan. > > thanks > -- PMM
On Tue, Mar 9, 2021 at 8:30 PM Yan Vugenfirer <yvugenfi@redhat.com> wrote: > > > > On 9 Mar 2021, at 12:13 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Tue, 9 Mar 2021 at 09:01, Bin Meng <bmeng.cn@gmail.com> wrote: > > > Hi Jason, > > On Tue, Mar 9, 2021 at 5:00 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > Hi Jason, > > On Tue, Mar 9, 2021 at 4:57 PM Jason Wang <jasowang@redhat.com> wrote: > > > > On 2021/3/9 4:35 下午, Bin Meng wrote: > > Hi Jason, > > On Tue, Mar 9, 2021 at 4:23 PM Jason Wang <jasowang@redhat.com> wrote: > > > On 2021/3/8 6:22 下午, Peter Maydell wrote: > > I think the key thing we need to do here is make a decision > and be clear about what we're doing. There are three options > I can see: > > (1) we say that the net API demands that backends pad > packets they emit to the minimum ethernet frame length > unless they specifically are intending to emit a short frame, > and we fix any backends that don't comply (or equivalently, > add support in the core code for a backend to mark itself > as "I don't pad; please do it for me"). > > (2) we say that the networking subsystem doesn't support > short packets, and just have the common code always enforce > padding short frames to the minimum length somewhere between > when it receives a packet from a backend and passes it to > a NIC model. > > (3) we say that it's the job of the NIC models to pad > short frames as they see them coming in. > > > I'm not sure how much value we can gain from (1). So (2) looks better to me. > > Bin or Philippe, want to send a new version? > > I think this series does what (2) asks for. Or am I missing anything? > > > > It only did the padding for user/TAP. > > > > (hit send too soon ...) > > Ah, so we want this: > > if (sender->info->type != NET_CLIENT_DRIVER_NIC) > > correct? > > > No, option (2) is "always pad short packets regardless of > sender->info->type". Even if a NIC driver sends out a short > packet, we want to pad it, because we might be feeding it to > something that assumes it does not see short packets. > > > Some thought on this option - in such case with virtio-net, can we also get an indication from the device that the packet will be padded? > Currently we are padding short packets in Windows driver (this is MS certification requirement), and it will be nice not do to this in the guest if device will announce such capability. > This is more of a virtio-net specification question. virtio-net should expose a register bit to control this behavior, just like a real world NIC does. Regards, Bin
On 2021/3/9 6:13 下午, Peter Maydell wrote: > On Tue, 9 Mar 2021 at 09:01, Bin Meng <bmeng.cn@gmail.com> wrote: >> Hi Jason, >> >> On Tue, Mar 9, 2021 at 5:00 PM Bin Meng <bmeng.cn@gmail.com> wrote: >>> Hi Jason, >>> >>> On Tue, Mar 9, 2021 at 4:57 PM Jason Wang <jasowang@redhat.com> wrote: >>>> >>>> On 2021/3/9 4:35 下午, Bin Meng wrote: >>>>> Hi Jason, >>>>> >>>>> On Tue, Mar 9, 2021 at 4:23 PM Jason Wang <jasowang@redhat.com> wrote: >>>>>> On 2021/3/8 6:22 下午, Peter Maydell wrote: >>>>>>> I think the key thing we need to do here is make a decision >>>>>>> and be clear about what we're doing. There are three options >>>>>>> I can see: >>>>>>> >>>>>>> (1) we say that the net API demands that backends pad >>>>>>> packets they emit to the minimum ethernet frame length >>>>>>> unless they specifically are intending to emit a short frame, >>>>>>> and we fix any backends that don't comply (or equivalently, >>>>>>> add support in the core code for a backend to mark itself >>>>>>> as "I don't pad; please do it for me"). >>>>>>> >>>>>>> (2) we say that the networking subsystem doesn't support >>>>>>> short packets, and just have the common code always enforce >>>>>>> padding short frames to the minimum length somewhere between >>>>>>> when it receives a packet from a backend and passes it to >>>>>>> a NIC model. >>>>>>> >>>>>>> (3) we say that it's the job of the NIC models to pad >>>>>>> short frames as they see them coming in. >>>>>> I'm not sure how much value we can gain from (1). So (2) looks better to me. >>>>>> >>>>>> Bin or Philippe, want to send a new version? >>>>>> >>>>> I think this series does what (2) asks for. Or am I missing anything? >>>> >>>> It only did the padding for user/TAP. >> (hit send too soon ...) >> >> Ah, so we want this: >> >> if (sender->info->type != NET_CLIENT_DRIVER_NIC) >> >> correct? > No, option (2) is "always pad short packets regardless of > sender->info->type". Even if a NIC driver sends out a short > packet, we want to pad it, because we might be feeding it to > something that assumes it does not see short packets. > > thanks > -- PMM So I'm not sure this is correct. There're NIC that has its own logic that choose whether to pad the frame during TX (e.g e1000). And after a discussion 10 years ago [1]. Michael (cced) seems to want to keep the padding logic in the NIC itself (probably with a generic helper in the core). Since 1) the padding is only required for ethernet 2) virito-net doesn't need that (it can pass incomplete packet by design). Thanks [1] https://patchwork.ozlabs.org/project/qemu-devel/patch/1284842625-13920-1-git-send-email-stefanha@linux.vnet.ibm.com/ >
On Thu, Mar 11, 2021 at 11:01 AM Jason Wang <jasowang@redhat.com> wrote: > > > On 2021/3/9 6:13 下午, Peter Maydell wrote: > > On Tue, 9 Mar 2021 at 09:01, Bin Meng <bmeng.cn@gmail.com> wrote: > >> Hi Jason, > >> > >> On Tue, Mar 9, 2021 at 5:00 PM Bin Meng <bmeng.cn@gmail.com> wrote: > >>> Hi Jason, > >>> > >>> On Tue, Mar 9, 2021 at 4:57 PM Jason Wang <jasowang@redhat.com> wrote: > >>>> > >>>> On 2021/3/9 4:35 下午, Bin Meng wrote: > >>>>> Hi Jason, > >>>>> > >>>>> On Tue, Mar 9, 2021 at 4:23 PM Jason Wang <jasowang@redhat.com> wrote: > >>>>>> On 2021/3/8 6:22 下午, Peter Maydell wrote: > >>>>>>> I think the key thing we need to do here is make a decision > >>>>>>> and be clear about what we're doing. There are three options > >>>>>>> I can see: > >>>>>>> > >>>>>>> (1) we say that the net API demands that backends pad > >>>>>>> packets they emit to the minimum ethernet frame length > >>>>>>> unless they specifically are intending to emit a short frame, > >>>>>>> and we fix any backends that don't comply (or equivalently, > >>>>>>> add support in the core code for a backend to mark itself > >>>>>>> as "I don't pad; please do it for me"). > >>>>>>> > >>>>>>> (2) we say that the networking subsystem doesn't support > >>>>>>> short packets, and just have the common code always enforce > >>>>>>> padding short frames to the minimum length somewhere between > >>>>>>> when it receives a packet from a backend and passes it to > >>>>>>> a NIC model. > >>>>>>> > >>>>>>> (3) we say that it's the job of the NIC models to pad > >>>>>>> short frames as they see them coming in. > >>>>>> I'm not sure how much value we can gain from (1). So (2) looks better to me. > >>>>>> > >>>>>> Bin or Philippe, want to send a new version? > >>>>>> > >>>>> I think this series does what (2) asks for. Or am I missing anything? > >>>> > >>>> It only did the padding for user/TAP. > >> (hit send too soon ...) > >> > >> Ah, so we want this: > >> > >> if (sender->info->type != NET_CLIENT_DRIVER_NIC) > >> > >> correct? > > No, option (2) is "always pad short packets regardless of > > sender->info->type". Even if a NIC driver sends out a short > > packet, we want to pad it, because we might be feeding it to > > something that assumes it does not see short packets. > > > > thanks > > -- PMM > > > So I'm not sure this is correct. There're NIC that has its own logic > that choose whether to pad the frame during TX (e.g e1000). Yes, that's why I mentioned in v2's cover letter that we should probably only do the padding for SLiRP and TAP. For NIC models, we can still support sending short frames in QEMU. > > And after a discussion 10 years ago [1]. Michael (cced) seems to want to > keep the padding logic in the NIC itself (probably with a generic helper > in the core). Since 1) the padding is only required for ethernet 2) > virito-net doesn't need that (it can pass incomplete packet by design). > I did read this discussion before working on this patch series. Providing a helper for NICs to call does NOT fix the issue for SLiRP and TAP. > Thanks > > [1] > https://patchwork.ozlabs.org/project/qemu-devel/patch/1284842625-13920-1-git-send-email-stefanha@linux.vnet.ibm.com/ > Regards, Bin
On 2021/3/11 11:12 上午, Bin Meng wrote: > On Thu, Mar 11, 2021 at 11:01 AM Jason Wang <jasowang@redhat.com> wrote: >> >> On 2021/3/9 6:13 下午, Peter Maydell wrote: >>> On Tue, 9 Mar 2021 at 09:01, Bin Meng <bmeng.cn@gmail.com> wrote: >>>> Hi Jason, >>>> >>>> On Tue, Mar 9, 2021 at 5:00 PM Bin Meng <bmeng.cn@gmail.com> wrote: >>>>> Hi Jason, >>>>> >>>>> On Tue, Mar 9, 2021 at 4:57 PM Jason Wang <jasowang@redhat.com> wrote: >>>>>> On 2021/3/9 4:35 下午, Bin Meng wrote: >>>>>>> Hi Jason, >>>>>>> >>>>>>> On Tue, Mar 9, 2021 at 4:23 PM Jason Wang <jasowang@redhat.com> wrote: >>>>>>>> On 2021/3/8 6:22 下午, Peter Maydell wrote: >>>>>>>>> I think the key thing we need to do here is make a decision >>>>>>>>> and be clear about what we're doing. There are three options >>>>>>>>> I can see: >>>>>>>>> >>>>>>>>> (1) we say that the net API demands that backends pad >>>>>>>>> packets they emit to the minimum ethernet frame length >>>>>>>>> unless they specifically are intending to emit a short frame, >>>>>>>>> and we fix any backends that don't comply (or equivalently, >>>>>>>>> add support in the core code for a backend to mark itself >>>>>>>>> as "I don't pad; please do it for me"). >>>>>>>>> >>>>>>>>> (2) we say that the networking subsystem doesn't support >>>>>>>>> short packets, and just have the common code always enforce >>>>>>>>> padding short frames to the minimum length somewhere between >>>>>>>>> when it receives a packet from a backend and passes it to >>>>>>>>> a NIC model. >>>>>>>>> >>>>>>>>> (3) we say that it's the job of the NIC models to pad >>>>>>>>> short frames as they see them coming in. >>>>>>>> I'm not sure how much value we can gain from (1). So (2) looks better to me. >>>>>>>> >>>>>>>> Bin or Philippe, want to send a new version? >>>>>>>> >>>>>>> I think this series does what (2) asks for. Or am I missing anything? >>>>>> It only did the padding for user/TAP. >>>> (hit send too soon ...) >>>> >>>> Ah, so we want this: >>>> >>>> if (sender->info->type != NET_CLIENT_DRIVER_NIC) >>>> >>>> correct? >>> No, option (2) is "always pad short packets regardless of >>> sender->info->type". Even if a NIC driver sends out a short >>> packet, we want to pad it, because we might be feeding it to >>> something that assumes it does not see short packets. >>> >>> thanks >>> -- PMM >> >> So I'm not sure this is correct. There're NIC that has its own logic >> that choose whether to pad the frame during TX (e.g e1000). > Yes, that's why I mentioned in v2's cover letter that we should > probably only do the padding for SLiRP and TAP. Actually it's a partail implementation of Peter's method 1. If we go that way, you need to make sure the packet is padded for every ethernet backend not just TAP and SLIRP. > For NIC models, we can > still support sending short frames in QEMU. Then it will be padded as well. > >> And after a discussion 10 years ago [1]. Michael (cced) seems to want to >> keep the padding logic in the NIC itself (probably with a generic helper >> in the core). Since 1) the padding is only required for ethernet 2) >> virito-net doesn't need that (it can pass incomplete packet by design). >> > I did read this discussion before working on this patch series. > Providing a helper for NICs to call does NOT fix the issue for SLiRP > and TAP. I'm not sure I get here. For TX, the padding is controlled by the guest driver. So we just need to emulate what real NIC did, pad only when it was required explicitly by the driver. For RX, if we receive short frames from an ethternet backend, we simply pad them to make sure it won't be dropped by the NIC model. So we're actually fine here? Thanks > >> Thanks >> >> [1] >> https://patchwork.ozlabs.org/project/qemu-devel/patch/1284842625-13920-1-git-send-email-stefanha@linux.vnet.ibm.com/ >> > Regards, > Bin >
On Thu, Mar 11, 2021 at 11:33 AM Jason Wang <jasowang@redhat.com> wrote: > > > On 2021/3/11 11:12 上午, Bin Meng wrote: > > On Thu, Mar 11, 2021 at 11:01 AM Jason Wang <jasowang@redhat.com> wrote: > >> > >> On 2021/3/9 6:13 下午, Peter Maydell wrote: > >>> On Tue, 9 Mar 2021 at 09:01, Bin Meng <bmeng.cn@gmail.com> wrote: > >>>> Hi Jason, > >>>> > >>>> On Tue, Mar 9, 2021 at 5:00 PM Bin Meng <bmeng.cn@gmail.com> wrote: > >>>>> Hi Jason, > >>>>> > >>>>> On Tue, Mar 9, 2021 at 4:57 PM Jason Wang <jasowang@redhat.com> wrote: > >>>>>> On 2021/3/9 4:35 下午, Bin Meng wrote: > >>>>>>> Hi Jason, > >>>>>>> > >>>>>>> On Tue, Mar 9, 2021 at 4:23 PM Jason Wang <jasowang@redhat.com> wrote: > >>>>>>>> On 2021/3/8 6:22 下午, Peter Maydell wrote: > >>>>>>>>> I think the key thing we need to do here is make a decision > >>>>>>>>> and be clear about what we're doing. There are three options > >>>>>>>>> I can see: > >>>>>>>>> > >>>>>>>>> (1) we say that the net API demands that backends pad > >>>>>>>>> packets they emit to the minimum ethernet frame length > >>>>>>>>> unless they specifically are intending to emit a short frame, > >>>>>>>>> and we fix any backends that don't comply (or equivalently, > >>>>>>>>> add support in the core code for a backend to mark itself > >>>>>>>>> as "I don't pad; please do it for me"). > >>>>>>>>> > >>>>>>>>> (2) we say that the networking subsystem doesn't support > >>>>>>>>> short packets, and just have the common code always enforce > >>>>>>>>> padding short frames to the minimum length somewhere between > >>>>>>>>> when it receives a packet from a backend and passes it to > >>>>>>>>> a NIC model. > >>>>>>>>> > >>>>>>>>> (3) we say that it's the job of the NIC models to pad > >>>>>>>>> short frames as they see them coming in. > >>>>>>>> I'm not sure how much value we can gain from (1). So (2) looks better to me. > >>>>>>>> > >>>>>>>> Bin or Philippe, want to send a new version? > >>>>>>>> > >>>>>>> I think this series does what (2) asks for. Or am I missing anything? > >>>>>> It only did the padding for user/TAP. > >>>> (hit send too soon ...) > >>>> > >>>> Ah, so we want this: > >>>> > >>>> if (sender->info->type != NET_CLIENT_DRIVER_NIC) > >>>> > >>>> correct? > >>> No, option (2) is "always pad short packets regardless of > >>> sender->info->type". Even if a NIC driver sends out a short > >>> packet, we want to pad it, because we might be feeding it to > >>> something that assumes it does not see short packets. > >>> > >>> thanks > >>> -- PMM > >> > >> So I'm not sure this is correct. There're NIC that has its own logic > >> that choose whether to pad the frame during TX (e.g e1000). > > Yes, that's why I mentioned in v2's cover letter that we should > > probably only do the padding for SLiRP and TAP. > > > Actually it's a partail implementation of Peter's method 1. If we go > that way, you need to make sure the packet is padded for every ethernet > backend not just TAP and SLIRP. > This latest version series implemented the method 1, except for providing a flag to indicate when NIC can send short frames and not pad in the network codes. > > > For NIC models, we can > > still support sending short frames in QEMU. > > > Then it will be padded as well. > > > > > >> And after a discussion 10 years ago [1]. Michael (cced) seems to want to > >> keep the padding logic in the NIC itself (probably with a generic helper > >> in the core). Since 1) the padding is only required for ethernet 2) > >> virito-net doesn't need that (it can pass incomplete packet by design). > >> > > I did read this discussion before working on this patch series. > > Providing a helper for NICs to call does NOT fix the issue for SLiRP > > and TAP. > > > I'm not sure I get here. > > For TX, the padding is controlled by the guest driver. So we just need > to emulate what real NIC did, pad only when it was required explicitly > by the driver. Correct, version 2 of the RFC series do allow NIC models to send short frames, but the latest version changed to pad short frames for every network backends including NICs. > > For RX, if we receive short frames from an ethternet backend, we simply > pad them to make sure it won't be dropped by the NIC model. > No, we don't do this. Hardware NICs don't do such. They either simply drop the short frames, or receive it as it is. > So we're actually fine here? > Regards, Bin
On Thu, 11 Mar 2021 at 03:01, Jason Wang <jasowang@redhat.com> wrote: > > > On 2021/3/9 6:13 下午, Peter Maydell wrote: > > On Tue, 9 Mar 2021 at 09:01, Bin Meng <bmeng.cn@gmail.com> wrote: > >> Ah, so we want this: > >> > >> if (sender->info->type != NET_CLIENT_DRIVER_NIC) > >> > >> correct? > > No, option (2) is "always pad short packets regardless of > > sender->info->type". Even if a NIC driver sends out a short > > packet, we want to pad it, because we might be feeding it to > > something that assumes it does not see short packets. > > So I'm not sure this is correct. There're NIC that has its own logic > that choose whether to pad the frame during TX (e.g e1000). Yes; that would be dead-code if we go for "always pad", the same as the code in devices to handle incoming short packets would also be dead. > And after a discussion 10 years ago [1]. Michael (cced) seems to want to > keep the padding logic in the NIC itself (probably with a generic helper > in the core). Since 1) the padding is only required for ethernet 2) > virito-net doesn't need that (it can pass incomplete packet by design). Like I said, we need to decide; either: (1) we do want to support short packets in the net core: every sender needs to either pad, or to have some flag to say "my implementation can't pad, please can the net core do it for me", unless they are deliberately sending a short packet. Every receiver needs to be able to cope with short packets, at least in the sense of not crashing (they should report them as a rx error if they have that kind of error reporting status register). I think we have mostly implemented our NIC models this way. (2) we simply don't support short packets in the net core: nobody (not NICs, not network backends) needs to pad, because they can rely on the core to do it. Some existing senders and receivers may have now-dead code to do their own padding which could be removed. MST is advocating for (1) in that old thread. That's a coherent position. You've said earlier that we want (2). That's also a coherent position. A mix of the two doesn't seem to me to be very workable. thanks -- PMM
On Thu, Mar 11, 2021 at 5:43 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Thu, 11 Mar 2021 at 03:01, Jason Wang <jasowang@redhat.com> wrote: > > > > > > On 2021/3/9 6:13 下午, Peter Maydell wrote: > > > On Tue, 9 Mar 2021 at 09:01, Bin Meng <bmeng.cn@gmail.com> wrote: > > >> Ah, so we want this: > > >> > > >> if (sender->info->type != NET_CLIENT_DRIVER_NIC) > > >> > > >> correct? > > > > No, option (2) is "always pad short packets regardless of > > > sender->info->type". Even if a NIC driver sends out a short > > > packet, we want to pad it, because we might be feeding it to > > > something that assumes it does not see short packets. > > > > So I'm not sure this is correct. There're NIC that has its own logic > > that choose whether to pad the frame during TX (e.g e1000). > > Yes; that would be dead-code if we go for "always pad", the same > as the code in devices to handle incoming short packets would also > be dead. > > > And after a discussion 10 years ago [1]. Michael (cced) seems to want to > > keep the padding logic in the NIC itself (probably with a generic helper > > in the core). Since 1) the padding is only required for ethernet 2) > > virito-net doesn't need that (it can pass incomplete packet by design). > > Like I said, we need to decide; either: > > (1) we do want to support short packets in the net core: > every sender needs to either pad, or to have some flag to say > "my implementation can't pad, please can the net core do it for me", > unless they are deliberately sending a short packet. Every > receiver needs to be able to cope with short packets, at least > in the sense of not crashing (they should report them as a rx > error if they have that kind of error reporting status register). > I think we have mostly implemented our NIC models this way. > > (2) we simply don't support short packets in the net core: > nobody (not NICs, not network backends) needs to pad, because > they can rely on the core to do it. Some existing senders and > receivers may have now-dead code to do their own padding which > could be removed. > > MST is advocating for (1) in that old thread. That's a coherent > position. But it's a wrong approach. As Edgar and Stefan also said in the old discussion thread, padding in the RX is wrong as real world NICs don't do this. > You've said earlier that we want (2). That's also > a coherent position. A mix of the two doesn't seem to > me to be very workable. Regards, Bin
On Thu, 11 Mar 2021 at 09:58, Bin Meng <bmeng.cn@gmail.com> wrote: > > On Thu, Mar 11, 2021 at 5:43 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > > > On Thu, 11 Mar 2021 at 03:01, Jason Wang <jasowang@redhat.com> wrote: > > > And after a discussion 10 years ago [1]. Michael (cced) seems to want to > > > keep the padding logic in the NIC itself (probably with a generic helper > > > in the core). Since 1) the padding is only required for ethernet 2) > > > virito-net doesn't need that (it can pass incomplete packet by design). > > > > Like I said, we need to decide; either: > > > > (1) we do want to support short packets in the net core: > > every sender needs to either pad, or to have some flag to say > > "my implementation can't pad, please can the net core do it for me", > > unless they are deliberately sending a short packet. Every > > receiver needs to be able to cope with short packets, at least > > in the sense of not crashing (they should report them as a rx > > error if they have that kind of error reporting status register). > > I think we have mostly implemented our NIC models this way. > > > > (2) we simply don't support short packets in the net core: > > nobody (not NICs, not network backends) needs to pad, because > > they can rely on the core to do it. Some existing senders and > > receivers may have now-dead code to do their own padding which > > could be removed. > > > > MST is advocating for (1) in that old thread. That's a coherent > > position. > > But it's a wrong approach. As Edgar and Stefan also said in the old > discussion thread, padding in the RX is wrong as real world NICs don't > do this. Neither option (1) nor option (2) involve padding in RX. Option (1) is: * no NIC implementation pads on TX, except as defined by whatever NIC-specific config registers or h/w behaviour might require (ie if the guest wants to send a short packet it can do that) * non-NIC sources like slirp need to pad on TX unless they're deliberately trying to send a short packet * all receivers of packets need to cope with being given a short packet; this is usually going to mean "flag it to the guest as an RX error", but exact behaviour is NIC-dependent Option (2) is: * the net core code pads any packet that goes through it * no NIC implementation needs to pad on TX (it is harmless if they do) * non-NIC sources don't need to pad on TX * no receivers of packets need to cope with being given short packets Option 1 is what the real world does. Option 2 is a simplification which throws away the ability to emulate handling of short packets, in exchange for not having to sort out senders like slirp and not having to be so careful about short-packet handling in NIC models. If MST is correct that some use cases require short-packet support, then we need to go for option 1, I think. thanks -- PMM
On Thu, Mar 11, 2021 at 6:22 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Thu, 11 Mar 2021 at 09:58, Bin Meng <bmeng.cn@gmail.com> wrote: > > > > On Thu, Mar 11, 2021 at 5:43 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > > > > > On Thu, 11 Mar 2021 at 03:01, Jason Wang <jasowang@redhat.com> wrote: > > > > And after a discussion 10 years ago [1]. Michael (cced) seems to want to > > > > keep the padding logic in the NIC itself (probably with a generic helper > > > > in the core). Since 1) the padding is only required for ethernet 2) > > > > virito-net doesn't need that (it can pass incomplete packet by design). > > > > > > Like I said, we need to decide; either: > > > > > > (1) we do want to support short packets in the net core: > > > every sender needs to either pad, or to have some flag to say > > > "my implementation can't pad, please can the net core do it for me", > > > unless they are deliberately sending a short packet. Every > > > receiver needs to be able to cope with short packets, at least > > > in the sense of not crashing (they should report them as a rx > > > error if they have that kind of error reporting status register). > > > I think we have mostly implemented our NIC models this way. > > > > > > (2) we simply don't support short packets in the net core: > > > nobody (not NICs, not network backends) needs to pad, because > > > they can rely on the core to do it. Some existing senders and > > > receivers may have now-dead code to do their own padding which > > > could be removed. > > > > > > MST is advocating for (1) in that old thread. That's a coherent > > > position. > > > > But it's a wrong approach. As Edgar and Stefan also said in the old > > discussion thread, padding in the RX is wrong as real world NICs don't > > do this. > > Neither option (1) nor option (2) involve padding in RX. Correct. What I referred to is the current approach used in many NIC modes, which is wrong, and we have to correct this. > > Option (1) is: > * no NIC implementation pads on TX, except as defined > by whatever NIC-specific config registers or h/w behaviour > might require (ie if the guest wants to send a short packet > it can do that) > * non-NIC sources like slirp need to pad on TX unless they're > deliberately trying to send a short packet > * all receivers of packets need to cope with being given a > short packet; this is usually going to mean "flag it to the > guest as an RX error", but exact behaviour is NIC-dependent > My patch series in RFC v2/v3 does almost exactly this option (1), except "flag it to the guest as an RX error". > Option (2) is: > * the net core code pads any packet that goes through it > * no NIC implementation needs to pad on TX (it is harmless if they do) > * non-NIC sources don't need to pad on TX > * no receivers of packets need to cope with being given short packets > > Option 1 is what the real world does. Option 2 is a simplification > which throws away the ability to emulate handling of short packets, > in exchange for not having to sort out senders like slirp and not > having to be so careful about short-packet handling in NIC models. > > If MST is correct that some use cases require short-packet support, > then we need to go for option 1, I think. Regards, Bin
On 2021/3/11 6:27 下午, Bin Meng wrote: > On Thu, Mar 11, 2021 at 6:22 PM Peter Maydell <peter.maydell@linaro.org> wrote: >> On Thu, 11 Mar 2021 at 09:58, Bin Meng <bmeng.cn@gmail.com> wrote: >>> On Thu, Mar 11, 2021 at 5:43 PM Peter Maydell <peter.maydell@linaro.org> wrote: >>>> On Thu, 11 Mar 2021 at 03:01, Jason Wang <jasowang@redhat.com> wrote: >>>>> And after a discussion 10 years ago [1]. Michael (cced) seems to want to >>>>> keep the padding logic in the NIC itself (probably with a generic helper >>>>> in the core). Since 1) the padding is only required for ethernet 2) >>>>> virito-net doesn't need that (it can pass incomplete packet by design). >>>> Like I said, we need to decide; either: >>>> >>>> (1) we do want to support short packets in the net core: >>>> every sender needs to either pad, or to have some flag to say >>>> "my implementation can't pad, please can the net core do it for me", >>>> unless they are deliberately sending a short packet. Every >>>> receiver needs to be able to cope with short packets, at least >>>> in the sense of not crashing (they should report them as a rx >>>> error if they have that kind of error reporting status register). >>>> I think we have mostly implemented our NIC models this way. >>>> >>>> (2) we simply don't support short packets in the net core: >>>> nobody (not NICs, not network backends) needs to pad, because >>>> they can rely on the core to do it. Some existing senders and >>>> receivers may have now-dead code to do their own padding which >>>> could be removed. >>>> >>>> MST is advocating for (1) in that old thread. That's a coherent >>>> position. >>> But it's a wrong approach. As Edgar and Stefan also said in the old >>> discussion thread, padding in the RX is wrong as real world NICs don't >>> do this. >> Neither option (1) nor option (2) involve padding in RX. > Correct. What I referred to is the current approach used in many NIC > modes, which is wrong, and we have to correct this. > >> Option (1) is: >> * no NIC implementation pads on TX, except as defined >> by whatever NIC-specific config registers or h/w behaviour >> might require (ie if the guest wants to send a short packet >> it can do that) >> * non-NIC sources like slirp need to pad on TX unless they're >> deliberately trying to send a short packet >> * all receivers of packets need to cope with being given a >> short packet; this is usually going to mean "flag it to the >> guest as an RX error", but exact behaviour is NIC-dependent >> > My patch series in RFC v2/v3 does almost exactly this option (1), > except "flag it to the guest as an RX error". Is it? You did it at net core instead of netdevs if I read the code correctly. > >> Option (2) is: >> * the net core code pads any packet that goes through it >> * no NIC implementation needs to pad on TX (it is harmless if they do) >> * non-NIC sources don't need to pad on TX >> * no receivers of packets need to cope with being given short packets >> >> Option 1 is what the real world does. Option 2 is a simplification >> which throws away the ability to emulate handling of short packets, >> in exchange for not having to sort out senders like slirp and not >> having to be so careful about short-packet handling in NIC models. >> >> If MST is correct that some use cases require short-packet support, >> then we need to go for option 1, I think. For NIC RX, I wonder whether we can introduce a boolean to whether it requries padding. And then the netdevs can only do the padding when it's required. E.g virito-net doesn't need padding. Thanks > Regards, > Bin >
On 2021/3/9 8:30 下午, Yan Vugenfirer wrote: > > >> On 9 Mar 2021, at 12:13 PM, Peter Maydell <peter.maydell@linaro.org >> <mailto:peter.maydell@linaro.org>> wrote: >> >> On Tue, 9 Mar 2021 at 09:01, Bin Meng <bmeng.cn@gmail.com >> <mailto:bmeng.cn@gmail.com>> wrote: >>> >>> Hi Jason, >>> >>> On Tue, Mar 9, 2021 at 5:00 PM Bin Meng <bmeng.cn@gmail.com >>> <mailto:bmeng.cn@gmail.com>> wrote: >>>> >>>> Hi Jason, >>>> >>>> On Tue, Mar 9, 2021 at 4:57 PM Jason Wang <jasowang@redhat.com >>>> <mailto:jasowang@redhat.com>> wrote: >>>>> >>>>> >>>>> On 2021/3/9 4:35 下午, Bin Meng wrote: >>>>>> Hi Jason, >>>>>> >>>>>> On Tue, Mar 9, 2021 at 4:23 PM Jason Wang <jasowang@redhat.com >>>>>> <mailto:jasowang@redhat.com>> wrote: >>>>>>> >>>>>>> On 2021/3/8 6:22 下午, Peter Maydell wrote: >>>>>>>> I think the key thing we need to do here is make a decision >>>>>>>> and be clear about what we're doing. There are three options >>>>>>>> I can see: >>>>>>>> >>>>>>>> (1) we say that the net API demands that backends pad >>>>>>>> packets they emit to the minimum ethernet frame length >>>>>>>> unless they specifically are intending to emit a short frame, >>>>>>>> and we fix any backends that don't comply (or equivalently, >>>>>>>> add support in the core code for a backend to mark itself >>>>>>>> as "I don't pad; please do it for me"). >>>>>>>> >>>>>>>> (2) we say that the networking subsystem doesn't support >>>>>>>> short packets, and just have the common code always enforce >>>>>>>> padding short frames to the minimum length somewhere between >>>>>>>> when it receives a packet from a backend and passes it to >>>>>>>> a NIC model. >>>>>>>> >>>>>>>> (3) we say that it's the job of the NIC models to pad >>>>>>>> short frames as they see them coming in. >> >>>>>>> I'm not sure how much value we can gain from (1). So (2) looks >>>>>>> better to me. >>>>>>> >>>>>>> Bin or Philippe, want to send a new version? >>>>>>> >>>>>> I think this series does what (2) asks for. Or am I missing anything? >>>>> >>>>> >>>>> It only did the padding for user/TAP. >>>> >>> >>> (hit send too soon ...) >>> >>> Ah, so we want this: >>> >>> if (sender->info->type != NET_CLIENT_DRIVER_NIC) >>> >>> correct? >> >> No, option (2) is "always pad short packets regardless of >> sender->info->type". Even if a NIC driver sends out a short >> packet, we want to pad it, because we might be feeding it to >> something that assumes it does not see short packets. > > Some thought on this option - in such case with virtio-net, can we > also get an indication from the device that the packet will be padded? > Currently we are padding short packets in Windows driver (this is MS > certification requirement), and it will be nice not do to this in the > guest if device will announce such capability. Just to make sure I understand the requirement. Is it a way to control padding from the driver or soemthing in the vnet header that tells the driver that a packet has been padded? Thanks > > Best regards, > Yan. > >> >> thanks >> -- PMM >
On Fri, Mar 12, 2021 at 2:23 PM Jason Wang <jasowang@redhat.com> wrote: > > > On 2021/3/11 6:27 下午, Bin Meng wrote: > > On Thu, Mar 11, 2021 at 6:22 PM Peter Maydell <peter.maydell@linaro.org> wrote: > >> On Thu, 11 Mar 2021 at 09:58, Bin Meng <bmeng.cn@gmail.com> wrote: > >>> On Thu, Mar 11, 2021 at 5:43 PM Peter Maydell <peter.maydell@linaro.org> wrote: > >>>> On Thu, 11 Mar 2021 at 03:01, Jason Wang <jasowang@redhat.com> wrote: > >>>>> And after a discussion 10 years ago [1]. Michael (cced) seems to want to > >>>>> keep the padding logic in the NIC itself (probably with a generic helper > >>>>> in the core). Since 1) the padding is only required for ethernet 2) > >>>>> virito-net doesn't need that (it can pass incomplete packet by design). > >>>> Like I said, we need to decide; either: > >>>> > >>>> (1) we do want to support short packets in the net core: > >>>> every sender needs to either pad, or to have some flag to say > >>>> "my implementation can't pad, please can the net core do it for me", > >>>> unless they are deliberately sending a short packet. Every > >>>> receiver needs to be able to cope with short packets, at least > >>>> in the sense of not crashing (they should report them as a rx > >>>> error if they have that kind of error reporting status register). > >>>> I think we have mostly implemented our NIC models this way. > >>>> > >>>> (2) we simply don't support short packets in the net core: > >>>> nobody (not NICs, not network backends) needs to pad, because > >>>> they can rely on the core to do it. Some existing senders and > >>>> receivers may have now-dead code to do their own padding which > >>>> could be removed. > >>>> > >>>> MST is advocating for (1) in that old thread. That's a coherent > >>>> position. > >>> But it's a wrong approach. As Edgar and Stefan also said in the old > >>> discussion thread, padding in the RX is wrong as real world NICs don't > >>> do this. > >> Neither option (1) nor option (2) involve padding in RX. > > Correct. What I referred to is the current approach used in many NIC > > modes, which is wrong, and we have to correct this. > > > >> Option (1) is: > >> * no NIC implementation pads on TX, except as defined > >> by whatever NIC-specific config registers or h/w behaviour > >> might require (ie if the guest wants to send a short packet > >> it can do that) > >> * non-NIC sources like slirp need to pad on TX unless they're > >> deliberately trying to send a short packet > >> * all receivers of packets need to cope with being given a > >> short packet; this is usually going to mean "flag it to the > >> guest as an RX error", but exact behaviour is NIC-dependent > >> > > My patch series in RFC v2/v3 does almost exactly this option (1), > > except "flag it to the guest as an RX error". > > > Is it? You did it at net core instead of netdevs if I read the code > correctly. > Literally I don't see Peter requested option (1) to be done in net core or net devs. If doing it in netdevs, the following codes need to be duplicated in both SLiRP and TAP codes. if (sender->info->type == NET_CLIENT_DRIVER_USER || sender->info->type == NET_CLIENT_DRIVER_TAP) { do the short frames padding; } > > > > >> Option (2) is: > >> * the net core code pads any packet that goes through it > >> * no NIC implementation needs to pad on TX (it is harmless if they do) > >> * non-NIC sources don't need to pad on TX > >> * no receivers of packets need to cope with being given short packets > >> > >> Option 1 is what the real world does. Option 2 is a simplification > >> which throws away the ability to emulate handling of short packets, > >> in exchange for not having to sort out senders like slirp and not > >> having to be so careful about short-packet handling in NIC models. > >> > >> If MST is correct that some use cases require short-packet support, > >> then we need to go for option 1, I think. > > > For NIC RX, I wonder whether we can introduce a boolean to whether it > requries padding. And then the netdevs can only do the padding when it's > required. E.g virito-net doesn't need padding. > Regards, Bin
On 2021/3/12 2:28 下午, Bin Meng wrote: > On Fri, Mar 12, 2021 at 2:23 PM Jason Wang <jasowang@redhat.com> wrote: >> >> On 2021/3/11 6:27 下午, Bin Meng wrote: >>> On Thu, Mar 11, 2021 at 6:22 PM Peter Maydell <peter.maydell@linaro.org> wrote: >>>> On Thu, 11 Mar 2021 at 09:58, Bin Meng <bmeng.cn@gmail.com> wrote: >>>>> On Thu, Mar 11, 2021 at 5:43 PM Peter Maydell <peter.maydell@linaro.org> wrote: >>>>>> On Thu, 11 Mar 2021 at 03:01, Jason Wang <jasowang@redhat.com> wrote: >>>>>>> And after a discussion 10 years ago [1]. Michael (cced) seems to want to >>>>>>> keep the padding logic in the NIC itself (probably with a generic helper >>>>>>> in the core). Since 1) the padding is only required for ethernet 2) >>>>>>> virito-net doesn't need that (it can pass incomplete packet by design). >>>>>> Like I said, we need to decide; either: >>>>>> >>>>>> (1) we do want to support short packets in the net core: >>>>>> every sender needs to either pad, or to have some flag to say >>>>>> "my implementation can't pad, please can the net core do it for me", >>>>>> unless they are deliberately sending a short packet. Every >>>>>> receiver needs to be able to cope with short packets, at least >>>>>> in the sense of not crashing (they should report them as a rx >>>>>> error if they have that kind of error reporting status register). >>>>>> I think we have mostly implemented our NIC models this way. >>>>>> >>>>>> (2) we simply don't support short packets in the net core: >>>>>> nobody (not NICs, not network backends) needs to pad, because >>>>>> they can rely on the core to do it. Some existing senders and >>>>>> receivers may have now-dead code to do their own padding which >>>>>> could be removed. >>>>>> >>>>>> MST is advocating for (1) in that old thread. That's a coherent >>>>>> position. >>>>> But it's a wrong approach. As Edgar and Stefan also said in the old >>>>> discussion thread, padding in the RX is wrong as real world NICs don't >>>>> do this. >>>> Neither option (1) nor option (2) involve padding in RX. >>> Correct. What I referred to is the current approach used in many NIC >>> modes, which is wrong, and we have to correct this. >>> >>>> Option (1) is: >>>> * no NIC implementation pads on TX, except as defined >>>> by whatever NIC-specific config registers or h/w behaviour >>>> might require (ie if the guest wants to send a short packet >>>> it can do that) >>>> * non-NIC sources like slirp need to pad on TX unless they're >>>> deliberately trying to send a short packet >>>> * all receivers of packets need to cope with being given a >>>> short packet; this is usually going to mean "flag it to the >>>> guest as an RX error", but exact behaviour is NIC-dependent >>>> >>> My patch series in RFC v2/v3 does almost exactly this option (1), >>> except "flag it to the guest as an RX error". >> >> Is it? You did it at net core instead of netdevs if I read the code >> correctly. >> > Literally I don't see Peter requested option (1) to be done in net > core or net devs. > > If doing it in netdevs, the following codes need to be duplicated in > both SLiRP and TAP codes. > > if (sender->info->type == NET_CLIENT_DRIVER_USER || > sender->info->type == NET_CLIENT_DRIVER_TAP) { > do the short frames padding; > } So my understanding is that it's better to be done at netdev where we know whether it's a ethernet dev, core should be protocol independent. Thanks > >>>> Option (2) is: >>>> * the net core code pads any packet that goes through it >>>> * no NIC implementation needs to pad on TX (it is harmless if they do) >>>> * non-NIC sources don't need to pad on TX >>>> * no receivers of packets need to cope with being given short packets >>>> >>>> Option 1 is what the real world does. Option 2 is a simplification >>>> which throws away the ability to emulate handling of short packets, >>>> in exchange for not having to sort out senders like slirp and not >>>> having to be so careful about short-packet handling in NIC models. >>>> >>>> If MST is correct that some use cases require short-packet support, >>>> then we need to go for option 1, I think. >> >> For NIC RX, I wonder whether we can introduce a boolean to whether it >> requries padding. And then the netdevs can only do the padding when it's >> required. E.g virito-net doesn't need padding. >> > Regards, > Bin >
On Fri, Mar 12, 2021 at 2:50 PM Jason Wang <jasowang@redhat.com> wrote: > > > On 2021/3/12 2:28 下午, Bin Meng wrote: > > On Fri, Mar 12, 2021 at 2:23 PM Jason Wang <jasowang@redhat.com> wrote: > >> > >> On 2021/3/11 6:27 下午, Bin Meng wrote: > >>> On Thu, Mar 11, 2021 at 6:22 PM Peter Maydell <peter.maydell@linaro.org> wrote: > >>>> On Thu, 11 Mar 2021 at 09:58, Bin Meng <bmeng.cn@gmail.com> wrote: > >>>>> On Thu, Mar 11, 2021 at 5:43 PM Peter Maydell <peter.maydell@linaro.org> wrote: > >>>>>> On Thu, 11 Mar 2021 at 03:01, Jason Wang <jasowang@redhat.com> wrote: > >>>>>>> And after a discussion 10 years ago [1]. Michael (cced) seems to want to > >>>>>>> keep the padding logic in the NIC itself (probably with a generic helper > >>>>>>> in the core). Since 1) the padding is only required for ethernet 2) > >>>>>>> virito-net doesn't need that (it can pass incomplete packet by design). > >>>>>> Like I said, we need to decide; either: > >>>>>> > >>>>>> (1) we do want to support short packets in the net core: > >>>>>> every sender needs to either pad, or to have some flag to say > >>>>>> "my implementation can't pad, please can the net core do it for me", > >>>>>> unless they are deliberately sending a short packet. Every > >>>>>> receiver needs to be able to cope with short packets, at least > >>>>>> in the sense of not crashing (they should report them as a rx > >>>>>> error if they have that kind of error reporting status register). > >>>>>> I think we have mostly implemented our NIC models this way. > >>>>>> > >>>>>> (2) we simply don't support short packets in the net core: > >>>>>> nobody (not NICs, not network backends) needs to pad, because > >>>>>> they can rely on the core to do it. Some existing senders and > >>>>>> receivers may have now-dead code to do their own padding which > >>>>>> could be removed. > >>>>>> > >>>>>> MST is advocating for (1) in that old thread. That's a coherent > >>>>>> position. > >>>>> But it's a wrong approach. As Edgar and Stefan also said in the old > >>>>> discussion thread, padding in the RX is wrong as real world NICs don't > >>>>> do this. > >>>> Neither option (1) nor option (2) involve padding in RX. > >>> Correct. What I referred to is the current approach used in many NIC > >>> modes, which is wrong, and we have to correct this. > >>> > >>>> Option (1) is: > >>>> * no NIC implementation pads on TX, except as defined > >>>> by whatever NIC-specific config registers or h/w behaviour > >>>> might require (ie if the guest wants to send a short packet > >>>> it can do that) > >>>> * non-NIC sources like slirp need to pad on TX unless they're > >>>> deliberately trying to send a short packet > >>>> * all receivers of packets need to cope with being given a > >>>> short packet; this is usually going to mean "flag it to the > >>>> guest as an RX error", but exact behaviour is NIC-dependent > >>>> > >>> My patch series in RFC v2/v3 does almost exactly this option (1), > >>> except "flag it to the guest as an RX error". > >> > >> Is it? You did it at net core instead of netdevs if I read the code > >> correctly. > >> > > Literally I don't see Peter requested option (1) to be done in net > > core or net devs. > > > > If doing it in netdevs, the following codes need to be duplicated in > > both SLiRP and TAP codes. > > > > if (sender->info->type == NET_CLIENT_DRIVER_USER || > > sender->info->type == NET_CLIENT_DRIVER_TAP) { > > do the short frames padding; > > } > > > So my understanding is that it's better to be done at netdev where we > know whether it's a ethernet dev, core should be protocol independent. OK, will change to pad short frames in SLiRP and TAP codes in the next version. Regards, Bin
On 2021/3/12 2:53 下午, Bin Meng wrote: > On Fri, Mar 12, 2021 at 2:50 PM Jason Wang <jasowang@redhat.com> wrote: >> >> On 2021/3/12 2:28 下午, Bin Meng wrote: >>> On Fri, Mar 12, 2021 at 2:23 PM Jason Wang <jasowang@redhat.com> wrote: >>>> On 2021/3/11 6:27 下午, Bin Meng wrote: >>>>> On Thu, Mar 11, 2021 at 6:22 PM Peter Maydell <peter.maydell@linaro.org> wrote: >>>>>> On Thu, 11 Mar 2021 at 09:58, Bin Meng <bmeng.cn@gmail.com> wrote: >>>>>>> On Thu, Mar 11, 2021 at 5:43 PM Peter Maydell <peter.maydell@linaro.org> wrote: >>>>>>>> On Thu, 11 Mar 2021 at 03:01, Jason Wang <jasowang@redhat.com> wrote: >>>>>>>>> And after a discussion 10 years ago [1]. Michael (cced) seems to want to >>>>>>>>> keep the padding logic in the NIC itself (probably with a generic helper >>>>>>>>> in the core). Since 1) the padding is only required for ethernet 2) >>>>>>>>> virito-net doesn't need that (it can pass incomplete packet by design). >>>>>>>> Like I said, we need to decide; either: >>>>>>>> >>>>>>>> (1) we do want to support short packets in the net core: >>>>>>>> every sender needs to either pad, or to have some flag to say >>>>>>>> "my implementation can't pad, please can the net core do it for me", >>>>>>>> unless they are deliberately sending a short packet. Every >>>>>>>> receiver needs to be able to cope with short packets, at least >>>>>>>> in the sense of not crashing (they should report them as a rx >>>>>>>> error if they have that kind of error reporting status register). >>>>>>>> I think we have mostly implemented our NIC models this way. >>>>>>>> >>>>>>>> (2) we simply don't support short packets in the net core: >>>>>>>> nobody (not NICs, not network backends) needs to pad, because >>>>>>>> they can rely on the core to do it. Some existing senders and >>>>>>>> receivers may have now-dead code to do their own padding which >>>>>>>> could be removed. >>>>>>>> >>>>>>>> MST is advocating for (1) in that old thread. That's a coherent >>>>>>>> position. >>>>>>> But it's a wrong approach. As Edgar and Stefan also said in the old >>>>>>> discussion thread, padding in the RX is wrong as real world NICs don't >>>>>>> do this. >>>>>> Neither option (1) nor option (2) involve padding in RX. >>>>> Correct. What I referred to is the current approach used in many NIC >>>>> modes, which is wrong, and we have to correct this. >>>>> >>>>>> Option (1) is: >>>>>> * no NIC implementation pads on TX, except as defined >>>>>> by whatever NIC-specific config registers or h/w behaviour >>>>>> might require (ie if the guest wants to send a short packet >>>>>> it can do that) >>>>>> * non-NIC sources like slirp need to pad on TX unless they're >>>>>> deliberately trying to send a short packet >>>>>> * all receivers of packets need to cope with being given a >>>>>> short packet; this is usually going to mean "flag it to the >>>>>> guest as an RX error", but exact behaviour is NIC-dependent >>>>>> >>>>> My patch series in RFC v2/v3 does almost exactly this option (1), >>>>> except "flag it to the guest as an RX error". >>>> Is it? You did it at net core instead of netdevs if I read the code >>>> correctly. >>>> >>> Literally I don't see Peter requested option (1) to be done in net >>> core or net devs. >>> >>> If doing it in netdevs, the following codes need to be duplicated in >>> both SLiRP and TAP codes. >>> >>> if (sender->info->type == NET_CLIENT_DRIVER_USER || >>> sender->info->type == NET_CLIENT_DRIVER_TAP) { >>> do the short frames padding; >>> } >> >> So my understanding is that it's better to be done at netdev where we >> know whether it's a ethernet dev, core should be protocol independent. > OK, will change to pad short frames in SLiRP and TAP codes in the next version. > > Regards, > Bin And we probably need a need_padding in the NetClientState, and only do the padding if it was required by the peer. Then we can say netdevs and virtio-net doesn't need padding. Thanks
diff --git a/include/net/eth.h b/include/net/eth.h index 0671be69165..7c825ecb2f7 100644 --- a/include/net/eth.h +++ b/include/net/eth.h @@ -31,6 +31,7 @@ #define ETH_ALEN 6 #define ETH_HLEN 14 +#define ETH_ZLEN 60 /* Min. octets in frame sans FCS */ struct eth_header { uint8_t h_dest[ETH_ALEN]; /* destination eth addr */ diff --git a/net/net.c b/net/net.c index 159e4d0ec25..d42ac9365eb 100644 --- a/net/net.c +++ b/net/net.c @@ -620,6 +620,7 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender, const uint8_t *buf, int size, NetPacketSent *sent_cb) { + static const uint8_t null_buf[ETH_ZLEN] = { }; NetQueue *queue; int ret; int iovcnt = 1; @@ -628,6 +629,10 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender, .iov_base = (void *)buf, .iov_len = size, }, + [1] = { + .iov_base = (void *)null_buf, + .iov_len = ETH_ZLEN, + }, }; #ifdef DEBUG_NET @@ -639,6 +644,15 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender, return size; } + /* Pad to minimum Ethernet frame length for SLiRP and TAP */ + if (sender->info->type == NET_CLIENT_DRIVER_USER || + sender->info->type == NET_CLIENT_DRIVER_TAP) { + if (size < ETH_ZLEN) { + iov[1].iov_len = ETH_ZLEN - size; + iovcnt = 2; + } + } + /* Let filters handle the packet first */ ret = filter_receive_iov(sender, NET_FILTER_DIRECTION_TX, sender, flags, iov, iovcnt, sent_cb);