Re: question about netif_rx
On Wed, 14 Aug 2013, Rami Rosen wrote: > Hi, > BTW, this is not the only NAPI issue here. When looking into cleanup > of resources in this driver, a call to netif_napi_del() is missing > (though there is a call to napi_disable(), which is not enough for > proper cleanup). Actually, netif_napi_del seems to be quite underused. At least approximately, I would assume that every file that uses netif_napi_add should use netif_napi_del at least once, and possible more times. But I find 118 .c files that use netif_napi_add, and only 35 that use netif_napi_del. Are there some cases where it might not be needed? julia -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: question about netif_rx
Hi, BTW, this is not the only NAPI issue here. When looking into cleanup of resources in this driver, a call to netif_napi_del() is missing (though there is a call to napi_disable(), which is not enough for proper cleanup). Best, Rami Rosen http://ramirose.wix.com/ramirosen On Tue, Aug 13, 2013 at 11:41 PM, Francois Romieu wrote: > (no top-post nor lazy quote please) > > David Shwatrz : > [...] >> In the napi_gro_receive() we check that the device supports >> NETIF_F_GRO, but I don't see that we inspect checksum or that >> NETIF_F_GRO is depends on checksum. > > napi_gro_receive is irrelevant. Let aside tunnel, the real work happens > in the protocol specific gro_receive handlers. > > However I am an happy retard and I missed that tcp gro stopped depending > on Rx checksum since commit 861b650101eb0c627d171eb18de81dddb93d395e. :o/ > > So, yes, napi_gro_receive could be used. > > -- > Ueimor > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: question about netif_rx
(no top-post nor lazy quote please) David Shwatrz : [...] > In the napi_gro_receive() we check that the device supports > NETIF_F_GRO, but I don't see that we inspect checksum or that > NETIF_F_GRO is depends on checksum. napi_gro_receive is irrelevant. Let aside tunnel, the real work happens in the protocol specific gro_receive handlers. However I am an happy retard and I missed that tcp gro stopped depending on Rx checksum since commit 861b650101eb0c627d171eb18de81dddb93d395e. :o/ So, yes, napi_gro_receive could be used. -- Ueimor -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: question about netif_rx
Hello, Sorry. I still don't understand what checksum has to do with it. Does GRO depends on Rx/Tx checksum ? I don't think so. In the napi_gro_receive() we check that the device supports NETIF_F_GRO, but I don't see that we inspect checksum or that NETIF_F_GRO is depends on checksum. Can you please explain how checsum offload is related ? rgs David On Tue, Aug 13, 2013 at 9:29 AM, Julia Lawall wrote: > On Tue, 13 Aug 2013, Francois Romieu wrote: > >> Julia Lawall : >> > François Romieu : >> [...] >> > > Can you send a netif_receive_skb replacement patch for it ? >> > >> > Just to be sure, I just replace netif_rx by netif_receive_skb, nothing >> > else? >> >> Yes. It should imho be fine with a comment incluing your analysis and a >> few words about the current state of checksum offloading support. > > I wouldn't know what to say about the checksum part. It is not supported > so I should use netif_receive_skb? > > thanks, > julia -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: question about netif_rx
On Tue, 13 Aug 2013, Francois Romieu wrote: > Julia Lawall : > > François Romieu : > [...] > > > Can you send a netif_receive_skb replacement patch for it ? > > > > Just to be sure, I just replace netif_rx by netif_receive_skb, nothing > > else? > > Yes. It should imho be fine with a comment incluing your analysis and a > few words about the current state of checksum offloading support. I wouldn't know what to say about the checksum part. It is not supported so I should use netif_receive_skb? thanks, julia
Re: question about netif_rx
David Shwatrz : [...] > what is the current state of checksum offloading support has to do > with it ? maybe you meant current state of NAPI support ? netif_receive_skb vs napi_gro_receive choice. -- Ueimor -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: question about netif_rx
Hello, what is the current state of checksum offloading support has to do with it ? maybe you meant current state of NAPI support ? regards, DS On Tue, Aug 13, 2013 at 8:20 AM, Francois Romieu wrote: > Julia Lawall : >> François Romieu : > [...] >> > Can you send a netif_receive_skb replacement patch for it ? >> >> Just to be sure, I just replace netif_rx by netif_receive_skb, nothing >> else? > > Yes. It should imho be fine with a comment incluing your analysis and a > few words about the current state of checksum offloading support. > > -- > Ueimor > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: question about netif_rx
Julia Lawall : > François Romieu : [...] > > Can you send a netif_receive_skb replacement patch for it ? > > Just to be sure, I just replace netif_rx by netif_receive_skb, nothing > else? Yes. It should imho be fine with a comment incluing your analysis and a few words about the current state of checksum offloading support. -- Ueimor -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: question about netif_rx
On Sun, 11 Aug 2013, Francois Romieu wrote: > Julia Lawall : > > To my limited understanding, in a NAPI polling function, one should use > > netif_receive_skb, rather than netif_rx. > > Nit: or napi_gro_receive (+ napi_gro_flush with __napi_complete) when the > device offers some checksum offloading features. OK, thanks for the information. I am just trying to understand these functions... > > However, the via-velocity driver defines the NAPI polling function > > velocity_poll, which is the only caller of velocity_rx_srv, which > > is the only caller of velocity_receive_frame, which calls netif_rx. > > The call to netif_rx seems to predate the introduction of NAPI in > > this driver. Is this correct? > > You are right. It's a leftover of the NAPI changes in this driver. > > Can you send a netif_receive_skb replacement patch for it ? Just to be sure, I just replace netif_rx by netif_receive_skb, nothing else? thanks, julia -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: question about netif_rx
Julia Lawall : > To my limited understanding, in a NAPI polling function, one should use > netif_receive_skb, rather than netif_rx. Nit: or napi_gro_receive (+ napi_gro_flush with __napi_complete) when the device offers some checksum offloading features. > However, the via-velocity driver defines the NAPI polling function > velocity_poll, which is the only caller of velocity_rx_srv, which > is the only caller of velocity_receive_frame, which calls netif_rx. > The call to netif_rx seems to predate the introduction of NAPI in > this driver. Is this correct? You are right. It's a leftover of the NAPI changes in this driver. Can you send a netif_receive_skb replacement patch for it ? -- Ueimor -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
