* [PATCH] tuntap: fix error return code in tun_set_iff() @ 2013-04-12 13:17 Wei Yongjun 2013-04-12 13:49 ` Eric Dumazet 2013-04-23 5:37 ` Jerry Chu 0 siblings, 2 replies; 7+ messages in thread From: Wei Yongjun @ 2013-04-12 13:17 UTC (permalink / raw) To: davem, jasowang, mst, edumazet, nhorman; +Cc: yongjun_wei, netdev From: Wei Yongjun <yongjun_wei@trendmicro.com.cn> Fix to return a negative error code from the error handling case instead of 0, as returned elsewhere in this function. Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn> --- drivers/net/tun.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index b7c457a..729ed53 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -1594,7 +1594,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr) if (tun->flags & TUN_TAP_MQ && (tun->numqueues + tun->numdisabled > 1)) - return err; + return -EBUSY; } else { char *name; ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] tuntap: fix error return code in tun_set_iff() 2013-04-12 13:17 [PATCH] tuntap: fix error return code in tun_set_iff() Wei Yongjun @ 2013-04-12 13:49 ` Eric Dumazet 2013-04-12 19:00 ` David Miller 2013-04-23 5:37 ` Jerry Chu 1 sibling, 1 reply; 7+ messages in thread From: Eric Dumazet @ 2013-04-12 13:49 UTC (permalink / raw) To: Wei Yongjun; +Cc: davem, jasowang, mst, edumazet, nhorman, yongjun_wei, netdev On Fri, 2013-04-12 at 21:17 +0800, Wei Yongjun wrote: > From: Wei Yongjun <yongjun_wei@trendmicro.com.cn> > > Fix to return a negative error code from the error handling > case instead of 0, as returned elsewhere in this function. > > Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn> > --- > drivers/net/tun.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > index b7c457a..729ed53 100644 > --- a/drivers/net/tun.c > +++ b/drivers/net/tun.c > @@ -1594,7 +1594,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr) > > if (tun->flags & TUN_TAP_MQ && > (tun->numqueues + tun->numdisabled > 1)) > - return err; > + return -EBUSY; > } > else { > char *name; Bug added in linux-3.8 , commit 4008e97f866db665 ("tuntap: fix ambigious multiqueue API") Acked-by: Eric Dumazet <edumazet@google.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tuntap: fix error return code in tun_set_iff() 2013-04-12 13:49 ` Eric Dumazet @ 2013-04-12 19:00 ` David Miller 0 siblings, 0 replies; 7+ messages in thread From: David Miller @ 2013-04-12 19:00 UTC (permalink / raw) To: eric.dumazet Cc: weiyj.lk, jasowang, mst, edumazet, nhorman, yongjun_wei, netdev From: Eric Dumazet <eric.dumazet@gmail.com> Date: Fri, 12 Apr 2013 06:49:20 -0700 > On Fri, 2013-04-12 at 21:17 +0800, Wei Yongjun wrote: >> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn> >> >> Fix to return a negative error code from the error handling >> case instead of 0, as returned elsewhere in this function. >> >> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn> >> --- >> drivers/net/tun.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/tun.c b/drivers/net/tun.c >> index b7c457a..729ed53 100644 >> --- a/drivers/net/tun.c >> +++ b/drivers/net/tun.c >> @@ -1594,7 +1594,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr) >> >> if (tun->flags & TUN_TAP_MQ && >> (tun->numqueues + tun->numdisabled > 1)) >> - return err; >> + return -EBUSY; >> } >> else { >> char *name; > > Bug added in linux-3.8 , commit 4008e97f866db665 > ("tuntap: fix ambigious multiqueue API") > > Acked-by: Eric Dumazet <edumazet@google.com> Applied and queued up for -stable, thanks. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tuntap: fix error return code in tun_set_iff() 2013-04-12 13:17 [PATCH] tuntap: fix error return code in tun_set_iff() Wei Yongjun 2013-04-12 13:49 ` Eric Dumazet @ 2013-04-23 5:37 ` Jerry Chu 2013-04-23 6:19 ` Jason Wang 1 sibling, 1 reply; 7+ messages in thread From: Jerry Chu @ 2013-04-23 5:37 UTC (permalink / raw) To: Wei Yongjun Cc: David Miller, Jason Wang, mst, Eric Dumazet, nhorman, yongjun_wei, netdev@vger.kernel.org On Fri, Apr 12, 2013 at 6:17 AM, Wei Yongjun <weiyj.lk@gmail.com> wrote: > > From: Wei Yongjun <yongjun_wei@trendmicro.com.cn> > > Fix to return a negative error code from the error handling > case instead of 0, as returned elsewhere in this function. > > Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn> > --- > drivers/net/tun.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > index b7c457a..729ed53 100644 > --- a/drivers/net/tun.c > +++ b/drivers/net/tun.c > @@ -1594,7 +1594,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr) > > if (tun->flags & TUN_TAP_MQ && > (tun->numqueues + tun->numdisabled > 1)) > - return err; > + return -EBUSY; I don't understand - yes it was a brainless bug to return err without setting it! But won't the fix pretty much disable multi-q because only the the creation of the 1st queue will succeed? I thought the intent of "tuntap: fix ambigious multiqueue API" was to "Only allow TUNSETIFF to create queues.". The code is very confusing. (Or am I the one who is confused? Sigh.) Jerry > } > else { > char *name; > > -- > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tuntap: fix error return code in tun_set_iff() 2013-04-23 5:37 ` Jerry Chu @ 2013-04-23 6:19 ` Jason Wang 2013-04-23 6:59 ` Jerry Chu 0 siblings, 1 reply; 7+ messages in thread From: Jason Wang @ 2013-04-23 6:19 UTC (permalink / raw) To: Jerry Chu Cc: Wei Yongjun, David Miller, mst, Eric Dumazet, nhorman, yongjun_wei, netdev@vger.kernel.org On 04/23/2013 01:37 PM, Jerry Chu wrote: > On Fri, Apr 12, 2013 at 6:17 AM, Wei Yongjun <weiyj.lk@gmail.com> wrote: >> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn> >> >> Fix to return a negative error code from the error handling >> case instead of 0, as returned elsewhere in this function. >> >> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn> >> --- >> drivers/net/tun.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/tun.c b/drivers/net/tun.c >> index b7c457a..729ed53 100644 >> --- a/drivers/net/tun.c >> +++ b/drivers/net/tun.c >> @@ -1594,7 +1594,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr) >> >> if (tun->flags & TUN_TAP_MQ && >> (tun->numqueues + tun->numdisabled > 1)) >> - return err; >> + return -EBUSY; > I don't understand - yes it was a brainless bug to return err without > setting it! err was in fact set by tun_attach, so it was always zero here. The code works by chance w/o this patch :) > But won't the fix pretty much disable multi-q because only the the creation of > the 1st queue will succeed? I thought the intent of "tuntap: fix ambigious > multiqueue API" was to "Only allow TUNSETIFF to create queues.". Yes this patch will break the creation of more than 1 queues. > > The code is very confusing. (Or am I the one who is confused? Sigh.) -EBUSY is wrong here, we need return 0 for succeed here. The logic is, if we have more than 1 queues attached, no need to re-initialize the net device again. Will send patch to correct this. Thanks. > > Jerry > >> } >> else { >> char *name; >> >> -- >> 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 > -- > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tuntap: fix error return code in tun_set_iff() 2013-04-23 6:19 ` Jason Wang @ 2013-04-23 6:59 ` Jerry Chu 2013-04-23 7:39 ` Jason Wang 0 siblings, 1 reply; 7+ messages in thread From: Jerry Chu @ 2013-04-23 6:59 UTC (permalink / raw) To: Jason Wang Cc: Wei Yongjun, David Miller, mst, Eric Dumazet, nhorman, yongjun_wei, netdev@vger.kernel.org On Mon, Apr 22, 2013 at 11:19 PM, Jason Wang <jasowang@redhat.com> wrote: > > On 04/23/2013 01:37 PM, Jerry Chu wrote: > > On Fri, Apr 12, 2013 at 6:17 AM, Wei Yongjun <weiyj.lk@gmail.com> wrote: > >> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn> > >> > >> Fix to return a negative error code from the error handling > >> case instead of 0, as returned elsewhere in this function. > >> > >> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn> > >> --- > >> drivers/net/tun.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/net/tun.c b/drivers/net/tun.c > >> index b7c457a..729ed53 100644 > >> --- a/drivers/net/tun.c > >> +++ b/drivers/net/tun.c > >> @@ -1594,7 +1594,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr) > >> > >> if (tun->flags & TUN_TAP_MQ && > >> (tun->numqueues + tun->numdisabled > 1)) > >> - return err; > >> + return -EBUSY; > > I don't understand - yes it was a brainless bug to return err without > > setting it! > > err was in fact set by tun_attach, so it was always zero here. The code > works by chance w/o this patch :) What if tun_attach() returns something > 0? > > But won't the fix pretty much disable multi-q because only the the creation of > > the 1st queue will succeed? I thought the intent of "tuntap: fix ambigious > > multiqueue API" was to "Only allow TUNSETIFF to create queues.". > > Yes this patch will break the creation of more than 1 queues. > > > > The code is very confusing. (Or am I the one who is confused? Sigh.) > > -EBUSY is wrong here, we need return 0 for succeed here. The logic is, > if we have more than 1 queues attached, no need to re-initialize the net > device again. Will send patch to correct this. A comment there will be useful! Thanks, Jerry > > Thanks. > > > > > Jerry > > > >> } > >> else { > >> char *name; > >> > >> -- > >> 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 > > -- > > 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tuntap: fix error return code in tun_set_iff() 2013-04-23 6:59 ` Jerry Chu @ 2013-04-23 7:39 ` Jason Wang 0 siblings, 0 replies; 7+ messages in thread From: Jason Wang @ 2013-04-23 7:39 UTC (permalink / raw) To: Jerry Chu Cc: Wei Yongjun, David Miller, mst, Eric Dumazet, nhorman, yongjun_wei, netdev@vger.kernel.org On 04/23/2013 02:59 PM, Jerry Chu wrote: > On Mon, Apr 22, 2013 at 11:19 PM, Jason Wang <jasowang@redhat.com> wrote: >> On 04/23/2013 01:37 PM, Jerry Chu wrote: >>> On Fri, Apr 12, 2013 at 6:17 AM, Wei Yongjun <weiyj.lk@gmail.com> wrote: >>>> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn> >>>> >>>> Fix to return a negative error code from the error handling >>>> case instead of 0, as returned elsewhere in this function. >>>> >>>> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn> >>>> --- >>>> drivers/net/tun.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c >>>> index b7c457a..729ed53 100644 >>>> --- a/drivers/net/tun.c >>>> +++ b/drivers/net/tun.c >>>> @@ -1594,7 +1594,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr) >>>> >>>> if (tun->flags & TUN_TAP_MQ && >>>> (tun->numqueues + tun->numdisabled > 1)) >>>> - return err; >>>> + return -EBUSY; >>> I don't understand - yes it was a brainless bug to return err without >>> setting it! >> err was in fact set by tun_attach, so it was always zero here. The code >> works by chance w/o this patch :) > What if tun_attach() returns something > 0? As far as I can see, it can't return something > 0. >>> But won't the fix pretty much disable multi-q because only the the creation of >>> the 1st queue will succeed? I thought the intent of "tuntap: fix ambigious >>> multiqueue API" was to "Only allow TUNSETIFF to create queues.". >> Yes this patch will break the creation of more than 1 queues. >>> The code is very confusing. (Or am I the one who is confused? Sigh.) >> -EBUSY is wrong here, we need return 0 for succeed here. The logic is, >> if we have more than 1 queues attached, no need to re-initialize the net >> device again. Will send patch to correct this. > A comment there will be useful! Yes, have added a comment. Thanks > > Thanks, > > Jerry > >> Thanks. >> >>> Jerry >>> >>>> } >>>> else { >>>> char *name; >>>> >>>> -- >>>> 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 >>> -- >>> 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 > -- > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-04-23 7:39 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-04-12 13:17 [PATCH] tuntap: fix error return code in tun_set_iff() Wei Yongjun 2013-04-12 13:49 ` Eric Dumazet 2013-04-12 19:00 ` David Miller 2013-04-23 5:37 ` Jerry Chu 2013-04-23 6:19 ` Jason Wang 2013-04-23 6:59 ` Jerry Chu 2013-04-23 7:39 ` Jason Wang
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).