Netdev List
 help / color / mirror / Atom feed
* Re: Philosophical question:  Is a UDP multicast datagram for which there is no socket match a drop or an ignore?
From: Rick Jones @ 2014-10-01  0:31 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1412123358.16704.22.camel@edumazet-glaptop2.roam.corp.google.com>

On 09/30/2014 05:29 PM, Eric Dumazet wrote:
> Yes, please submit your patch formally, maybe using a more complete
> form ? ;)

Will do.

rick

^ permalink raw reply

* Re: Philosophical question:  Is a UDP multicast datagram for which there is no socket match a drop or an ignore?
From: Eric Dumazet @ 2014-10-01  0:29 UTC (permalink / raw)
  To: Rick Jones; +Cc: netdev
In-Reply-To: <542B4950.8050301@hp.com>

On Tue, 2014-09-30 at 17:22 -0700, Rick Jones wrote:
> On 09/30/2014 04:23 PM, Eric Dumazet wrote:
> > On Tue, 2014-09-30 at 16:09 -0700, Rick Jones wrote:
> >> I've been looking at some additional perf <mutter> -e skb_kfree_skb
> >> results, this time with a laptop connected to a corporate network with a
> >> large number of Windows systems sending out what they are wont to
> >> send...  The laptop is just sitting there no active netperfs or anything :)
> >>
> >> I see profile hits for __udp4_lib_mcast_deliver() which has a
> >> kfree_skb() call which will happen if either there were no sockets
> >> found, or if an integral multiple of ARRAY_SIZE(stack) sockets are
> >> found.  I'm assuming the latter is exceedingly rare.
> >>
> >> Anywho, the philosophical question - is such a situation a drop
> >> (indicating the existing kfree_skb()), or is it an ignore (indicating a
> >> consume_skb())?  Should there be a statistic incremented for either of
> >> those?
> >
> > I guess we lack a UDP_MIB_NOPORTS increase here.
> 
> I was going back and forth on that - since it is a multicast it may not 
> have really been directed at us in which case it would be an ignore (and 
> perhaps a new "ignored" stat?).  But on the assumption that it should 
> indeed remain a drop, and so a kfree_skb(), something along the lines of:
> 

> 
> ?  The idea being that in the unlikely event there were indeed enough 
> matches to trigger the flush_stack in the for_each and only enough for 
> that it will be a consume_skb() and no statistic rather than a 
> kfree_skb() and a statistic increment.

Yes, please submit your patch formally, maybe using a more complete
form ? ;)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index cd0db5471bb5..be7db86046af 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1656,6 +1656,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
 	int dif = skb->dev->ifindex;
 	unsigned int count = 0, offset = offsetof(typeof(*sk), sk_nulls_node);
 	unsigned int hash2 = 0, hash2_any = 0, use_hash2 = (hslot->count > 10);
+	bool delivered = false;
 
 	if (use_hash2) {
 		hash2_any = udp4_portaddr_hash(net, htonl(INADDR_ANY), hnum) &
@@ -1674,6 +1675,7 @@ start_lookup:
 					dif, hnum)) {
 			if (unlikely(count == ARRAY_SIZE(stack))) {
 				flush_stack(stack, count, skb, ~0);
+				delivered = true;
 				count = 0;
 			}
 			stack[count++] = sk;
@@ -1694,8 +1696,11 @@ start_lookup:
 	 */
 	if (count) {
 		flush_stack(stack, count, skb, count - 1);
-	} else {
+	} else if (!delivered) {
+		UDP_INC_STATS_BH(net, UDP_MIB_NOPORTS, proto == IPPROTO_UDPLITE);
 		kfree_skb(skb);
+	} else {
+		consume_skb(skb);
 	}
 	return 0;
 }

^ permalink raw reply related

* Re: [PATCH 1/1 linux-next] ieee802154: add __init to lowpan_frags_sysctl_register
From: Alexander Aring @ 2014-10-01  0:25 UTC (permalink / raw)
  To: Fabian Frederick; +Cc: linux-kernel, David S. Miller, linux-wpan, netdev
In-Reply-To: <1412109249-7432-1-git-send-email-fabf@skynet.be>

Hi,

On Tue, Sep 30, 2014 at 10:34:08PM +0200, Fabian Frederick wrote:
> lowpan_frags_sysctl_register is only called by __init lowpan_net_frag_init
> (part of the lowpan module).
> 
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
> This is untested.
> 
>  net/ieee802154/reassembly.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ieee802154/reassembly.c b/net/ieee802154/reassembly.c
> index 32755cb..30ec608 100644
> --- a/net/ieee802154/reassembly.c
> +++ b/net/ieee802154/reassembly.c
> @@ -485,7 +485,7 @@ static void __net_exit lowpan_frags_ns_sysctl_unregister(struct net *net)
>  
>  static struct ctl_table_header *lowpan_ctl_header;
>  
> -static int lowpan_frags_sysctl_register(void)
> +static int __init lowpan_frags_sysctl_register(void)

yes right, but there is more lacks of missing "__init". See below.

