On Fri, Mar 16, 2018 at 09:02:40PM +0100, Guillaume Nault wrote:
> On Fri, Mar 16, 2018 at 02:49:40PM +0800, xu heng wrote:
> >
> > For testing, in __ppp_channel_push(), disable sending anything from
> > the attached unit, just disable __ppp_xmit_process(ppp) in
> > __ppp_channel_push(). In my opinion, __ppp_xmit_process() should only
> > called by ppp_xmit_process(), because of ppp->xmit_recursion __percpu.
> >
> ppp_channel_push() needs to call __ppp_xmit_process() because some
> drivers (like ppp_async) need to notify ppp_generic when they can
> accept new packets. This is done by ppp_output_wakeup() which then
> calls ppp_channel_push(). So we have to handle the unit backlog there.
>
>
> Please try the following patch (untested).
>
FYI, I've now fully tested the patch. I'm going to send it formally.
> diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
> index b883af93929c..af22eb11bbaa 100644
> --- a/drivers/net/ppp/ppp_generic.c
> +++ b/drivers/net/ppp/ppp_generic.c
> @@ -255,7 +255,7 @@ struct ppp_net {
> /* Prototypes. */
> static int ppp_unattached_ioctl(struct net *net, struct ppp_file *pf,
> struct file *file, unsigned int cmd, unsigned long arg);
> -static void ppp_xmit_process(struct ppp *ppp);
> +static void ppp_xmit_process(struct ppp *ppp, struct sk_buff *skb);
> static void ppp_send_frame(struct ppp *ppp, struct sk_buff *skb);
> static void ppp_push(struct ppp *ppp);
> static void ppp_channel_push(struct channel *pch);
> @@ -511,13 +511,12 @@ static ssize_t ppp_write(struct file *file, const char
> __user *buf,
> goto out;
> }
>
> - skb_queue_tail(&pf->xq, skb);
> -
> switch (pf->kind) {
> case INTERFACE:
> - ppp_xmit_process(PF_TO_PPP(pf));
> + ppp_xmit_process(PF_TO_PPP(pf), skb);
> break;
> case CHANNEL:
> + skb_queue_tail(&pf->xq, skb);
> ppp_channel_push(PF_TO_CHANNEL(pf));
> break;
> }
> @@ -1260,8 +1259,8 @@ ppp_start_xmit(struct sk_buff *skb, struct net_device
> *dev)
> put_unaligned_be16(proto, pp);
>
> skb_scrub_packet(skb, !net_eq(ppp->ppp_net, dev_net(dev)));
> - skb_queue_tail(&ppp->file.xq, skb);
> - ppp_xmit_process(ppp);
> + ppp_xmit_process(ppp, skb);
> +
> return NETDEV_TX_OK;
>
> outf:
> @@ -1415,13 +1414,14 @@ static void ppp_setup(struct net_device *dev)
> */
>
> /* Called to do any work queued up on the transmit side that can now be done
> */
> -static void __ppp_xmit_process(struct ppp *ppp)
> +static void __ppp_xmit_process(struct ppp *ppp, struct sk_buff *skb)
> {
> - struct sk_buff *skb;
> -
> ppp_xmit_lock(ppp);
> if (!ppp->closing) {
> ppp_push(ppp);
> +
> + if (skb)
> + skb_queue_tail(&ppp->file.xq, skb);
> while (!ppp->xmit_pending &&
> (skb = skb_dequeue(&ppp->file.xq)))
> ppp_send_frame(ppp, skb);
> @@ -1435,7 +1435,7 @@ static void __ppp_xmit_process(struct ppp *ppp)
> ppp_xmit_unlock(ppp);
> }
>
> -static void ppp_xmit_process(struct ppp *ppp)
> +static void ppp_xmit_process(struct ppp *ppp, struct sk_buff *skb)
> {
> local_bh_disable();
>
> @@ -1443,7 +1443,7 @@ static void ppp_xmit_process(struct ppp *ppp)
> goto err;
>
> (*this_cpu_ptr(ppp->xmit_recursion))++;
> - __ppp_xmit_process(ppp);
> + __ppp_xmit_process(ppp, skb);
> (*this_cpu_ptr(ppp->xmit_recursion))--;
>
> local_bh_enable();
> @@ -1452,6 +1452,7 @@ static void ppp_xmit_process(struct ppp *ppp)
>
> err:
> local_bh_enable();
> + kfree_skb(skb);
>
> if (net_ratelimit())
> netdev_err(ppp->dev, "recursion detected\n");
> @@ -1937,7 +1938,7 @@ static void __ppp_channel_push(struct channel *pch)
> if (skb_queue_empty(&pch->file.xq)) {
> ppp = pch->ppp;
> if (ppp)
> - __ppp_xmit_process(ppp);
> + __ppp_xmit_process(ppp, NULL);
> }
> }
>
>
