> -----Original Message----- > From: devel [mailto:driverdev-devel-bounces@xxxxxxxxxxxxxxxxxxxxxx] On > Behalf Of Vitaly Kuznetsov > Sent: Friday, December 11, 2015 5:53 AM > To: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx> > Cc: olaf@xxxxxxxxx; netdev@xxxxxxxxxxxxxxx; jasowang@xxxxxxxxxx; > driverdev-devel@xxxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > davem@xxxxxxxxxxxxx > Subject: Re: [PATCH net-next] hv_netvsc: Fix race condition on Multi-Send > Data field > > Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> writes: > > > Haiyang Zhang <haiyangz@xxxxxxxxxxxxx> writes: > > > >> In commit 2a04ae8acb14 ("hv_netvsc: remove locking in netvsc_send()"), > the > >> locking for MSD (Multi-Send Data) field was removed. This could cause a > >> race condition between RNDIS control messages and data packets > processing, > >> because these two types of traffic are not synchronized. > >> This patch fixes this issue by sending control messages out directly > >> without reading MSD field. > >> > >> Signed-off-by: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx> > >> Reviewed-by: K. Y. Srinivasan <kys@xxxxxxxxxxxxx> > >> --- > >> drivers/net/hyperv/netvsc.c | 9 +++++++++ > >> 1 files changed, 9 insertions(+), 0 deletions(-) > >> > >> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c > >> index 02bab9a..059fc52 100644 > >> --- a/drivers/net/hyperv/netvsc.c > >> +++ b/drivers/net/hyperv/netvsc.c > >> @@ -867,6 +867,14 @@ int netvsc_send(struct hv_device *device, > >> packet->send_buf_index = NETVSC_INVALID_INDEX; > >> packet->cp_partial = false; > >> > >> + /* Send control message directly without accessing msd (Multi-Send > >> + * Data) field which may be changed during data packet processing. > >> + */ > >> + if (!skb) { > >> + cur_send = packet; > >> + goto send_now; > >> + } > >> + > > > > Is is supposed to be applied on top of some other patches? It doesn't > > compile on top of current net-next: > > > > drivers/net/hyperv/netvsc.c: In function ‘netvsc_send’: > > drivers/net/hyperv/netvsc.c:865:7: error: ‘skb’ undeclared (first use in > > this function) > > if (!skb) { > > ^ > > > > Did you mean to check rndis_msg instead (as skb is not defined here)? > > Oh, my bad, it was me who was looking at the wrong branch... Sorry for > the noise. > > > > >> msdp = &net_device->msd[q_idx]; > >> > >> /* batch packets in send buffer if possible */ > >> @@ -939,6 +947,7 @@ int netvsc_send(struct hv_device *device, > >> } > >> } > >> > >> +send_now: > >> if (cur_send) > >> ret = netvsc_send_pkt(cur_send, net_device, pb, skb); > > > > I suppose we untangle these two pathes completely: let > > rndis_filter_send_request() call netvsc_send_pkt() directly. Please see > > my patch attached (note: it should be split in 3 patches if > > submitted). If you like the idea I can send it. > > This patch will need some changes but the suggestion still stands: let's > untangle sending data and control messages. Control messages will have skb set to NULL and that is what Haiyang is doing to distinguish control packets. K. Y > > -- > Vitaly > _______________________________________________ > devel mailing list > devel@xxxxxxxxxxxxxxxxxxxxxx > https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fdriverde > v.linuxdriverproject.org%2fmailman%2flistinfo%2fdriverdev- > devel%0a&data=01%7c01%7ckys%40microsoft.com%7c741026ba8cbb47a993 > 2c08d3023257b7%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=VdQiM > NOzCtlRcsN5%2b%2faf%2bGrPNDHPmrvo2TuUs1T%2frlQ%3d _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