>  {
>  	lowpan_ctl_header = register_net_sysctl(&init_net,
>  						"net/ieee802154/6lowpan",
> @@ -498,7 +498,7 @@ static void lowpan_frags_sysctl_unregister(void)
>  	unregister_net_sysctl_table(lowpan_ctl_header);
>  }
>  #else
> -static inline int lowpan_frags_ns_sysctl_register(struct net *net)
> +static inline int __init lowpan_frags_ns_sysctl_register(struct net *net)

This is wrong, it's callback from "struct pernet_operations lowpan_frags_ops".

>  {
>  	return 0;
>  }


Your patch adds "__init" now for two different functions and we have
two different declarations for these functions, depends if CONFIG_SYSCTL is
enabled.

Now if CONFIG_SYSCTL select we have as lowpan_frags_sysctl_register declaration:

static int __init lowpan_frags_sysctl_register(void)

if not:

static inline int lowpan_frags_sysctl_register(void)

Same for lowpan_frags_ns_sysctl_register and vice versa for CONFIG_SYSCTL.

Your changes are for two different functions (Don't know if you realized that):

"lowpan_frags_sysctl_register" and "lowpan_frags_ns_sysctl_register",
which makes no sense. Also lowpan_frags_ns_sysctl_register isn't called
by module init which is wrong.


To make "correct" cleanup for this "__init" should be add to the functions, for
both declarations if CONFIG_SYSCTL is set or not:

 - lowpan_net_frag_init
 - lowpan_frags_sysctl_register


I see now it's already applied, David please revert this change. Are you
fine to apply a correct version of this to wpan-next tree, next time?

- Alex

^ permalink raw reply

* Re: Philosophical question:  Is a UDP multicast datagram for which there is no socket match a drop or an ignore?
From: Rick Jones @ 2014-10-01  0:22 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1412119398.16704.11.camel@edumazet-glaptop2.roam.corp.google.com>

On 09/30/2014 04:23 PM, Eric Dumazet wrote:
> On Tue, 2014-09-30 at 16:09 -0700, Rick Jones wrote:
>> I've been looking at some additional perf <mutter> -e skb_kfree_skb
>> results, this time with a laptop connected to a corporate network with a
>> large number of Windows systems sending out what they are wont to
>> send...  The laptop is just sitting there no active netperfs or anything :)
>>
>> I see profile hits for __udp4_lib_mcast_deliver() which has a
>> kfree_skb() call which will happen if either there were no sockets
>> found, or if an integral multiple of ARRAY_SIZE(stack) sockets are
>> found.  I'm assuming the latter is exceedingly rare.
>>
>> Anywho, the philosophical question - is such a situation a drop
>> (indicating the existing kfree_skb()), or is it an ignore (indicating a
>> consume_skb())?  Should there be a statistic incremented for either of
>> those?
>
> I guess we lack a UDP_MIB_NOPORTS increase here.

I was going back and forth on that - since it is a multicast it may not 
have really been directed at us in which case it would be an ignore (and 
perhaps a new "ignored" stat?).  But on the assumption that it should 
indeed remain a drop, and so a kfree_skb(), something along the lines of:


diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index cd0db54..376e3d3 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1656,6 +1656,7 @@ static int __udp4_lib_mcast_deliver(struct net 
*net, struc
         int dif = skb->dev->ifindex;
         unsigned int count = 0, offset = offsetof(typeof(*sk), 
sk_nulls_node);
         unsigned int hash2 = 0, hash2_any = 0, use_hash2 = 
(hslot->count > 10);
+       unsigned int inner_flushed = 0;

         if (use_hash2) {
                 hash2_any = udp4_portaddr_hash(net, htonl(INADDR_ANY), 
hnum) &
@@ -1694,8 +1695,12 @@ start_lookup:
          */
         if (count) {
                 flush_stack(stack, count, skb, count - 1);
-       } else {
+       } else if (!inner_flushed) {
+               UDP_INC_STATS_BH(net, UDP_MIB_NOPORTS, 0);
                 kfree_skb(skb);
+       } else {
+               /* there were matches flushed in the for_each */
+               consume_skb(skb);
         }
         return 0;
  }


?  The idea being that in the unlikely event there were indeed enough 
matches to trigger the flush_stack in the for_each and only enough for 
that it will be a consume_skb() and no statistic rather than a 
kfree_skb() and a statistic increment.

(likely munged by my mailer)

rick

^ permalink raw reply related

* Re: [PATCH net-next] net: pktgen: packet bursting via skb->xmit_more
From: Alexei Starovoitov @ 2014-10-01  0:18 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S. Miller, Eric Dumazet, Jesper Dangaard Brouer,
	Network Development
In-Reply-To: <1412122303.16704.15.camel@edumazet-glaptop2.roam.corp.google.com>

On Tue, Sep 30, 2014 at 5:11 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> It seems you dont really need @burst_cnt or @more in this function.

excellent suggestion! That is indeed much easier to read. will respin.

^ permalink raw reply

* Re: [PATCH] net: Add ndo_gso_check
From: Eric Dumazet @ 2014-10-01  0:18 UTC (permalink / raw)
  To: Tom Herbert
  Cc: Stephen Hemminger, Or Gerlitz, Jeff Kirsher, Alexander Duyck,
	David Miller, Linux Netdev List, Thomas Graf, Pravin Shelar,
	John Fastabend, Andy Zhou
In-Reply-To: <CA+mtBx-wp5qNHGM0jG+2RjtXor3x1zJcSvZafz7kEKNEm4jYLA@mail.gmail.com>

On Tue, 2014-09-30 at 17:05 -0700, Tom Herbert wrote:

> 
> Will this still be an advantage with the burst TX implementation?

Segmenting a 64K packet into 45 sk_buff adds an additional 90
allocations (45 sk_buff, 45 sk->head) and dirtying a _lot_ of memory.

We now have some helpers that might be used by drivers to perform TSO
emulation.

commit e876f208af18b074f800656e4d1b99da75b2135f ("net: Add a software
TSO helper API")

Check commit 3ae8f4e0b98b640aadf410c21185ccb6b5b02351 for reference
("net: mv643xx_eth: Implement software TSO")

^ permalink raw reply

* Re: [PATCH net-next] net: pktgen: packet bursting via skb->xmit_more
From: Eric Dumazet @ 2014-10-01  0:11 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David S. Miller, Eric Dumazet, Jesper Dangaard Brouer, netdev
In-Reply-To: <1412121820-28633-1-git-send-email-ast@plumgrid.com>

On Tue, 2014-09-30 at 17:03 -0700, Alexei Starovoitov wrote:
...

>  static void pktgen_xmit(struct pktgen_dev *pkt_dev)
>  {
> +	unsigned int burst = ACCESS_ONCE(pkt_dev->burst), burst_cnt;
>  	struct net_device *odev = pkt_dev->odev;
>  	struct netdev_queue *txq;
> +	bool more;
>  	int ret;
>  
>  	/* If device is offline, then don't send */
> @@ -3347,8 +3363,14 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
>  		pkt_dev->last_ok = 0;
>  		goto unlock;
>  	}
> -	atomic_inc(&(pkt_dev->skb->users));
> -	ret = netdev_start_xmit(pkt_dev->skb, odev, txq, false);
> +	atomic_add(burst, &pkt_dev->skb->users);
> +
> +	burst_cnt = 0;
> +

It seems you dont really need @burst_cnt or @more in this function.

xmit_more:
	ret = netdev_start_xmit(pkt_dev->skb, odev, txq, --burst > 0);

> +xmit_more:
> +	more = ++burst_cnt < burst;
> +
> +	ret = netdev_start_xmit(pkt_dev->skb, odev, txq, more);
>  
>  	switch (ret) {
>  	case NETDEV_TX_OK:
> @@ -3356,6 +3378,8 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
>  		pkt_dev->sofar++;
>  		pkt_dev->seq_num++;
>  		pkt_dev->tx_bytes += pkt_dev->last_pkt_size;
> +		if (more && !netif_xmit_frozen_or_drv_stopped(txq))
> +			goto xmit_more;

	if (burst > 0 && !netif_xmit_frozen_or_drv_stopped(txq))
		goto xmit_more

>  		break;
>  	case NET_XMIT_DROP:
>  	case NET_XMIT_CN:
> @@ -3374,6 +3398,9 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
>  		atomic_dec(&(pkt_dev->skb->users));
>  		pkt_dev->last_ok = 0;
>  	}
> +
> +	if (unlikely(burst - burst_cnt > 0))
> +		atomic_sub(burst - burst_cnt, &pkt_dev->skb->users);

	if (unlikely(burst))
		atomic_sub(burst, &pkt_dev->skb->users);

>  unlock:
>  	HARD_TX_UNLOCK(odev, txq);
>  
> @@ -3572,6 +3599,7 @@ static int pktgen_add_device(struct pktgen_thread *t, const char *ifname)
>  	pkt_dev->svlan_p = 0;
>  	pkt_dev->svlan_cfi = 0;
>  	pkt_dev->svlan_id = 0xffff;
> +	pkt_dev->burst = 1;
>  	pkt_dev->node = -1;
>  
>  	err = pktgen_setup_dev(t->net, pkt_dev, ifname);

^ permalink raw reply

* Re: [PATCH] net: Add ndo_gso_check
From: Tom Herbert @ 2014-10-01  0:05 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Or Gerlitz, Jeff Kirsher, Alexander Duyck, David Miller,
	Linux Netdev List, Thomas Graf, Pravin Shelar, John Fastabend,
	Andy Zhou
In-Reply-To: <20140930143700.3dffac4d@urahara>

On Tue, Sep 30, 2014 at 2:37 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Tue, 30 Sep 2014 08:34:14 -0700
> Tom Herbert <therbert@google.com> wrote:
>
> > On Tue, Sep 30, 2014 at 7:30 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> > > On Tue, Sep 30, 2014 at 12:38 AM, Tom Herbert <therbert@google.com> wrote:
> > >> On Mon, Sep 29, 2014 at 2:10 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> > >>> On Mon, Sep 29, 2014 at 11:53 PM, Tom Herbert <therbert@google.com> wrote:
> > >>>> On Mon, Sep 29, 2014 at 12:59 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> > >>>>> On Mon, Sep 29, 2014 at 6:50 AM, Tom Herbert <therbert@google.com> wrote:
> > >>>>>> Add ndo_gso_check which a device can define to indicate whether is
> > >>>>>> is capable of doing GSO on a packet. This funciton would be called from
> > >>>>>> the stack to determine whether software GSO is needed to be done.
> > >>>>>
> > >>>>> please, no...
> > >>>>>
> > >>>>> We should strive to have a model/architecture under which the driver
> > >>>>> can clearly advertize up something (bits, collection of bits,
> > >>>>> whatever) which the stack can run through some dec-sion making code
> > >>>>> and decide if to GSO yes/no, do it ala SW or ala HW. As life, this
> > >>>>> model need not be perfect and can be biased towards being
> > >>>>> simple/robust/conservative and developed incrementally.
> > >
> > >>>> Please make a specific proposal then.
> > >
> > > OK
> > >
> > >>> We 1st need to bring the system back into a consistent state, see my
> > >>> band-aid proposal. BTW, this band-aid might turn to be the basis for
> > >>> the longer term solution too. I admit that saying "no" is ten (100)
> > >>> times harder vs. say "let's do it this way", but IMHO the fm10k call
> > >>> chain I pointed on is what you are suggesting more-or-less and is no-no
> > >
> > >> I'd much rather have drivers do this, than inflict the stack with more
> > >> complexity. As you describe "the driver can clearly advertise up
> > >> something (bits, collection of bits, whatever) which the stack can run
> > >> through some dec-sion making code and decide if to GSO yes/no"-- seems
> > >> very complex to me. My proposed alternative is to just ask the driver
> > >
> > > I see the point you are trying to make, but
> > >
> > >> and they can implement whatever policy they want, stack should doesn't
> > >> care about the specifics, just needs an answer. Neither does this
> > >> necessarily mean that driver needs to inspect packet, for instance I
> > >> suspect that just be looking at inner header lengths and skb->protocol
> > >> being TEB would be standard check to match VXLAN.
> > >
> > > I'm not sure how exactly this (inner protocol being Ethernet and inner
> > > header lengths)
> > > is going to work to differentiate between VXLAN and NVGRE (or @ least
> > > the GRE-ing done
> > > by OVS on guest Ethernet frames).
> > >
> > GSO processing for VXLAN and NVGRE should be identical. They both have
> > a four byte header that needs to be copied per packet and both only
> > carry Ethernet frames.
> >
> > >> In any case, if you can formulate your proposal in a patch that would
> > >> be very helpful.
> > >
> > > Quick idea is the following:
> > >
> > > It's common that when someone along the stack (e,g OVS vxlan/gre datapath logic)
> > > encapsulates a packet, they do know what sort of encapsulation they are doing.
> > >
> > > So the encapsulating entity can color the packet skb and the driver
> > > would advertize
> > > to what colors (UDP encap types) they can do GSO. When we come to a
> > > point where the
> > > stack has to decide if go for SW or HW GSO, they attempt to match the colors.
> > >
> > This would be equivalent to adding more protocol specific GSO feature
> > bits. I still don't see how this will scale. The number of protocols
> > that we might want to encapsulate over UDP is vast-- even before FOU
> > adding possibility of encapsulating any IP protocol in UDP. And, as
> > already pointed out, devices might have other arbitrary limitations
> > such as length of inner headers that wouldn't easily be represented in
> > features.
> >
> > Also, this does not benefit the stack or drivers that already support
> > generic SKB_GSO_UDP_TUNNEL mechanism.
> >
> > Would any other driver maintainers like to chime in on this?
>
> I prefer the simplistic "yes I can do GSO" flag and let the
> driver do software GSO for the cases where it detects "no never mind
> that, I can't' do GSO on a Q-in-Q VLAN with NVGRE".
>
That would be nice since it entails no change to the stack and follows
same model used for checksum (I couldn't find any drivers that call
skb_mac_gso_segment right now). Though the driver might need to handle
the case where it is unable to queue all the segments created.

> Software GSO at driver level is sometimes better anyway because it means driver can
> enqueue a burst of packets into Tx ring.


Will this still be an advantage with the burst TX implementation?

^ permalink raw reply

* [PATCH net-next] net: pktgen: packet bursting via skb->xmit_more
From: Alexei Starovoitov @ 2014-10-01  0:03 UTC (permalink / raw)
  To: David S. Miller; +Cc: Eric Dumazet, Jesper Dangaard Brouer, netdev

This patch demonstrates the effect of delaying update of HW tailptr.
(based on earlier patch by Jesper)

burst=1 is the default. It sends one packet with xmit_more=false
burst=2 sends one packet with xmit_more=true and
        2nd copy of the same packet with xmit_more=false
burst=3 sends two copies of the same packet with xmit_more=true and
        3rd copy with xmit_more=false

Performance with ixgbe (usec 30):
burst=1  tx:9.2 Mpps
burst=2  tx:13.5 Mpps
burst=3  tx:14.5 Mpps full 10G line rate

Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---

RFC -> v1:
- added netif_xmit_frozen_or_drv_stopped() check, since bnx2x
  stops the queue and returns TX_OK (suggested by Eric)
- read pkt_dev->burst once to avoid races (suggested by Eric)
- changed 'int burst' to 'unsigned int burst'
- updated doc
- tested on ixgbe, mlx4, bnx2x

Comparing to Jesper patch this one amortizes the cost
of spin_lock and atomic_inc by doing HARD_TX_LOCK and
atomic_add(N) once across N packets.

I didn't add Jesper and Eric's acks. Guys, please re-ack
if you still think it's ok. Thanks!

 Documentation/networking/pktgen.txt |    3 +++
 net/core/pktgen.c                   |   32 ++++++++++++++++++++++++++++++--
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/Documentation/networking/pktgen.txt b/Documentation/networking/pktgen.txt
index 0dffc6e37902..6915c6b27869 100644
--- a/Documentation/networking/pktgen.txt
+++ b/Documentation/networking/pktgen.txt
@@ -99,6 +99,9 @@ Examples:
 
  pgset "clone_skb 1"     sets the number of copies of the same packet
  pgset "clone_skb 0"     use single SKB for all transmits
+ pgset "burst 8"         uses xmit_more API to queue 8 copies of the same
+                         packet and update HW tx queue tail pointer once.
+                         "burst 1" is the default
  pgset "pkt_size 9014"   sets packet size to 9014
  pgset "frags 5"         packet will consist of 5 fragments
  pgset "count 200000"    sets number of packets to send, set to zero
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 5c728aaf8d6c..fe53aa57bc5c 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -387,6 +387,7 @@ struct pktgen_dev {
 	u16 queue_map_min;
 	u16 queue_map_max;
 	__u32 skb_priority;	/* skb priority field */
+	unsigned int burst;	/* number of duplicated packets to burst */
 	int node;               /* Memory node */
 
 #ifdef CONFIG_XFRM
@@ -613,6 +614,9 @@ static int pktgen_if_show(struct seq_file *seq, void *v)
 	if (pkt_dev->traffic_class)
 		seq_printf(seq, "     traffic_class: 0x%02x\n", pkt_dev->traffic_class);
 
+	if (pkt_dev->burst > 1)
+		seq_printf(seq, "     burst: %d\n", pkt_dev->burst);
+
 	if (pkt_dev->node >= 0)
 		seq_printf(seq, "     node: %d\n", pkt_dev->node);
 
@@ -1124,6 +1128,16 @@ static ssize_t pktgen_if_write(struct file *file,
 			pkt_dev->dst_mac_count);
 		return count;
 	}
+	if (!strcmp(name, "burst")) {
+		len = num_arg(&user_buffer[i], 10, &value);
+		if (len < 0)
+			return len;
+
+		i += len;
+		pkt_dev->burst = value < 1 ? 1 : value;
+		sprintf(pg_result, "OK: burst=%d", pkt_dev->burst);
+		return count;
+	}
 	if (!strcmp(name, "node")) {
 		len = num_arg(&user_buffer[i], 10, &value);
 		if (len < 0)
@@ -3297,8 +3311,10 @@ static void pktgen_wait_for_skb(struct pktgen_dev *pkt_dev)
 
 static void pktgen_xmit(struct pktgen_dev *pkt_dev)
 {
+	unsigned int burst = ACCESS_ONCE(pkt_dev->burst), burst_cnt;
 	struct net_device *odev = pkt_dev->odev;
 	struct netdev_queue *txq;
+	bool more;
 	int ret;
 
 	/* If device is offline, then don't send */
@@ -3347,8 +3363,14 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
 		pkt_dev->last_ok = 0;
 		goto unlock;
 	}
-	atomic_inc(&(pkt_dev->skb->users));
-	ret = netdev_start_xmit(pkt_dev->skb, odev, txq, false);
+	atomic_add(burst, &pkt_dev->skb->users);
+
+	burst_cnt = 0;
+
+xmit_more:
+	more = ++burst_cnt < burst;
+
+	ret = netdev_start_xmit(pkt_dev->skb, odev, txq, more);
 
 	switch (ret) {
 	case NETDEV_TX_OK:
@@ -3356,6 +3378,8 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
 		pkt_dev->sofar++;
 		pkt_dev->seq_num++;
 		pkt_dev->tx_bytes += pkt_dev->last_pkt_size;
+		if (more && !netif_xmit_frozen_or_drv_stopped(txq))
+			goto xmit_more;
 		break;
 	case NET_XMIT_DROP:
 	case NET_XMIT_CN:
@@ -3374,6 +3398,9 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
 		atomic_dec(&(pkt_dev->skb->users));
 		pkt_dev->last_ok = 0;
 	}
+
+	if (unlikely(burst - burst_cnt > 0))
+		atomic_sub(burst - burst_cnt, &pkt_dev->skb->users);
 unlock:
 	HARD_TX_UNLOCK(odev, txq);
 
@@ -3572,6 +3599,7 @@ static int pktgen_add_device(struct pktgen_thread *t, const char *ifname)
 	pkt_dev->svlan_p = 0;
 	pkt_dev->svlan_cfi = 0;
 	pkt_dev->svlan_id = 0xffff;
+	pkt_dev->burst = 1;
 	pkt_dev->node = -1;
 
 	err = pktgen_setup_dev(t->net, pkt_dev, ifname);
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH v6 00/10] ARM: Berlin: Ethernet support
From: David Miller @ 2014-09-30 23:59 UTC (permalink / raw)
  To: sebastian.hesselbarth
  Cc: antoine.tenart, alexandre.belloni, thomas.petazzoni, zmxu,
	jszhang, netdev, linux-arm-kernel, devicetree, linux-kernel
In-Reply-To: <542B2851.2050705@gmail.com>

From: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Date: Wed, 01 Oct 2014 00:01:53 +0200

> Just to make sure how long I have to take care of potential merge
> issues, is your current net-next meant for v3.18 or v3.19?

3.18

^ permalink raw reply

* Re: [Patch net-next] net_sched: fix another crash in cls_tcindex
From: John Fastabend @ 2014-09-30 23:50 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, davem, John Fastabend
In-Reply-To: <1412118444-29179-1-git-send-email-xiyou.wangcong@gmail.com>

On 09/30/2014 04:07 PM, Cong Wang wrote:
> This patch fixes the following crash:
>
> [  166.670795] BUG: unable to handle kernel NULL pointer dereference at           (null)
> [  166.674230] IP: [<ffffffff814b739f>] __list_del_entry+0x5c/0x98
> [  166.674230] PGD d0ea5067 PUD ce7fc067 PMD 0
> [  166.674230] Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> [  166.674230] CPU: 1 PID: 775 Comm: tc Not tainted 3.17.0-rc6+ #642
> [  166.674230] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> [  166.674230] task: ffff8800d03c4d20 ti: ffff8800cae7c000 task.ti: ffff8800cae7c000
> [  166.674230] RIP: 0010:[<ffffffff814b739f>]  [<ffffffff814b739f>] __list_del_entry+0x5c/0x98
> [  166.674230] RSP: 0018:ffff8800cae7f7d0  EFLAGS: 00010207
> [  166.674230] RAX: 0000000000000000 RBX: ffff8800cba8d700 RCX: ffff8800cba8d700
> [  166.674230] RDX: 0000000000000000 RSI: dead000000200200 RDI: ffff8800cba8d700
> [  166.674230] RBP: ffff8800cae7f7d0 R08: 0000000000000001 R09: 0000000000000001
> [  166.674230] R10: 0000000000000000 R11: 000000000000859a R12: ffffffffffffffe8
> [  166.674230] R13: ffff8800cba8c5b8 R14: 0000000000000001 R15: ffff8800cba8d700
> [  166.674230] FS:  00007fdb5f04a740(0000) GS:ffff88011a800000(0000) knlGS:0000000000000000
> [  166.674230] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [  166.674230] CR2: 0000000000000000 CR3: 00000000cf929000 CR4: 00000000000006e0
> [  166.674230] Stack:
> [  166.674230]  ffff8800cae7f7e8 ffffffff814b73e8 ffff8800cba8d6e8 ffff8800cae7f828
> [  166.674230]  ffffffff817caeec 0000000000000046 ffff8800cba8c5b0 ffff8800cba8c5b8
> [  166.674230]  0000000000000000 0000000000000001 ffff8800cf8e33e8 ffff8800cae7f848
> [  166.674230] Call Trace:
> [  166.674230]  [<ffffffff814b73e8>] list_del+0xd/0x2b
> [  166.674230]  [<ffffffff817caeec>] tcf_action_destroy+0x4c/0x71
> [  166.674230]  [<ffffffff817ca0ce>] tcf_exts_destroy+0x20/0x2d
> [  166.674230]  [<ffffffff817ec2b5>] tcindex_delete+0x196/0x1b7
>
> struct list_head can not be simply copied and we should always init it.
>
> Cc: John Fastabend <john.r.fastabend@intel.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---

Thanks again.

Acked-by: John Fastabend <john.r.fastabend@intel.com>


-- 
John Fastabend         Intel Corporation

^ permalink raw reply

* [PATCH v2 net 2/2] bnx2: Correctly receive full sized 802.1ad fragmes
From: Vladislav Yasevich @ 2014-09-30 23:39 UTC (permalink / raw)
  To: netdev
  Cc: prashant, mchan, sony.chacko, Dept-HSGLinuxNICDev,
	Vladislav Yasevich
In-Reply-To: <1412120377-11125-1-git-send-email-vyasevic@redhat.com>

This driver, similar to tg3, has a check that will
cause full sized 802.1ad frames to be dropped.  The
frame will be larger then the standard mtu due to the
presense of vlan header that has not been stripped.
The driver should not drop this frame and should process
it just like it does for 802.1q.

CC: Sony Chacko <sony.chacko@qlogic.com>
CC: Dept-HSGLinuxNICDev@qlogic.com
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 drivers/net/ethernet/broadcom/bnx2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2.c b/drivers/net/ethernet/broadcom/bnx2.c
index 2fee73b..823d01c 100644
--- a/drivers/net/ethernet/broadcom/bnx2.c
+++ b/drivers/net/ethernet/broadcom/bnx2.c
@@ -3236,8 +3236,9 @@ bnx2_rx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget)
 
 		skb->protocol = eth_type_trans(skb, bp->dev);
 
-		if ((len > (bp->dev->mtu + ETH_HLEN)) &&
-			(ntohs(skb->protocol) != 0x8100)) {
+		if (len > (bp->dev->mtu + ETH_HLEN) &&
+		    skb->protocol != htons(0x8100) &&
+		    skb->protocol != htons(ETH_P_8021AD)) {
 
 			dev_kfree_skb(skb);
 			goto next_rx;
-- 
1.9.3

^ permalink raw reply related

* [PATCH v2 net 1/2] tg3: Allow for recieve of full-size 8021AD frames
From: Vladislav Yasevich @ 2014-09-30 23:39 UTC (permalink / raw)
  To: netdev
  Cc: prashant, mchan, sony.chacko, Dept-HSGLinuxNICDev,
	Vladislav Yasevich
In-Reply-To: <1412120377-11125-1-git-send-email-vyasevic@redhat.com>

When receiving a vlan-tagged frame that still contains
a vlan header, the length of the packet will be greater
then MTU+ETH_HLEN since it will account of the extra
vlan header.  TG3 checks this for the case for 802.1Q,
but not for 802.1ad.  As a result, full sized 802.1ad
frames get dropped by the card.

Add a check for 802.1ad protocol when receving full
sized frames.

Suggested-by: Prashant Sreedharan <prashant@broadcom.com>
CC: Prashant Sreedharan <prashant@broadcom.com>
CC: Michael Chan <mchan@broadcom.com>
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index e7d3a62..ba49948 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -6918,7 +6918,8 @@ static int tg3_rx(struct tg3_napi *tnapi, int budget)
 		skb->protocol = eth_type_trans(skb, tp->dev);
 
 		if (len > (tp->dev->mtu + ETH_HLEN) &&
-		    skb->protocol != htons(ETH_P_8021Q)) {
+		    skb->protocol != htons(ETH_P_8021Q) &&
+		    skb->protocol != htons(ETH_P_8021AD)) {
 			dev_kfree_skb_any(skb);
 			goto drop_it_no_recycle;
 		}
-- 
1.9.3

^ permalink raw reply related

* [PATCH v2 net 0/2] Alow tg3/bnx recevie full sized 802.1ad frames
From: Vladislav Yasevich @ 2014-09-30 23:39 UTC (permalink / raw)
  To: netdev
  Cc: prashant, mchan, sony.chacko, Dept-HSGLinuxNICDev,
	Vladislav Yasevich

bnx2 and tg3 drivers drop packets that exceed device mtu and that
do not have a vlan tag.  The idea is to catch ethernet frames
that are too long.  This also ends up dropping 802.1ad tagged
frames since the encapsulation protocol is different.
We should not be dropping this packets.

v2:  rebased and consolidated tg3 and bnx patches.

Vladislav Yasevich (2):
  tg3: Allow for recieve of full-size 8021AD frames
  bnx2: Correctly receive full sized 802.1ad fragmes

 drivers/net/ethernet/broadcom/bnx2.c | 5 +++--
 drivers/net/ethernet/broadcom/tg3.c  | 3 ++-
 2 files changed, 5 insertions(+), 3 deletions(-)

-- 
1.9.3

^ permalink raw reply

* Re: Philosophical question:  Is a UDP multicast datagram for which there is no socket match a drop or an ignore?
From: Eric Dumazet @ 2014-09-30 23:23 UTC (permalink / raw)
  To: Rick Jones; +Cc: netdev
In-Reply-To: <542B3831.5030701@hp.com>

On Tue, 2014-09-30 at 16:09 -0700, Rick Jones wrote:
> I've been looking at some additional perf <mutter> -e skb_kfree_skb 
> results, this time with a laptop connected to a corporate network with a 
> large number of Windows systems sending out what they are wont to 
> send...  The laptop is just sitting there no active netperfs or anything :)
> 
> I see profile hits for __udp4_lib_mcast_deliver() which has a 
> kfree_skb() call which will happen if either there were no sockets 
> found, or if an integral multiple of ARRAY_SIZE(stack) sockets are 
> found.  I'm assuming the latter is exceedingly rare.
> 
> Anywho, the philosophical question - is such a situation a drop 
> (indicating the existing kfree_skb()), or is it an ignore (indicating a 
> consume_skb())?  Should there be a statistic incremented for either of 
> those?

I guess we lack a UDP_MIB_NOPORTS increase here.

^ permalink raw reply

* Re: [PATCH v2 net 0/3] bridge: Add vlan filtering support for default pvid
From: Vlad Yasevich @ 2014-09-30 23:16 UTC (permalink / raw)
  To: David Miller, vyasevich; +Cc: stephen, netdev, bridge
In-Reply-To: <20140930.170718.1417042696038383945.davem@davemloft.net>

On 09/30/2014 05:07 PM, David Miller wrote:
> From: Vladislav Yasevich <vyasevich@gmail.com>
> Date: Tue, 30 Sep 2014 15:30:59 -0400
> 
>> Version 2 of the series to introduce the default pvid support to
>> vlan filtering in the bridge.  VLAN 1 (as recommended by 802.1q spec)
>> is used as default pvid on ports. 
>> The the user can over-ride this configuration by configuring their
>> own vlan information. 
>> The user can additionally change the default value throught the
>> sysfs interface (netlink comming shortly).
>> The user can turn off default pvid functionality by setting default
>> pvid to 0. 
>> This series changes the default behavior of the bridge when
>> vlan filtering is turned on.  Currently, ports without any vlan
>> filtering configured will not recevie any traffic at all.  This patch
>> changes the behavior of the above ports to receive only untagged traffic.
>>
>> Since v2:
>> - Add ability to turn off default_pvid settings.
>> - Drop the automiatic filtering support based on configured vlan devices (will
>>   be its own series)
> 
> Looks like a new feature to me, thus why is this being targetted at 'net'
> instead of 'net-next'?
> 

goof on my part.  This should be net-next.

-vlad

^ permalink raw reply

* [PATCH net-next] net: bridge: add a br_set_state helper function
From: Florian Fainelli @ 2014-09-30 23:13 UTC (permalink / raw)
  To: netdev; +Cc: davem, bridge, stephen, vyasevich, jiri, Florian Fainelli

In preparation for being able to propagate port states to e.g: notifiers
or other kernel parts, do not manipulate the port state directly, but
instead use a helper function which will allow us to do a bit more than
just setting the state.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/bridge/br_if.c        |  2 +-
 net/bridge/br_netlink.c   |  2 +-
 net/bridge/br_private.h   |  1 +
 net/bridge/br_stp.c       | 15 ++++++++++-----
 net/bridge/br_stp_if.c    |  4 ++--
 net/bridge/br_stp_timer.c |  4 ++--
 6 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index a9f54a9b6690..7b7289ca2992 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -332,7 +332,7 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
 	p->port_no = index;
 	p->flags = BR_LEARNING | BR_FLOOD;
 	br_init_port(p);
-	p->state = BR_STATE_DISABLED;
+	br_set_state(p, BR_STATE_DISABLED);
 	br_stp_port_timer_init(p);
 	br_multicast_add_port(p);
 
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 0fa66b83685f..2ff9706647f2 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -301,7 +301,7 @@ static int br_set_port_state(struct net_bridge_port *p, u8 state)
 	    (!netif_oper_up(p->dev) && state != BR_STATE_DISABLED))
 		return -ENETDOWN;
 
-	p->state = state;
+	br_set_state(p, state);
 	br_log_state(p);
 	br_port_state_selection(p->br);
 	return 0;
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index f53592fc3ef9..fe7463c2af9a 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -766,6 +766,7 @@ static inline void br_nf_core_fini(void) {}
 
 /* br_stp.c */
 void br_log_state(const struct net_bridge_port *p);
+void br_set_state(struct net_bridge_port *p, unsigned int state);
 struct net_bridge_port *br_get_port(struct net_bridge *br, u16 port_no);
 void br_init_port(struct net_bridge_port *p);
 void br_become_designated_port(struct net_bridge_port *p);
diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
index 3c86f0538cbb..2b047bcf42a4 100644
--- a/net/bridge/br_stp.c
+++ b/net/bridge/br_stp.c
@@ -36,6 +36,11 @@ void br_log_state(const struct net_bridge_port *p)
 		br_port_state_names[p->state]);
 }
 
