On Fri, Nov 02, 2012 at 10:40:18AM +0100, Krzysztof Mazur wrote:
> On Thu, Nov 01, 2012 at 10:26:28AM -0400, chas williams - CONTRACTOR wrote:
> > On Wed, 31 Oct 2012 23:04:35 +0100
> > Krzysztof Mazur wrote:
> > > - missing check for SS_CONNECTED in pppoatm_ioctl,
> >
> > in practice you will
On Thu, Nov 01, 2012 at 10:26:28AM -0400, chas williams - CONTRACTOR wrote:
> On Wed, 31 Oct 2012 23:04:35 +0100
> Krzysztof Mazur wrote:
>
> > There are also some minor potential issues in pppoatm driver:
> >
> > - locking issues, but now only between pppoatm_send() and
> >
On Thu, Nov 01, 2012 at 10:26:28AM -0400, chas williams - CONTRACTOR wrote:
On Wed, 31 Oct 2012 23:04:35 +0100
Krzysztof Mazur krzys...@podlesie.net wrote:
There are also some minor potential issues in pppoatm driver:
- locking issues, but now only between pppoatm_send() and
On Fri, Nov 02, 2012 at 10:40:18AM +0100, Krzysztof Mazur wrote:
On Thu, Nov 01, 2012 at 10:26:28AM -0400, chas williams - CONTRACTOR wrote:
On Wed, 31 Oct 2012 23:04:35 +0100
Krzysztof Mazur krzys...@podlesie.net wrote:
- missing check for SS_CONNECTED in pppoatm_ioctl,
in practice
On Wed, 31 Oct 2012 23:04:35 +0100
Krzysztof Mazur wrote:
> There are also some minor potential issues in pppoatm driver:
>
> - locking issues, but now only between pppoatm_send() and
> vcc_sendmsg() and maybe some other functions,
these have been around for a while. i agree
On Wed, 31 Oct 2012 23:04:35 +0100
Krzysztof Mazur krzys...@podlesie.net wrote:
There are also some minor potential issues in pppoatm driver:
- locking issues, but now only between pppoatm_send() and
vcc_sendmsg() and maybe some other functions,
these have been around for a
On Wed, Oct 31, 2012 at 04:03:52PM -0400, chas williams - CONTRACTOR wrote:
>
> reversing the order of the push and close certainly seems like the right
> thing to do. i would like to see if it would fix your problem. making
> the minimal change to get something working would be preferred before
On Wed, 31 Oct 2012 10:41:47 +0100
Krzysztof Mazur wrote:
> On Tue, Oct 30, 2012 at 07:20:01PM +0100, Krzysztof Mazur wrote:
> > Yes, this problem can be probably fixed by reversing close and push
> > and adding some synchronization to pppoatm_unassign_vcc(), but I think
> > we need that locking
On Wed, 2012-10-31 at 12:30 +0100, Krzysztof Mazur wrote:
> Yes, original patch had also the same problem with sock_owned_by_user(),
> so I just incorrectly assumed that we can do "goto nospace" after
> pppoatm_may_send(), but ppooatm_may_send() must be the last test.
>
> So I just moved all
On Wed, Oct 31, 2012 at 10:16:18AM +, David Woodhouse wrote:
>
> Does this break the pvcc->blocked handling that coordinates with
> pppoatm_pop()?
>
> If we have one packet in flight, so pppoatm_may_send() permits a new one
> to be queued... but they're *large* packets to sk_wmem_alloc
On Wed, Oct 31, 2012 at 10:41:47AM +0100, Krzysztof Mazur wrote:
>
> I think that we should add a wrapper to vcc->send(), based on
> fixed pppoatm_send(), that performs required checks and takes the ATM socket
> lock.
>
I'm sending initial version of such wrapper and update to pppoatm.
Untested
On Tue, 2012-10-30 at 20:52 +0100, Krzysztof Mazur wrote:
>
> --- a/net/atm/pppoatm.c
> +++ b/net/atm/pppoatm.c
> @@ -306,12 +306,9 @@ static int pppoatm_send(struct ppp_channel *chan, struct
> sk_buff *skb)
>
> /*
> * It's not clear that we need to bother with using
On Tue, Oct 30, 2012 at 07:20:01PM +0100, Krzysztof Mazur wrote:
> On Tue, Oct 30, 2012 at 10:26:46AM -0400, Chas Williams (CONTRACTOR) wrote:
> > In message
> > <1350926091-12642-2-git-send-email-krzys...@podlesie.net>,Krzysztof Mazur
> > writes:
> >
> > as i recall from way back, this
On Tue, Oct 30, 2012 at 07:20:01PM +0100, Krzysztof Mazur wrote:
On Tue, Oct 30, 2012 at 10:26:46AM -0400, Chas Williams (CONTRACTOR) wrote:
In message
1350926091-12642-2-git-send-email-krzys...@podlesie.net,Krzysztof Mazur
writes:
as i recall from way back, this shouldnt be
On Tue, 2012-10-30 at 20:52 +0100, Krzysztof Mazur wrote:
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -306,12 +306,9 @@ static int pppoatm_send(struct ppp_channel *chan, struct
sk_buff *skb)
/*
* It's not clear that we need to bother with using atm_may_send()
-
On Wed, Oct 31, 2012 at 10:41:47AM +0100, Krzysztof Mazur wrote:
I think that we should add a wrapper to vcc-send(), based on
fixed pppoatm_send(), that performs required checks and takes the ATM socket
lock.
I'm sending initial version of such wrapper and update to pppoatm.
Untested but
On Wed, Oct 31, 2012 at 10:16:18AM +, David Woodhouse wrote:
Does this break the pvcc-blocked handling that coordinates with
pppoatm_pop()?
If we have one packet in flight, so pppoatm_may_send() permits a new one
to be queued... but they're *large* packets to sk_wmem_alloc doesn't
On Wed, 2012-10-31 at 12:30 +0100, Krzysztof Mazur wrote:
Yes, original patch had also the same problem with sock_owned_by_user(),
so I just incorrectly assumed that we can do goto nospace after
pppoatm_may_send(), but ppooatm_may_send() must be the last test.
So I just moved all other tests
On Wed, 31 Oct 2012 10:41:47 +0100
Krzysztof Mazur krzys...@podlesie.net wrote:
On Tue, Oct 30, 2012 at 07:20:01PM +0100, Krzysztof Mazur wrote:
Yes, this problem can be probably fixed by reversing close and push
and adding some synchronization to pppoatm_unassign_vcc(), but I think
we
On Wed, Oct 31, 2012 at 04:03:52PM -0400, chas williams - CONTRACTOR wrote:
reversing the order of the push and close certainly seems like the right
thing to do. i would like to see if it would fix your problem. making
the minimal change to get something working would be preferred before
On Tue, Oct 30, 2012 at 08:07:25PM +0100, Krzysztof Mazur wrote:
> On Tue, Oct 30, 2012 at 09:37:48AM +, David Woodhouse wrote:
> >
> > Should we be locking it earlier, so that the atm_may_send() call is also
> > covered by the lock?
>
> Yes, but only to protect against concurent
On Tue, Oct 30, 2012 at 09:37:48AM +, David Woodhouse wrote:
>
> Should we be locking it earlier, so that the atm_may_send() call is also
> covered by the lock?
Yes, but only to protect against concurent vcc_sendmsg().
>
> Either way, it's an obvious improvement on what we had before ???
On Tue, Oct 30, 2012 at 10:26:46AM -0400, Chas Williams (CONTRACTOR) wrote:
> In message
> <1350926091-12642-2-git-send-email-krzys...@podlesie.net>,Krzysztof Mazur
> writes:
>
> as i recall from way back, this shouldnt be necessary. closing a vcc
> for an attached protocol isnt supposed to
In message <1350926091-12642-2-git-send-email-krzys...@podlesie.net>,Krzysztof
Mazur writes:
>The pppoatm_send() calls vcc->send() and now also checks for
>some vcc flags that indicate destroyed vcc without proper locking.
...
>The vcc_sendmsg() uses lock_sock(sk). This lock is used by
On Mon, 2012-10-22 at 19:14 +0200, Krzysztof Mazur wrote:
> The pppoatm_send() calls vcc->send() and now also checks for
> some vcc flags that indicate destroyed vcc without proper locking.
>
> The vcc_sendmsg() uses lock_sock(sk). This lock is used by
> vcc_release(), so vcc_destroy_socket()
On Mon, 2012-10-22 at 19:14 +0200, Krzysztof Mazur wrote:
The pppoatm_send() calls vcc-send() and now also checks for
some vcc flags that indicate destroyed vcc without proper locking.
The vcc_sendmsg() uses lock_sock(sk). This lock is used by
vcc_release(), so vcc_destroy_socket() will not
In message 1350926091-12642-2-git-send-email-krzys...@podlesie.net,Krzysztof
Mazur writes:
The pppoatm_send() calls vcc-send() and now also checks for
some vcc flags that indicate destroyed vcc without proper locking.
...
The vcc_sendmsg() uses lock_sock(sk). This lock is used by
vcc_release(),
On Tue, Oct 30, 2012 at 10:26:46AM -0400, Chas Williams (CONTRACTOR) wrote:
In message
1350926091-12642-2-git-send-email-krzys...@podlesie.net,Krzysztof Mazur
writes:
as i recall from way back, this shouldnt be necessary. closing a vcc
for an attached protocol isnt supposed to require
On Tue, Oct 30, 2012 at 09:37:48AM +, David Woodhouse wrote:
Should we be locking it earlier, so that the atm_may_send() call is also
covered by the lock?
Yes, but only to protect against concurent vcc_sendmsg().
Either way, it's an obvious improvement on what we had before ??? and
On Tue, Oct 30, 2012 at 08:07:25PM +0100, Krzysztof Mazur wrote:
On Tue, Oct 30, 2012 at 09:37:48AM +, David Woodhouse wrote:
Should we be locking it earlier, so that the atm_may_send() call is also
covered by the lock?
Yes, but only to protect against concurent vcc_sendmsg().
The pppoatm_send() calls vcc->send() and now also checks for
some vcc flags that indicate destroyed vcc without proper locking.
The vcc_sendmsg() uses lock_sock(sk). This lock is used by
vcc_release(), so vcc_destroy_socket() will not be called between
check and during ->send(). The
The pppoatm_send() calls vcc-send() and now also checks for
some vcc flags that indicate destroyed vcc without proper locking.
The vcc_sendmsg() uses lock_sock(sk). This lock is used by
vcc_release(), so vcc_destroy_socket() will not be called between
check and during -send(). The
32 matches
Mail list logo
