| Message ID | 20150820070733.GA3711@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Ken-ichirou MATSUZAWA <chamaken@gmail.com> Date: Thu, 20 Aug 2015 16:07:33 +0900 > I don't know the intension of setting VALID status in the skb > destructor. But I think it need to be set UNUSED status in case of > error then release skb, or rx ring might be filled with RESERVED > frames. > > Signed-off-by: Ken-ichirou MATSUZAWA <chamas@h4.dion.ne.jp> I think the idea is to have the user process this "zero length" frame and advance the status itself. I think it is probably racy and problematic to have the kernel set a frame's state to UNUSED. It is not a valid state transition for the kernel side of RX ring processing. Only the user can safely release ring entries back to the kernel. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 25, 2015 at 08:22:03PM -0700, David Miller wrote: > From: Ken-ichirou MATSUZAWA <chamaken@gmail.com> > > I don't know the intension of setting VALID status in the skb > > destructor. But I think it need to be set UNUSED status in case of > > I think the idea is to have the user process this "zero length" frame > and advance the status itself. > > I think it is probably racy and problematic to have the kernel set a > frame's state to UNUSED. It is not a valid state transition for the > kernel side of RX ring processing. > > Only the user can safely release ring entries back to the kernel. I will just update the frame status to UNUSED and advance ring position in user space in case of nm_len == 0. Thank you. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index e6134f4..85ccd8b 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -864,7 +864,7 @@ static void netlink_skb_destructor(struct sk_buff *skb) } else { if (!(NETLINK_CB(skb).flags & NETLINK_SKB_DELIVERED)) { hdr->nm_len = 0; - netlink_set_status(hdr, NL_MMAP_STATUS_VALID); + netlink_set_status(hdr, NL_MMAP_STATUS_UNUSED); } ring = &nlk_sk(sk)->rx_ring; }
I don't know the intension of setting VALID status in the skb destructor. But I think it need to be set UNUSED status in case of error then release skb, or rx ring might be filled with RESERVED frames. Signed-off-by: Ken-ichirou MATSUZAWA <chamas@h4.dion.ne.jp> --- net/netlink/af_netlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)