+void br_set_state(struct net_bridge_port *p, unsigned int state)
+{
+	p->state = state;
+}
+
 /* called under bridge lock */
 struct net_bridge_port *br_get_port(struct net_bridge *br, u16 port_no)
 {
@@ -107,7 +112,7 @@ static void br_root_port_block(const struct net_bridge *br,
 	br_notice(br, "port %u(%s) tried to become root port (blocked)",
 		  (unsigned int) p->port_no, p->dev->name);
 
-	p->state = BR_STATE_LISTENING;
+	br_set_state(p, BR_STATE_LISTENING);
 	br_log_state(p);
 	br_ifinfo_notify(RTM_NEWLINK, p);
 
@@ -387,7 +392,7 @@ static void br_make_blocking(struct net_bridge_port *p)
 		    p->state == BR_STATE_LEARNING)
 			br_topology_change_detection(p->br);
 
-		p->state = BR_STATE_BLOCKING;
+		br_set_state(p, BR_STATE_BLOCKING);
 		br_log_state(p);
 		br_ifinfo_notify(RTM_NEWLINK, p);
 
@@ -404,13 +409,13 @@ static void br_make_forwarding(struct net_bridge_port *p)
 		return;
 
 	if (br->stp_enabled == BR_NO_STP || br->forward_delay == 0) {
-		p->state = BR_STATE_FORWARDING;
+		br_set_state(p, BR_STATE_FORWARDING);
 		br_topology_change_detection(br);
 		del_timer(&p->forward_delay_timer);
 	} else if (br->stp_enabled == BR_KERNEL_STP)
-		p->state = BR_STATE_LISTENING;
+		br_set_state(p, BR_STATE_LISTENING);
 	else
-		p->state = BR_STATE_LEARNING;
+		br_set_state(p, BR_STATE_LEARNING);
 
 	br_multicast_enable_port(p);
 	br_log_state(p);
diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index 189ba1e7d851..41146872c1b4 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -37,7 +37,7 @@ void br_init_port(struct net_bridge_port *p)
 {
 	p->port_id = br_make_port_id(p->priority, p->port_no);
 	br_become_designated_port(p);
-	p->state = BR_STATE_BLOCKING;
+	br_set_state(p, BR_STATE_BLOCKING);
 	p->topology_change_ack = 0;
 	p->config_pending = 0;
 }
@@ -100,7 +100,7 @@ void br_stp_disable_port(struct net_bridge_port *p)
 
 	wasroot = br_is_root_bridge(br);
 	br_become_designated_port(p);
-	p->state = BR_STATE_DISABLED;
+	br_set_state(p, BR_STATE_DISABLED);
 	p->topology_change_ack = 0;
 	p->config_pending = 0;
 
diff --git a/net/bridge/br_stp_timer.c b/net/bridge/br_stp_timer.c
index 558c46d19e05..4fcaa67750fd 100644
--- a/net/bridge/br_stp_timer.c
+++ b/net/bridge/br_stp_timer.c
@@ -87,11 +87,11 @@ static void br_forward_delay_timer_expired(unsigned long arg)
 		 (unsigned int) p->port_no, p->dev->name);
 	spin_lock(&br->lock);
 	if (p->state == BR_STATE_LISTENING) {
-		p->state = BR_STATE_LEARNING;
+		br_set_state(p, BR_STATE_LEARNING);
 		mod_timer(&p->forward_delay_timer,
 			  jiffies + br->forward_delay);
 	} else if (p->state == BR_STATE_LEARNING) {
-		p->state = BR_STATE_FORWARDING;
+		br_set_state(p, BR_STATE_FORWARDING);
 		if (br_is_designated_for_some_port(br))
 			br_topology_change_detection(br);
 		netif_carrier_on(br->dev);
-- 
1.9.1

^ permalink raw reply related

* Philosophical question:  Is a UDP multicast datagram for which there is no socket match a drop or an ignore?
From: Rick Jones @ 2014-09-30 23:09 UTC (permalink / raw)
  To: netdev

I've been looking at some additional perf <mutter> -e skb_kfree_skb 
results, this time with a laptop connected to a corporate network with a 
large number of Windows systems sending out what they are wont to 
send...  The laptop is just sitting there no active netperfs or anything :)

