| Message ID | 20180906040526.22518-6-jasowang@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Vhost_net TX batching | expand |
On Thu, Sep 06, 2018 at 12:05:20PM +0800, Jason Wang wrote: > If we're sure not to go native XDP, there's no need for several things > like bh and rcu stuffs. > > Signed-off-by: Jason Wang <jasowang@redhat.com> True... > --- > drivers/net/tun.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > index f8cdcfa392c3..389aa0727cc6 100644 > --- a/drivers/net/tun.c > +++ b/drivers/net/tun.c > @@ -1675,10 +1675,12 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun, > * of xdp_prog above, this should be rare and for simplicity > * we do XDP on skb in case the headroom is not enough. > */ > - if (hdr->gso_type || !xdp_prog) > + if (hdr->gso_type || !xdp_prog) { > *skb_xdp = 1; > - else > - *skb_xdp = 0; > + goto build; > + } > + > + *skb_xdp = 0; > > local_bh_disable(); > rcu_read_lock(); > @@ -1724,6 +1726,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun, > rcu_read_unlock(); > local_bh_enable(); > > +build: But this is spaghetti code. Please just put common code into functions and call them, don't goto. > skb = build_skb(buf, buflen); > if (!skb) { > skb = ERR_PTR(-ENOMEM); > -- > 2.17.1
On 2018年09月07日 01:16, Michael S. Tsirkin wrote: > On Thu, Sep 06, 2018 at 12:05:20PM +0800, Jason Wang wrote: >> If we're sure not to go native XDP, there's no need for several things >> like bh and rcu stuffs. >> >> Signed-off-by: Jason Wang <jasowang@redhat.com> > True... > >> --- >> drivers/net/tun.c | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/tun.c b/drivers/net/tun.c >> index f8cdcfa392c3..389aa0727cc6 100644 >> --- a/drivers/net/tun.c >> +++ b/drivers/net/tun.c >> @@ -1675,10 +1675,12 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun, >> * of xdp_prog above, this should be rare and for simplicity >> * we do XDP on skb in case the headroom is not enough. >> */ >> - if (hdr->gso_type || !xdp_prog) >> + if (hdr->gso_type || !xdp_prog) { >> *skb_xdp = 1; >> - else >> - *skb_xdp = 0; >> + goto build; >> + } >> + >> + *skb_xdp = 0; >> >> local_bh_disable(); >> rcu_read_lock(); >> @@ -1724,6 +1726,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun, >> rcu_read_unlock(); >> local_bh_enable(); >> >> +build: > But this is spaghetti code. Please just put common > code into functions and call them, don't goto. Ok, will do this. Thanks > >> skb = build_skb(buf, buflen); >> if (!skb) { >> skb = ERR_PTR(-ENOMEM); >> -- >> 2.17.1
diff --git a/drivers/net/tun.c b/drivers/net/tun.c index f8cdcfa392c3..389aa0727cc6 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -1675,10 +1675,12 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun, * of xdp_prog above, this should be rare and for simplicity * we do XDP on skb in case the headroom is not enough. */ - if (hdr->gso_type || !xdp_prog) + if (hdr->gso_type || !xdp_prog) { *skb_xdp = 1; - else - *skb_xdp = 0; + goto build; + } + + *skb_xdp = 0; local_bh_disable(); rcu_read_lock(); @@ -1724,6 +1726,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun, rcu_read_unlock(); local_bh_enable(); +build: skb = build_skb(buf, buflen); if (!skb) { skb = ERR_PTR(-ENOMEM);
If we're sure not to go native XDP, there's no need for several things like bh and rcu stuffs. Signed-off-by: Jason Wang <jasowang@redhat.com> --- drivers/net/tun.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)