* [PATCH] virtio_net: don't set NETIF_F_FRAGLIST
@ 2008-11-05 12:34 Mark McLoughlin
2008-11-05 21:53 ` Rusty Russell
2008-11-06 5:43 ` Jeff Garzik
0 siblings, 2 replies; 7+ messages in thread
From: Mark McLoughlin @ 2008-11-05 12:34 UTC (permalink / raw)
To: Rusty Russell; +Cc: netdev, Mark McLoughlin
We don't handle skb_shared_info->frag_list, so we shouldn't
be setting the NETIF_F_FRAGLIST flag.
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
drivers/net/virtio_net.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0196a0d..9d4fdad 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -505,7 +505,7 @@ static int virtnet_probe(struct virtio_device *vdev)
/* Do we support "hardware" checksums? */
if (csum && virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
/* This opens up the world of extra features. */
- dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
+ dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG;
if (gso && virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
dev->features |= NETIF_F_TSO | NETIF_F_UFO
| NETIF_F_TSO_ECN | NETIF_F_TSO6;
--
1.5.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] virtio_net: don't set NETIF_F_FRAGLIST
2008-11-05 12:34 [PATCH] virtio_net: don't set NETIF_F_FRAGLIST Mark McLoughlin
@ 2008-11-05 21:53 ` Rusty Russell
2008-11-06 10:10 ` Mark McLoughlin
2008-11-06 5:43 ` Jeff Garzik
1 sibling, 1 reply; 7+ messages in thread
From: Rusty Russell @ 2008-11-05 21:53 UTC (permalink / raw)
To: Mark McLoughlin; +Cc: netdev
On Wednesday 05 November 2008 23:34:46 Mark McLoughlin wrote:
> We don't handle skb_shared_info->frag_list, so we shouldn't
> be setting the NETIF_F_FRAGLIST flag.
We don't?
skb_to_sgvec() should handle it OK, is there some subtlety I'm missing?
Rusty.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] virtio_net: don't set NETIF_F_FRAGLIST
2008-11-05 21:53 ` Rusty Russell
@ 2008-11-06 10:10 ` Mark McLoughlin
0 siblings, 0 replies; 7+ messages in thread
From: Mark McLoughlin @ 2008-11-06 10:10 UTC (permalink / raw)
To: Rusty Russell; +Cc: netdev
On Thu, 2008-11-06 at 08:53 +1100, Rusty Russell wrote:
> On Wednesday 05 November 2008 23:34:46 Mark McLoughlin wrote:
> > We don't handle skb_shared_info->frag_list, so we shouldn't
> > be setting the NETIF_F_FRAGLIST flag.
>
> We don't?
>
> skb_to_sgvec() should handle it OK, is there some subtlety I'm missing?
Good grief ... yes, sorry.
Same is true for the tun patch, but I wonder ... could a fraglist skb
exceed 64k? i.e. we need to know the max possible size of a packet
before reading it from the tap.
Cheers,
Mark.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] virtio_net: don't set NETIF_F_FRAGLIST
2008-11-05 12:34 [PATCH] virtio_net: don't set NETIF_F_FRAGLIST Mark McLoughlin
2008-11-05 21:53 ` Rusty Russell
@ 2008-11-06 5:43 ` Jeff Garzik
2008-11-06 10:12 ` Mark McLoughlin
1 sibling, 1 reply; 7+ messages in thread
From: Jeff Garzik @ 2008-11-06 5:43 UTC (permalink / raw)
Cc: Rusty Russell, netdev, Mark McLoughlin
Mark McLoughlin wrote:
> We don't handle skb_shared_info->frag_list, so we shouldn't
> be setting the NETIF_F_FRAGLIST flag.
>
> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> ---
> drivers/net/virtio_net.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 0196a0d..9d4fdad 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -505,7 +505,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> /* Do we support "hardware" checksums? */
> if (csum && virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
> /* This opens up the world of extra features. */
> - dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
> + dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG;
> if (gso && virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
> dev->features |= NETIF_F_TSO | NETIF_F_UFO
> | NETIF_F_TSO_ECN | NETIF_F_TSO6;
applied this and tun patch
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] virtio_net: don't set NETIF_F_FRAGLIST
2008-11-06 5:43 ` Jeff Garzik
@ 2008-11-06 10:12 ` Mark McLoughlin
2008-11-06 10:48 ` Jeff Garzik
0 siblings, 1 reply; 7+ messages in thread
From: Mark McLoughlin @ 2008-11-06 10:12 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Rusty Russell, netdev
On Thu, 2008-11-06 at 00:43 -0500, Jeff Garzik wrote:
> Mark McLoughlin wrote:
> > We don't handle skb_shared_info->frag_list, so we shouldn't
> > be setting the NETIF_F_FRAGLIST flag.
> >
> > Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> > ---
> > drivers/net/virtio_net.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 0196a0d..9d4fdad 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -505,7 +505,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> > /* Do we support "hardware" checksums? */
> > if (csum && virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
> > /* This opens up the world of extra features. */
> > - dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
> > + dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG;
> > if (gso && virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
> > dev->features |= NETIF_F_TSO | NETIF_F_UFO
> > | NETIF_F_TSO_ECN | NETIF_F_TSO6;
>
> applied this and tun patch
Sorry Jeff, Rusty correctly points out that we do in fact handle
shinfo->frags; just a moment of blindness on my part.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] virtio_net: don't set NETIF_F_FRAGLIST
2008-11-06 10:12 ` Mark McLoughlin
@ 2008-11-06 10:48 ` Jeff Garzik
2008-11-06 10:56 ` Mark McLoughlin
0 siblings, 1 reply; 7+ messages in thread
From: Jeff Garzik @ 2008-11-06 10:48 UTC (permalink / raw)
To: Mark McLoughlin; +Cc: Rusty Russell, netdev
Mark McLoughlin wrote:
> On Thu, 2008-11-06 at 00:43 -0500, Jeff Garzik wrote:
>> Mark McLoughlin wrote:
>>> We don't handle skb_shared_info->frag_list, so we shouldn't
>>> be setting the NETIF_F_FRAGLIST flag.
>>>
>>> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
>>> ---
>>> drivers/net/virtio_net.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>>> index 0196a0d..9d4fdad 100644
>>> --- a/drivers/net/virtio_net.c
>>> +++ b/drivers/net/virtio_net.c
>>> @@ -505,7 +505,7 @@ static int virtnet_probe(struct virtio_device *vdev)
>>> /* Do we support "hardware" checksums? */
>>> if (csum && virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
>>> /* This opens up the world of extra features. */
>>> - dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
>>> + dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG;
>>> if (gso && virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
>>> dev->features |= NETIF_F_TSO | NETIF_F_UFO
>>> | NETIF_F_TSO_ECN | NETIF_F_TSO6;
>> applied this and tun patch
>
> Sorry Jeff, Rusty correctly points out that we do in fact handle
> shinfo->frags; just a moment of blindness on my part.
but what about frag_list ?
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] virtio_net: don't set NETIF_F_FRAGLIST
2008-11-06 10:48 ` Jeff Garzik
@ 2008-11-06 10:56 ` Mark McLoughlin
0 siblings, 0 replies; 7+ messages in thread
From: Mark McLoughlin @ 2008-11-06 10:56 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Rusty Russell, netdev
On Thu, 2008-11-06 at 05:48 -0500, Jeff Garzik wrote:
> Mark McLoughlin wrote:
> > On Thu, 2008-11-06 at 00:43 -0500, Jeff Garzik wrote:
> >> Mark McLoughlin wrote:
> >>> We don't handle skb_shared_info->frag_list, so we shouldn't
> >>> be setting the NETIF_F_FRAGLIST flag.
> >>>
> >>> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> >>> ---
> >>> drivers/net/virtio_net.c | 2 +-
> >>> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >>> index 0196a0d..9d4fdad 100644
> >>> --- a/drivers/net/virtio_net.c
> >>> +++ b/drivers/net/virtio_net.c
> >>> @@ -505,7 +505,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> >>> /* Do we support "hardware" checksums? */
> >>> if (csum && virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
> >>> /* This opens up the world of extra features. */
> >>> - dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
> >>> + dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG;
> >>> if (gso && virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
> >>> dev->features |= NETIF_F_TSO | NETIF_F_UFO
> >>> | NETIF_F_TSO_ECN | NETIF_F_TSO6;
> >> applied this and tun patch
> >
> > Sorry Jeff, Rusty correctly points out that we do in fact handle
> > shinfo->frags; just a moment of blindness on my part.
>
> but what about frag_list ?
Yeah, I meant shinfo->frag_list.
virtio_net uses skb_to_sgvec() and tun uses skb_copy_datagram_iovec()
Both handle shinfo->frag_list correctly.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-11-06 10:57 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-11-05 12:34 [PATCH] virtio_net: don't set NETIF_F_FRAGLIST Mark McLoughlin
2008-11-05 21:53 ` Rusty Russell
2008-11-06 10:10 ` Mark McLoughlin
2008-11-06 5:43 ` Jeff Garzik
2008-11-06 10:12 ` Mark McLoughlin
2008-11-06 10:48 ` Jeff Garzik
2008-11-06 10:56 ` Mark McLoughlin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).