I see profile hits for __udp4_lib_mcast_deliver() which has a 
kfree_skb() call which will happen if either there were no sockets 
found, or if an integral multiple of ARRAY_SIZE(stack) sockets are 
found.  I'm assuming the latter is exceedingly rare.

Anywho, the philosophical question - is such a situation a drop 
(indicating the existing kfree_skb()), or is it an ignore (indicating a 
consume_skb())?  Should there be a statistic incremented for either of 
those?

happy benchmarking,

rick jones

^ permalink raw reply

* r8168 is needed to enter P-state: Package State 6 (pc6) on Haswell hardware
From: Ceriel Jacobs @ 2014-09-30 23:09 UTC (permalink / raw)
  To: Realtek linux nic maintainers, Francois Romieu; +Cc: netdev

With in-kernel r8169 module, only P-state package C3 (pc3) can be 
reached when enabling ASPM.

Only after installing r8168 driver (and enabling ASPM), a Haswell 
Celeron G1820/G1840 processer will enter package C6 state (pc6).

Tested with Ubuntu kernel 3.13 x86_64
and Ubuntu mainline
Linux ubuntu14 3.17.0-999-generic #201409240305 SMP Wed Sep 24 02:07:04 
UTC 2014 x86_64 x86_64 x86_64 GNU/Linux

