| Message ID | 1465561289-15401-1-git-send-email-fw@strlen.de |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, 2016-06-10 at 14:21 +0200, Florian Westphal wrote: > ... its not returned anywhere. > > BATMAN uses it as an intermediate return value to signal > forwarding vs. buffering, but it will not return POLICED to > callers outside of BATMAN. > > sch_atm uses it, but on closer look its not returned either since > the initialisation is always overwritten with qdisc_enqueue() retval. > > Signed-off-by: Florian Westphal <fw@strlen.de> > --- Reviewed-by: Eric Dumazet <edumazet@google.com> I have a patch series removing couples of qdisc_unthrottled() and qdisc_throttled() which are pretty expensive for very little gain.
On Friday 10 June 2016 14:21:29 Florian Westphal wrote: [...] > BATMAN uses it as an intermediate return value to signal > forwarding vs. buffering, but it will not return POLICED to > callers outside of BATMAN. [...] > diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c > index f2f1256..b1a4e8a 100644 > --- a/net/batman-adv/send.c > +++ b/net/batman-adv/send.c > @@ -156,7 +156,7 @@ int batadv_send_unicast_skb(struct sk_buff *skb, > * attempted. > * > * Return: NET_XMIT_SUCCESS on success, NET_XMIT_DROP on failure, or > - * NET_XMIT_POLICED if the skb is buffered for later transmit. > + * -EINPROGRESS if the skb is buffered for later transmit. > */ > int batadv_send_skb_to_orig(struct sk_buff *skb, > struct batadv_orig_node *orig_node, > @@ -188,7 +188,7 @@ int batadv_send_skb_to_orig(struct sk_buff *skb, > * network coding fails, then send the packet as usual. > */ > if (recv_if && batadv_nc_skb_forward(skb, neigh_node)) { > - ret = NET_XMIT_POLICED; > + ret = -EINPROGRESS; > } else { > batadv_send_unicast_skb(skb, neigh_node); > ret = NET_XMIT_SUCCESS; Looks good to me. But it has a minor conflict with a queued up patch for David (not yet submitted by Antonio). This is not real problem for you but it looks like there is something wrong with the queued up patch. I have submitted a fix for it and Antonio should just know that it is related to this one. But to your patch: Reviewed-by: Sven Eckelmann <sven@narfation.org> Kind regards, Sven
From: Florian Westphal <fw@strlen.de> Date: Fri, 10 Jun 2016 14:21:29 +0200 > ... its not returned anywhere. > > BATMAN uses it as an intermediate return value to signal > forwarding vs. buffering, but it will not return POLICED to > callers outside of BATMAN. > > sch_atm uses it, but on closer look its not returned either since > the initialisation is always overwritten with qdisc_enqueue() retval. > > Signed-off-by: Florian Westphal <fw@strlen.de> Applied, thanks.
From: David Miller <davem@davemloft.net> Date: Fri, 10 Jun 2016 23:31:37 -0700 (PDT) > From: Florian Westphal <fw@strlen.de> > Date: Fri, 10 Jun 2016 14:21:29 +0200 > >> ... its not returned anywhere. >> >> BATMAN uses it as an intermediate return value to signal >> forwarding vs. buffering, but it will not return POLICED to >> callers outside of BATMAN. >> >> sch_atm uses it, but on closer look its not returned either since >> the initialisation is always overwritten with qdisc_enqueue() retval. >> >> Signed-off-by: Florian Westphal <fw@strlen.de> > > Applied, thanks. I have to revert this, the sch_atm.c change is wrong. NET_XMIT_POLICED is the default value of 'ret' which can be used if we branch to 'drop'. So now GCC says that 'ret' might be used uninitialized. So it is actually used, and reported by sch_atm when TC_ACT_SHOT classifications occur.
David Miller <davem@davemloft.net> wrote: > I have to revert this, the sch_atm.c change is wrong. > > NET_XMIT_POLICED is the default value of 'ret' which can be used > if we branch to 'drop'. So now GCC says that 'ret' might be used > uninitialized. Hmpf, seems I need to update gcc :-| > So it is actually used, and reported by sch_atm when TC_ACT_SHOT > classifications occur. I'll resubmit a v2 changing initializer to NET_XMIT_DROP.
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 5415623..745a408 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -90,7 +90,6 @@ void netdev_set_default_ethtool_ops(struct net_device *dev, #define NET_XMIT_SUCCESS 0x00 #define NET_XMIT_DROP 0x01 /* skb dropped */ #define NET_XMIT_CN 0x02 /* congestion notification */ -#define NET_XMIT_POLICED 0x03 /* skb is shot by police */ #define NET_XMIT_MASK 0x0f /* qdisc flags in net/sch_generic.h */ /* NET_XMIT_CN is special. It does not guarantee that this packet is lost. It diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c index e3857ed..f75091c 100644 --- a/net/batman-adv/routing.c +++ b/net/batman-adv/routing.c @@ -653,7 +653,7 @@ static int batadv_route_unicast_packet(struct sk_buff *skb, len + ETH_HLEN); ret = NET_RX_SUCCESS; - } else if (res == NET_XMIT_POLICED) { + } else if (res == -EINPROGRESS) { /* skb was buffered and consumed */ ret = NET_RX_SUCCESS; } diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c index f2f1256..b1a4e8a 100644 --- a/net/batman-adv/send.c +++ b/net/batman-adv/send.c @@ -156,7 +156,7 @@ int batadv_send_unicast_skb(struct sk_buff *skb, * attempted. * * Return: NET_XMIT_SUCCESS on success, NET_XMIT_DROP on failure, or - * NET_XMIT_POLICED if the skb is buffered for later transmit. + * -EINPROGRESS if the skb is buffered for later transmit. */ int batadv_send_skb_to_orig(struct sk_buff *skb, struct batadv_orig_node *orig_node, @@ -188,7 +188,7 @@ int batadv_send_skb_to_orig(struct sk_buff *skb, * network coding fails, then send the packet as usual. */ if (recv_if && batadv_nc_skb_forward(skb, neigh_node)) { - ret = NET_XMIT_POLICED; + ret = -EINPROGRESS; } else { batadv_send_unicast_skb(skb, neigh_node); ret = NET_XMIT_SUCCESS; diff --git a/net/core/pktgen.c b/net/core/pktgen.c index 8b02df0..f74ab9c 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -3463,7 +3463,6 @@ xmit_more: break; case NET_XMIT_DROP: case NET_XMIT_CN: - case NET_XMIT_POLICED: /* skb has been consumed */ pkt_dev->errors++; break; diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c index d4a8bbf..7460d56 100644 --- a/net/sched/sch_api.c +++ b/net/sched/sch_api.c @@ -95,8 +95,6 @@ static int tclass_notify(struct net *net, struct sk_buff *oskb, Expected action: do not backoff, but wait until queue will clear. NET_XMIT_CN - probably this packet enqueued, but another one dropped. Expected action: backoff or ignore - NET_XMIT_POLICED - dropped by police. - Expected action: backoff or error to real-time apps. Auxiliary routines: diff --git a/net/sched/sch_atm.c b/net/sched/sch_atm.c index 7e6c12d..ac298d3 100644 --- a/net/sched/sch_atm.c +++ b/net/sched/sch_atm.c @@ -362,8 +362,7 @@ static int atm_tc_enqueue(struct sk_buff *skb, struct Qdisc *sch) struct atm_qdisc_data *p = qdisc_priv(sch); struct atm_flow_data *flow; struct tcf_result res; - int result; - int ret = NET_XMIT_POLICED; + int result, ret; pr_debug("atm_tc_enqueue(skb %p,sch %p,[qdisc %p])\n", skb, sch, p); result = TC_POLICE_OK; /* be nice to gcc */
... its not returned anywhere. BATMAN uses it as an intermediate return value to signal forwarding vs. buffering, but it will not return POLICED to callers outside of BATMAN. sch_atm uses it, but on closer look its not returned either since the initialisation is always overwritten with qdisc_enqueue() retval. Signed-off-by: Florian Westphal <fw@strlen.de> --- include/linux/netdevice.h | 1 - net/batman-adv/routing.c | 2 +- net/batman-adv/send.c | 4 ++-- net/core/pktgen.c | 1 - net/sched/sch_api.c | 2 -- net/sched/sch_atm.c | 3 +-- 6 files changed, 4 insertions(+), 9 deletions(-)