netdev - Re: [STRAW MAN PATCH] sch_teql doesn't load-balance ppp(oatm) slaves

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 27 Mar 2012 21:55:38 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	David Woodhouse <dwmw2@...radead.org>
Cc:	David Miller <davem@...emloft.net>, paulus@...ba.org,
	netdev@...r.kernel.org
Subject: Re: [STRAW MAN PATCH] sch_teql doesn't load-balance ppp(oatm)
 slaves

On Tue, 2012-03-27 at 20:10 +0100, David Woodhouse wrote:
> On Sun, 2012-03-25 at 17:36 -0400, David Miller wrote:
> > Yes, the ATM devices deep transmit queue is quite undesirable.
> 
> This should fix that, and while I'm at it should fix the gratuitous
> running of ppp_output_wakeup() from a tasklet on *every* packet, when
> it's almost never necessary. Some careful eyes over the locking issues
> on that would be much appreciated. I've documented how I *think* it
> works...
> 
> I'm tempted to rip out the atm_may_send() bit; there's not a lot of
> point in checking against sk_sndbuf when we're limiting to two packets
> anyway, is there? There's always been a problem here if sk_sndbuf was
> set lower than the MTU of the interface; it would block for ever.
> 
> I'm running this now on my ADSL router. I can watch it working, keeping
> precisely two packets in the queue at a time (one really in-flight and
> one ready for the ATM driver). My leftover debugging in sch_teql is
> triggering when the xmit returns NETDEV_TX_BUSY, and all seems to be
> well.
> 
> --- net/atm/pppoatm.c~	2012-03-27 19:59:54.379565896 +0100
> +++ net/atm/pppoatm.c	2012-03-27 20:03:02.676561017 +0100
> @@ -62,10 +62,13 @@ struct pppoatm_vcc {
>  	void (*old_pop)(struct atm_vcc *, struct sk_buff *);
>  					/* keep old push/pop for detaching */
>  	enum pppoatm_encaps encaps;
> +	atomic_t inflight;
> +	unsigned long blocked;
>  	int flags;			/* SC_COMP_PROT - compress protocol */
>  	struct ppp_channel chan;	/* interface to generic ppp layer */
>  	struct tasklet_struct wakeup_tasklet;
>  };
> +#define BLOCKED 0
>  
>  /*
>   * Header used for LLC Encapsulated PPP (4 bytes) followed by the LCP protocol
> @@ -102,16 +105,31 @@ static void pppoatm_wakeup_sender(unsign
>  static void pppoatm_pop(struct atm_vcc *atmvcc, struct sk_buff *skb)
>  {
>  	struct pppoatm_vcc *pvcc = atmvcc_to_pvcc(atmvcc);
> +
>  	pvcc->old_pop(atmvcc, skb);
> +	smp_mb__before_atomic_dec();

I have no idea why you added all these barriers...

> +	atomic_dec(&pvcc->inflight);
> +
>  	/*


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html