I hope this gets fixed: Haswell processor entering pc6 with ASPM enabled 
and r8169 in-kernel module.

R8111GR hardware:
# lspci -nnkvv -s 03:00.0
03:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd. 
RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller [10ec:8168] 
(rev 11)
	Subsystem: ASRock Incorporation Motherboard (one of many) [1849:8168]
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx+
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- 
<MAbort- >SERR- <PERR- INTx-
	Latency: 0, Cache Line Size: 64 bytes
	Interrupt: pin A routed to IRQ 27
	Region 0: I/O ports at e000 [size=256]
	Region 2: Memory at f0404000 (64-bit, non-prefetchable) [size=4K]
	Region 4: Memory at f0400000 (64-bit, prefetchable) [size=16K]
	Capabilities: [40] Power Management version 3
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA 
PME(D0+,D1+,D2+,D3hot+,D3cold+)
		Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
	Capabilities: [50] MSI: Enable+ Count=1/1 Maskable- 64bit+
		Address: 00000000fee0100c  Data: 41e1
	Capabilities: [70] Express (v2) Endpoint, MSI 01
		DevCap:	MaxPayload 128 bytes, PhantFunc 0, Latency L0s <512ns, L1 <64us
			ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
		DevCtl:	Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
			RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
			MaxPayload 128 bytes, MaxReadReq 4096 bytes
		DevSta:	CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend-
		LnkCap:	Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency 
L0s unlimited, L1 <64us
			ClockPM+ Surprise- LLActRep- BwNot-
		LnkCtl:	ASPM Disabled; RCB 64 bytes Disabled- CommClk+
			ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
		LnkSta:	Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- 
BWMgmt- ABWMgmt-
		DevCap2: Completion Timeout: Range ABCD, TimeoutDis+, LTR+, OBFF Via 
message/WAKE#
		DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR+, OBFF 
Disabled
		LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis-
			 Transmit Margin: Normal Operating Range, EnterModifiedCompliance- 
ComplianceSOS-
			 Compliance De-emphasis: -6dB
		LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-, 
EqualizationPhase1-
			 EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
	Capabilities: [b0] MSI-X: Enable- Count=4 Masked-
		Vector table: BAR=4 offset=00000000
		PBA: BAR=4 offset=00000800
	Capabilities: [d0] Vital Product Data
		Unknown small resource type 00, will not decode more.
	Capabilities: [100 v1] Advanced Error Reporting
		UESta:	DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- 
MalfTLP- ECRC- UnsupReq- ACSViol-
		UEMsk:	DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- 
MalfTLP- ECRC- UnsupReq- ACSViol-
		UESvrt:	DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ 
MalfTLP+ ECRC- UnsupReq- ACSViol-
		CESta:	RxErr- BadTLP+ BadDLLP- Rollover- Timeout+ NonFatalErr-
		CEMsk:	RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
		AERCap:	First Error Pointer: 00, GenCap+ CGenEn- ChkCap+ ChkEn-
	Capabilities: [140 v1] Virtual Channel
		Caps:	LPEVC=0 RefClk=100ns PATEntryBits=1
		Arb:	Fixed- WRR32- WRR64- WRR128-
		Ctrl:	ArbSelect=Fixed
		Status:	InProgress-
		VC0:	Caps:	PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
			Arb:	Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
			Ctrl:	Enable+ ID=0 ArbSelect=Fixed TC/VC=01
			Status:	NegoPending- InProgress-
	Capabilities: [160 v1] Device Serial Number <cut>
	Capabilities: [170 v1] Latency Tolerance Reporting
		Max snoop latency: 71680ns
		Max no snoop latency: 71680ns

^ permalink raw reply

* [Patch net-next] net_sched: avoid calling tcf_unbind_filter() in call_rcu callback
From: Cong Wang @ 2014-09-30 23:07 UTC (permalink / raw)
  To: netdev; +Cc: davem, Cong Wang, John Fastabend
In-Reply-To: <1412118444-29179-1-git-send-email-xiyou.wangcong@gmail.com>

This fixes the following crash:

[   63.976822] general protection fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
[   63.980094] CPU: 1 PID: 15 Comm: ksoftirqd/1 Not tainted 3.17.0-rc6+ #648
[   63.980094] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[   63.980094] task: ffff880117dea690 ti: ffff880117dfc000 task.ti: ffff880117dfc000
[   63.980094] RIP: 0010:[<ffffffff817e6d07>]  [<ffffffff817e6d07>] u32_destroy_key+0x27/0x6d
[   63.980094] RSP: 0018:ffff880117dffcc0  EFLAGS: 00010202
[   63.980094] RAX: ffff880117dea690 RBX: ffff8800d02e0820 RCX: 0000000000000000
[   63.980094] RDX: 0000000000000001 RSI: 0000000000000002 RDI: 6b6b6b6b6b6b6b6b
[   63.980094] RBP: ffff880117dffcd0 R08: 0000000000000000 R09: 0000000000000000
[   63.980094] R10: 00006c0900006ba8 R11: 00006ba100006b9d R12: 0000000000000001
[   63.980094] R13: ffff8800d02e0898 R14: ffffffff817e6d4d R15: ffff880117387a30
[   63.980094] FS:  0000000000000000(0000) GS:ffff88011a800000(0000) knlGS:0000000000000000
[   63.980094] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[   63.980094] CR2: 00007f07e6732fed CR3: 000000011665b000 CR4: 00000000000006e0
[   63.980094] Stack:
[   63.980094]  ffff88011a9cd300 ffffffff82051ac0 ffff880117dffce0 ffffffff817e6d68
[   63.980094]  ffff880117dffd70 ffffffff810cb4c7 ffffffff810cb3cd ffff880117dfffd8
[   63.980094]  ffff880117dea690 ffff880117dea690 ffff880117dfffd8 000000000000000a
[   63.980094] Call Trace:
[   63.980094]  [<ffffffff817e6d68>] u32_delete_key_freepf_rcu+0x1b/0x1d
[   63.980094]  [<ffffffff810cb4c7>] rcu_process_callbacks+0x3bb/0x691
[   63.980094]  [<ffffffff810cb3cd>] ? rcu_process_callbacks+0x2c1/0x691
[   63.980094]  [<ffffffff817e6d4d>] ? u32_destroy_key+0x6d/0x6d
[   63.980094]  [<ffffffff810780a4>] __do_softirq+0x142/0x323
[   63.980094]  [<ffffffff810782a8>] run_ksoftirqd+0x23/0x53
[   63.980094]  [<ffffffff81092126>] smpboot_thread_fn+0x203/0x221
[   63.980094]  [<ffffffff81091f23>] ? smpboot_unpark_thread+0x33/0x33
[   63.980094]  [<ffffffff8108e44d>] kthread+0xc9/0xd1
[   63.980094]  [<ffffffff819e00ea>] ? do_wait_for_common+0xf8/0x125
[   63.980094]  [<ffffffff8108e384>] ? __kthread_parkme+0x61/0x61
[   63.980094]  [<ffffffff819e43ec>] ret_from_fork+0x7c/0xb0
[   63.980094]  [<ffffffff8108e384>] ? __kthread_parkme+0x61/0x61

tp could be freed in call_rcu callback too, the order is not guaranteed.

Cc: John Fastabend <john.r.fastabend@intel.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/pkt_cls.h |  6 +-----
 net/sched/cls_u32.c   | 10 ++++++----
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 73f9532..ef44ad9 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -20,11 +20,7 @@ int unregister_tcf_proto_ops(struct tcf_proto_ops *ops);
 static inline unsigned long
 __cls_set_class(unsigned long *clp, unsigned long cl)
 {
-	unsigned long old_cl;
- 
-	old_cl = *clp;
-	*clp = cl;
-	return old_cl;
+	return xchg(clp, cl);
 }
 
 static inline unsigned long
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 4be3ebf..0472909 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -358,7 +358,6 @@ static int u32_destroy_key(struct tcf_proto *tp,
 			   struct tc_u_knode *n,
 			   bool free_pf)
 {
-	tcf_unbind_filter(tp, &n->res);
 	tcf_exts_destroy(&n->exts);
 	if (n->ht_down)
 		n->ht_down->refcnt--;
@@ -416,6 +415,7 @@ static int u32_delete_key(struct tcf_proto *tp, struct tc_u_knode *key)
 			if (pkp == key) {
 				RCU_INIT_POINTER(*kp, key->next);
 
+				tcf_unbind_filter(tp, &key->res);
 				call_rcu(&key->rcu, u32_delete_key_freepf_rcu);
 				return 0;
 			}
@@ -425,7 +425,7 @@ static int u32_delete_key(struct tcf_proto *tp, struct tc_u_knode *key)
 	return 0;
 }
 
-static void u32_clear_hnode(struct tc_u_hnode *ht)
+static void u32_clear_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht)
 {
 	struct tc_u_knode *n;
 	unsigned int h;
@@ -434,6 +434,7 @@ static void u32_clear_hnode(struct tc_u_hnode *ht)
 		while ((n = rtnl_dereference(ht->ht[h])) != NULL) {
 			RCU_INIT_POINTER(ht->ht[h],
 					 rtnl_dereference(n->next));
+			tcf_unbind_filter(tp, &n->res);
 			call_rcu(&n->rcu, u32_delete_key_freepf_rcu);
 		}
 	}
@@ -447,7 +448,7 @@ static int u32_destroy_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht)
 
 	WARN_ON(ht->refcnt);
 
-	u32_clear_hnode(ht);
+	u32_clear_hnode(tp, ht);
 
 	hn = &tp_c->hlist;
 	for (phn = rtnl_dereference(*hn);
@@ -482,7 +483,7 @@ static void u32_destroy(struct tcf_proto *tp)
 		     ht;
 		     ht = rtnl_dereference(ht->next)) {
 			ht->refcnt--;
-			u32_clear_hnode(ht);
+			u32_clear_hnode(tp, ht);
 		}
 
 		while ((ht = rtnl_dereference(tp_c->hlist)) != NULL) {
@@ -731,6 +732,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 		}
 
 		u32_replace_knode(tp, tp_c, new);
+		tcf_unbind_filter(tp, &n->res);
 		call_rcu(&n->rcu, u32_delete_key_rcu);
 		return 0;
 	}
-- 
1.8.3.1

^ permalink raw reply related

* [Patch net-next] net_sched: fix another crash in cls_tcindex
From: Cong Wang @ 2014-09-30 23:07 UTC (permalink / raw)
  To: netdev; +Cc: davem, Cong Wang, John Fastabend

This patch fixes the following crash:

[  166.670795] BUG: unable to handle kernel NULL pointer dereference at           (null)
[  166.674230] IP: [<ffffffff814b739f>] __list_del_entry+0x5c/0x98
[  166.674230] PGD d0ea5067 PUD ce7fc067 PMD 0
[  166.674230] Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
[  166.674230] CPU: 1 PID: 775 Comm: tc Not tainted 3.17.0-rc6+ #642
[  166.674230] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[  166.674230] task: ffff8800d03c4d20 ti: ffff8800cae7c000 task.ti: ffff8800cae7c000
[  166.674230] RIP: 0010:[<ffffffff814b739f>]  [<ffffffff814b739f>] __list_del_entry+0x5c/0x98
[  166.674230] RSP: 0018:ffff8800cae7f7d0  EFLAGS: 00010207
[  166.674230] RAX: 0000000000000000 RBX: ffff8800cba8d700 RCX: ffff8800cba8d700
[  166.674230] RDX: 0000000000000000 RSI: dead000000200200 RDI: ffff8800cba8d700
[  166.674230] RBP: ffff8800cae7f7d0 R08: 0000000000000001 R09: 0000000000000001
[  166.674230] R10: 0000000000000000 R11: 000000000000859a R12: ffffffffffffffe8
[  166.674230] R13: ffff8800cba8c5b8 R14: 0000000000000001 R15: ffff8800cba8d700
[  166.674230] FS:  00007fdb5f04a740(0000) GS:ffff88011a800000(0000) knlGS:0000000000000000
[  166.674230] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  166.674230] CR2: 0000000000000000 CR3: 00000000cf929000 CR4: 00000000000006e0
[  166.674230] Stack:
[  166.674230]  ffff8800cae7f7e8 ffffffff814b73e8 ffff8800cba8d6e8 ffff8800cae7f828
[  166.674230]  ffffffff817caeec 0000000000000046 ffff8800cba8c5b0 ffff8800cba8c5b8
[  166.674230]  0000000000000000 0000000000000001 ffff8800cf8e33e8 ffff8800cae7f848
[  166.674230] Call Trace:
[  166.674230]  [<ffffffff814b73e8>] list_del+0xd/0x2b
[  166.674230]  [<ffffffff817caeec>] tcf_action_destroy+0x4c/0x71
[  166.674230]  [<ffffffff817ca0ce>] tcf_exts_destroy+0x20/0x2d
[  166.674230]  [<ffffffff817ec2b5>] tcindex_delete+0x196/0x1b7

struct list_head can not be simply copied and we should always init it.

Cc: John Fastabend <john.r.fastabend@intel.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/sched/cls_tcindex.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index 8d0e83d..30f10fb 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -254,10 +254,15 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
 	cp->tp = tp;
 
 	if (p->perfect) {
+		int i;
+
 		cp->perfect = kmemdup(p->perfect,
 				      sizeof(*r) * cp->hash, GFP_KERNEL);
 		if (!cp->perfect)
 			goto errout;
+		for (i = 0; i < cp->hash; i++)
+			tcf_exts_init(&cp->perfect[i].exts,
+				      TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
 		balloc = 1;
 	}
 	cp->h = p->h;
@@ -353,6 +358,9 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
 		f = kzalloc(sizeof(*f), GFP_KERNEL);
 		if (!f)
 			goto errout_alloc;
+		f->key = handle;
+		tcindex_filter_result_init(&f->result);
+		f->next = NULL;
 	}
 
 	if (tb[TCA_TCINDEX_CLASSID]) {
@@ -376,9 +384,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
 		struct tcindex_filter *nfp;
 		struct tcindex_filter __rcu **fp;
 
-		f->key = handle;
-		f->result = new_filter_result;
-		f->next = NULL;
+		tcf_exts_change(tp, &f->result.exts, &r->exts);
 
 		fp = cp->h + (handle % cp->hash);
 		for (nfp = rtnl_dereference(*fp);
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH] net: bridge: add a br_set_state helper function
From: Florian Fainelli @ 2014-09-30 23:00 UTC (permalink / raw)
  To: netdev; +Cc: davem, bridge, stephen, vyasevich, jiri
In-Reply-To: <1412117947-796-1-git-send-email-f.fainelli@gmail.com>

On 09/30/2014 03:59 PM, Florian Fainelli wrote:
> In preparation for being able to propagate port states to e.g: notifiers
> or other kernel parts, do not manipulate the port state directly, but
> instead use a helper function which will allow us to do a bit more than
> just setting the state.

I sent the wrong version of the patch, but I would still appreciate if
you could comment on the idea. Thanks!

> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  net/bridge/br_if.c        |  2 +-
>  net/bridge/br_multicast.c |  2 +-
>  net/bridge/br_netlink.c   |  2 +-
>  net/bridge/br_private.h   |  1 +
>  net/bridge/br_stp.c       | 15 ++++++++++-----
>  net/bridge/br_stp_if.c    |  4 ++--
>  net/bridge/br_stp_timer.c |  4 ++--
>  7 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index a9f54a9b6690..7b7289ca2992 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -332,7 +332,7 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
>  	p->port_no = index;
>  	p->flags = BR_LEARNING | BR_FLOOD;
>  	br_init_port(p);
> -	p->state = BR_STATE_DISABLED;
> +	br_set_state(p, BR_STATE_DISABLED);
>  	br_stp_port_timer_init(p);
>  	br_multicast_add_port(p);
>  
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 648d79ccf462..1f4fd22c84fc 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -645,7 +645,7 @@ struct net_bridge_port_group *br_multicast_new_port_group(
>  
>  	p->addr = *group;
>  	p->port = port;
> -	p->state = state;
> +	br_set_state(p, state);
>  	rcu_assign_pointer(p->next, next);
>  	hlist_add_head(&p->mglist, &port->mglist);
>  	setup_timer(&p->timer, br_multicast_port_group_expired,
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 0fa66b83685f..2ff9706647f2 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -301,7 +301,7 @@ static int br_set_port_state(struct net_bridge_port *p, u8 state)
>  	    (!netif_oper_up(p->dev) && state != BR_STATE_DISABLED))
>  		return -ENETDOWN;
>  
> -	p->state = state;
> +	br_set_state(p, state);
>  	br_log_state(p);
>  	br_port_state_selection(p->br);
>  	return 0;
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index f53592fc3ef9..4ff82fc0e79c 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -766,6 +766,7 @@ static inline void br_nf_core_fini(void) {}
>  
>  /* br_stp.c */
>  void br_log_state(const struct net_bridge_port *p);
> +void br_set_stp_state(const struct net_bridge_port *p);
>  struct net_bridge_port *br_get_port(struct net_bridge *br, u16 port_no);
>  void br_init_port(struct net_bridge_port *p);
>  void br_become_designated_port(struct net_bridge_port *p);
> diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
> index 3c86f0538cbb..a859373b7d68 100644
> --- a/net/bridge/br_stp.c
> +++ b/net/bridge/br_stp.c
> @@ -36,6 +36,11 @@ void br_log_state(const struct net_bridge_port *p)
>  		br_port_state_names[p->state]);
>  }
>  
> +void br_set_state(const struct net_bridge_port *p, unsigned int state)
> +{
> +	p->state = state;
> +}
> +
>  /* called under bridge lock */
>  struct net_bridge_port *br_get_port(struct net_bridge *br, u16 port_no)
>  {
> @@ -107,7 +112,7 @@ static void br_root_port_block(const struct net_bridge *br,
>  	br_notice(br, "port %u(%s) tried to become root port (blocked)",
>  		  (unsigned int) p->port_no, p->dev->name);
>  
> -	p->state = BR_STATE_LISTENING;
> +	br_set_state(p, BR_STATE_LISTENING);
>  	br_log_state(p);
>  	br_ifinfo_notify(RTM_NEWLINK, p);
>  
> @@ -387,7 +392,7 @@ static void br_make_blocking(struct net_bridge_port *p)
>  		    p->state == BR_STATE_LEARNING)
>  			br_topology_change_detection(p->br);
>  
> -		p->state = BR_STATE_BLOCKING;
> +		br_set_state(p, BR_STATE_BLOCKING);
>  		br_log_state(p);
>  		br_ifinfo_notify(RTM_NEWLINK, p);
>  
> @@ -404,13 +409,13 @@ static void br_make_forwarding(struct net_bridge_port *p)
>  		return;
>  
>  	if (br->stp_enabled == BR_NO_STP || br->forward_delay == 0) {
> -		p->state = BR_STATE_FORWARDING;
> +		br_set_state(p, BR_STATE_FORWARDING);
>  		br_topology_change_detection(br);
>  		del_timer(&p->forward_delay_timer);
>  	} else if (br->stp_enabled == BR_KERNEL_STP)
> -		p->state = BR_STATE_LISTENING;
> +		br_set_state(p, BR_STATE_LISTENING);
>  	else
> -		p->state = BR_STATE_LEARNING;
> +		br_set_state(p, BR_STATE_LEARNING);
>  
>  	br_multicast_enable_port(p);
>  	br_log_state(p);
> diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
> index 189ba1e7d851..41146872c1b4 100644
> --- a/net/bridge/br_stp_if.c
> +++ b/net/bridge/br_stp_if.c
> @@ -37,7 +37,7 @@ void br_init_port(struct net_bridge_port *p)
>  {
>  	p->port_id = br_make_port_id(p->priority, p->port_no);
>  	br_become_designated_port(p);
> -	p->state = BR_STATE_BLOCKING;
> +	br_set_state(p, BR_STATE_BLOCKING);
>  	p->topology_change_ack = 0;
>  	p->config_pending = 0;
>  }
> @@ -100,7 +100,7 @@ void br_stp_disable_port(struct net_bridge_port *p)
>  
>  	wasroot = br_is_root_bridge(br);
>  	br_become_designated_port(p);
> -	p->state = BR_STATE_DISABLED;
> +	br_set_state(p, BR_STATE_DISABLED);
>  	p->topology_change_ack = 0;
>  	p->config_pending = 0;
>  
> diff --git a/net/bridge/br_stp_timer.c b/net/bridge/br_stp_timer.c
> index 558c46d19e05..4fcaa67750fd 100644
> --- a/net/bridge/br_stp_timer.c
> +++ b/net/bridge/br_stp_timer.c
> @@ -87,11 +87,11 @@ static void br_forward_delay_timer_expired(unsigned long arg)
>  		 (unsigned int) p->port_no, p->dev->name);
>  	spin_lock(&br->lock);
>  	if (p->state == BR_STATE_LISTENING) {
> -		p->state = BR_STATE_LEARNING;
> +		br_set_state(p, BR_STATE_LEARNING);
>  		mod_timer(&p->forward_delay_timer,
>  			  jiffies + br->forward_delay);
>  	} else if (p->state == BR_STATE_LEARNING) {
> -		p->state = BR_STATE_FORWARDING;
> +		br_set_state(p, BR_STATE_FORWARDING);
>  		if (br_is_designated_for_some_port(br))
>  			br_topology_change_detection(br);
>  		netif_carrier_on(br->dev);
> 

^ permalink raw reply

* [PATCH] net: bridge: add a br_set_state helper function
From: Florian Fainelli @ 2014-09-30 22:59 UTC (permalink / raw)
  To: netdev; +Cc: davem, bridge, stephen, vyasevich, jiri, Florian Fainelli

In preparation for being able to propagate port states to e.g: notifiers
or other kernel parts, do not manipulate the port state directly, but
instead use a helper function which will allow us to do a bit more than
just setting the state.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/bridge/br_if.c        |  2 +-
 net/bridge/br_multicast.c |  2 +-
 net/bridge/br_netlink.c   |  2 +-
 net/bridge/br_private.h   |  1 +
 net/bridge/br_stp.c       | 15 ++++++++++-----
 net/bridge/br_stp_if.c    |  4 ++--
 net/bridge/br_stp_timer.c |  4 ++--
 7 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index a9f54a9b6690..7b7289ca2992 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -332,7 +332,7 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
 	p->port_no = index;
 	p->flags = BR_LEARNING | BR_FLOOD;
 	br_init_port(p);
-	p->state = BR_STATE_DISABLED;
+	br_set_state(p, BR_STATE_DISABLED);
 	br_stp_port_timer_init(p);
 	br_multicast_add_port(p);
 
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 648d79ccf462..1f4fd22c84fc 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -645,7 +645,7 @@ struct net_bridge_port_group *br_multicast_new_port_group(
 
 	p->addr = *group;
 	p->port = port;
-	p->state = state;
+	br_set_state(p, state);
 	rcu_assign_pointer(p->next, next);
 	hlist_add_head(&p->mglist, &port->mglist);
 	setup_timer(&p->timer, br_multicast_port_group_expired,
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 0fa66b83685f..2ff9706647f2 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -301,7 +301,7 @@ static int br_set_port_state(struct net_bridge_port *p, u8 state)
 	    (!netif_oper_up(p->dev) && state != BR_STATE_DISABLED))
 		return -ENETDOWN;
 
-	p->state = state;
+	br_set_state(p, state);
 	br_log_state(p);
 	br_port_state_selection(p->br);
 	return 0;
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index f53592fc3ef9..4ff82fc0e79c 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -766,6 +766,7 @@ static inline void br_nf_core_fini(void) {}
 
 /* br_stp.c */
 void br_log_state(const struct net_bridge_port *p);
+void br_set_stp_state(const struct net_bridge_port *p);
 struct net_bridge_port *br_get_port(struct net_bridge *br, u16 port_no);
 void br_init_port(struct net_bridge_port *p);
 void br_become_designated_port(struct net_bridge_port *p);
diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
index 3c86f0538cbb..a859373b7d68 100644
--- a/net/bridge/br_stp.c
+++ b/net/bridge/br_stp.c
@@ -36,6 +36,11 @@ void br_log_state(const struct net_bridge_port *p)
 		br_port_state_names[p->state]);
 }
 
+void br_set_state(const struct net_bridge_port *p, unsigned int state)
+{
+	p->state = state;
+}
+
 /* called under bridge lock */
 struct net_bridge_port *br_get_port(struct net_bridge *br, u16 port_no)
 {
@@ -107,7 +112,7 @@ static void br_root_port_block(const struct net_bridge *br,
 	br_notice(br, "port %u(%s) tried to become root port (blocked)",
 		  (unsigned int) p->port_no, p->dev->name);
 
-	p->state = BR_STATE_LISTENING;
+	br_set_state(p, BR_STATE_LISTENING);
 	br_log_state(p);
 	br_ifinfo_notify(RTM_NEWLINK, p);
 
@@ -387,7 +392,7 @@ static void br_make_blocking(struct net_bridge_port *p)
 		    p->state == BR_STATE_LEARNING)
 			br_topology_change_detection(p->br);
 
-		p->state = BR_STATE_BLOCKING;
+		br_set_state(p, BR_STATE_BLOCKING);
 		br_log_state(p);
 		br_ifinfo_notify(RTM_NEWLINK, p);
 
@@ -404,13 +409,13 @@ static void br_make_forwarding(struct net_bridge_port *p)
 		return;
 
 	if (br->stp_enabled == BR_NO_STP || br->forward_delay == 0) {
-		p->state = BR_STATE_FORWARDING;
+		br_set_state(p, BR_STATE_FORWARDING);
 		br_topology_change_detection(br);
 		del_timer(&p->forward_delay_timer);
 	} else if (br->stp_enabled == BR_KERNEL_STP)
-		p->state = BR_STATE_LISTENING;
+		br_set_state(p, BR_STATE_LISTENING);
 	else
-		p->state = BR_STATE_LEARNING;
+		br_set_state(p, BR_STATE_LEARNING);
 
 	br_multicast_enable_port(p);
 	br_log_state(p);
diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index 189ba1e7d851..41146872c1b4 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -37,7 +37,7 @@ void br_init_port(struct net_bridge_port *p)
 {
 	p->port_id = br_make_port_id(p->priority, p->port_no);
 	br_become_designated_port(p);
-	p->state = BR_STATE_BLOCKING;
+	br_set_state(p, BR_STATE_BLOCKING);
 	p->topology_change_ack = 0;
 	p->config_pending = 0;
 }
@@ -100,7 +100,7 @@ void br_stp_disable_port(struct net_bridge_port *p)
 
 	wasroot = br_is_root_bridge(br);
 	br_become_designated_port(p);
-	p->state = BR_STATE_DISABLED;
+	br_set_state(p, BR_STATE_DISABLED);
 	p->topology_change_ack = 0;
 	p->config_pending = 0;
 
diff --git a/net/bridge/br_stp_timer.c b/net/bridge/br_stp_timer.c
index 558c46d19e05..4fcaa67750fd 100644
--- a/net/bridge/br_stp_timer.c
+++ b/net/bridge/br_stp_timer.c
@@ -87,11 +87,11 @@ static void br_forward_delay_timer_expired(unsigned long arg)
 		 (unsigned int) p->port_no, p->dev->name);
 	spin_lock(&br->lock);
 	if (p->state == BR_STATE_LISTENING) {
-		p->state = BR_STATE_LEARNING;
+		br_set_state(p, BR_STATE_LEARNING);
 		mod_timer(&p->forward_delay_timer,
 			  jiffies + br->forward_delay);
 	} else if (p->state == BR_STATE_LEARNING) {
-		p->state = BR_STATE_FORWARDING;
+		br_set_state(p, BR_STATE_FORWARDING);
 		if (br_is_designated_for_some_port(br))
 			br_topology_change_detection(br);
 		netif_carrier_on(br->dev);
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH] hostap: proc: Remove unused variable
From: Mark Brown @ 2014-09-30 22:56 UTC (permalink / raw)
  To: John W. Linville
  Cc: Jouni Malinen, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linaro-kernel-cunTk1MwBs8s++Sfvej+rw
In-Reply-To: <20140930165626.GD11919-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 440 bytes --]

On Tue, Sep 30, 2014 at 12:56:26PM -0400, John W. Linville wrote:
> On Tue, Sep 16, 2014 at 04:16:55PM -0700, Mark Brown wrote:

> > Since "hostap: proc: substitute loops by %*phN" (62c5afb8e388) the
> > variable i has been unused in prism2_bss_list_proc_show() so remove it.

> What tree are you using?  In wireless-next, I still see a loop in
> that function using i as an index variable.

This was in -next for whatever day I sent that.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply

* Re: [PATCH net-next 07/10] r8169:change function name of function "rtl_w1w0_eri"
From: Francois Romieu @ 2014-09-30 22:37 UTC (permalink / raw)
  To: Chun-Hao Lin; +Cc: netdev, nic_swsd, linux-kernel
In-Reply-To: <1412089230-10208-7-git-send-email-hau@realtek.com>

Chun-Hao Lin <hau@realtek.com> :
> Change the name of this function to "rtl_w0_w1_eri".
> It is more suitable for this function's behavior.

Afaiks it used to follow the same rule as the one I outlined in the
comment to #6/10 for rtl_w0_w1_phy.

Could you elaborate (or say so if it should be clear for me after
some sleep) ?

[...]
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index e68fe5e..e79def5 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
[...]
> @@ -1629,14 +1629,14 @@ static void __rtl8169_set_wol(struct rtl8169_private *tp, u32 wolopts)
>  	case RTL_GIGA_MAC_VER_48:
>  		tmp = ARRAY_SIZE(cfg) - 1;
>  		if (wolopts & WAKE_MAGIC)
> -			rtl_w1w0_eri(tp,
> +			rtl_w0w1_eri(tp,
>  				     0x0dc,
>  				     ERIAR_MASK_0100,
>  				     MagicPacket_v2,
>  				     0x0000,
>  				     ERIAR_EXGMAC);
>  		else
> -			rtl_w1w0_eri(tp,
> +			rtl_w0w1_eri(tp,
>  				     0x0dc,
>  				     ERIAR_MASK_0100,
>  				     0x0000,

Please turn these into:

			rtl_..._eri(tp, 0x0dc, ERIAR_MASK_0100, MagicPacket_v2,
 				    0x0000, ERIAR_EXGMAC);

-- 
Ueimor

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox