Re: pipex(4): kill pipexintr()

2020-08-04 Thread Vitaliy Makkoveev
On Tue, Aug 04, 2020 at 01:53:55PM +0900, YASUOKA Masahiko wrote: > On Mon, 3 Aug 2020 23:36:09 +0300 > Vitaliy Makkoveev wrote: > > On Tue, Aug 04, 2020 at 01:26:14AM +0900, YASUOKA Masahiko wrote: > >> Comments? > > > > You introduce `cookie' as > > > > cookie = session->protocol << 16 |

Re: pipex(4): kill pipexintr()

2020-08-03 Thread YASUOKA Masahiko
On Mon, 3 Aug 2020 23:36:09 +0300 Vitaliy Makkoveev wrote: > On Tue, Aug 04, 2020 at 01:26:14AM +0900, YASUOKA Masahiko wrote: >> Comments? > > You introduce `cookie' as > > cookie = session->protocol << 16 | session->session_id; > > also multicast sessions initialized as > >

Re: pipex(4): kill pipexintr()

2020-08-03 Thread Vitaliy Makkoveev
On Tue, Aug 04, 2020 at 01:26:14AM +0900, YASUOKA Masahiko wrote: > On Sat, 1 Aug 2020 18:52:27 +0300 > Vitaliy Makkoveev wrote: > > On Sat, Aug 01, 2020 at 07:44:17PM +0900, YASUOKA Masahiko wrote: > >> I'm not sure when it is broken, in old versions, it was assumed the > >> pipex queues are

Re: pipex(4): kill pipexintr()

2020-08-03 Thread YASUOKA Masahiko
On Sat, 1 Aug 2020 18:52:27 +0300 Vitaliy Makkoveev wrote: > On Sat, Aug 01, 2020 at 07:44:17PM +0900, YASUOKA Masahiko wrote: >> I'm not sure when it is broken, in old versions, it was assumed the >> pipex queues are empty when pipex_iface_stop() is called. The problem >> mvs@ found is the

Re: pipex(4): kill pipexintr()

2020-08-01 Thread Vitaliy Makkoveev
On Sat, Aug 01, 2020 at 07:44:17PM +0900, YASUOKA Masahiko wrote: > Hi, > > I'm not sure when it is broken, in old versions, it was assumed the > pipex queues are empty when pipex_iface_stop() is called. The problem > mvs@ found is the assumption is not true any more. > > pipex has a mechanism

Re: pipex(4): kill pipexintr()

2020-08-01 Thread YASUOKA Masahiko
Hi, I'm not sure when it is broken, in old versions, it was assumed the pipex queues are empty when pipex_iface_stop() is called. The problem mvs@ found is the assumption is not true any more. pipex has a mechanism that delete a session when the queues are empty. 819 Static void 820

Re: pipex(4): kill pipexintr()

2020-07-31 Thread Vitaliy Makkoveev
On Fri, Jul 31, 2020 at 10:25:48PM +0200, Martin Pieuchot wrote: > On 31/07/20(Fri) 21:58, Vitaliy Makkoveev wrote: > > [...] > > What denies us to move pipex(4) under it's own lock? > > Such question won't lead us anywhere. It assumes it makes sense to move > pipex under its own lock. This

Re: pipex(4): kill pipexintr()

2020-07-31 Thread Martin Pieuchot
On 31/07/20(Fri) 21:58, Vitaliy Makkoveev wrote: > [...] > What denies us to move pipex(4) under it's own lock? Such question won't lead us anywhere. It assumes it makes sense to move pipex under its own lock. This assumption has many drawback which clearly haven't been studied and more

Re: pipex(4): kill pipexintr()

2020-07-31 Thread Vitaliy Makkoveev
Well, since pipexintr() killing was rejected, I propose to implement reference counters to protect pipex(4) session itself. Diff below does this. Index: sys/net/if_ethersubr.c === RCS file: /cvs/src/sys/net/if_ethersubr.c,v

Re: pipex(4): kill pipexintr()

2020-07-31 Thread Vitaliy Makkoveev
On Fri, Jul 31, 2020 at 08:26:22PM +0200, Martin Pieuchot wrote: > On 31/07/20(Fri) 12:15, Vitaliy Makkoveev wrote: > > On Fri, Jul 31, 2020 at 09:36:32AM +0900, YASUOKA Masahiko wrote: > > > On Thu, 30 Jul 2020 22:43:10 +0300 > > > Vitaliy Makkoveev wrote: > > > > On Thu, Jul 30, 2020 at

Re: pipex(4): kill pipexintr()

2020-07-31 Thread Martin Pieuchot
On 31/07/20(Fri) 12:15, Vitaliy Makkoveev wrote: > On Fri, Jul 31, 2020 at 09:36:32AM +0900, YASUOKA Masahiko wrote: > > On Thu, 30 Jul 2020 22:43:10 +0300 > > Vitaliy Makkoveev wrote: > > > On Thu, Jul 30, 2020 at 10:05:13PM +0900, YASUOKA Masahiko wrote: > > >> On Thu, 30 Jul 2020 15:34:09

Re: pipex(4): kill pipexintr()

2020-07-31 Thread Vitaliy Makkoveev
On Fri, Jul 31, 2020 at 09:36:32AM +0900, YASUOKA Masahiko wrote: > On Thu, 30 Jul 2020 22:43:10 +0300 > Vitaliy Makkoveev wrote: > > On Thu, Jul 30, 2020 at 10:05:13PM +0900, YASUOKA Masahiko wrote: > >> On Thu, 30 Jul 2020 15:34:09 +0300 > >> Vitaliy Makkoveev wrote: > >> > On Thu, Jul 30,

Re: pipex(4): kill pipexintr()

2020-07-31 Thread Martin Pieuchot
On 30/07/20(Thu) 21:13, YASUOKA Masahiko wrote: > sys/net/if_ethersubr.c: > 372 void > 373 ether_input(struct ifnet *ifp, struct mbuf *m) > (snip) > 519 #if NPPPOE > 0 || defined(PIPEX) > 520 case ETHERTYPE_PPPOEDISC: > 521 case ETHERTYPE_PPPOE: > 522 if (m->m_flags

Re: pipex(4): kill pipexintr()

2020-07-30 Thread YASUOKA Masahiko
On Thu, 30 Jul 2020 22:43:10 +0300 Vitaliy Makkoveev wrote: > On Thu, Jul 30, 2020 at 10:05:13PM +0900, YASUOKA Masahiko wrote: >> On Thu, 30 Jul 2020 15:34:09 +0300 >> Vitaliy Makkoveev wrote: >> > On Thu, Jul 30, 2020 at 09:13:46PM +0900, YASUOKA Masahiko wrote: >> >> If the diff removes the

Re: pipex(4): kill pipexintr()

2020-07-30 Thread YASUOKA Masahiko
On Thu, 30 Jul 2020 22:43:10 +0300 Vitaliy Makkoveev wrote: > On Thu, Jul 30, 2020 at 10:05:13PM +0900, YASUOKA Masahiko wrote: >> On Thu, 30 Jul 2020 15:34:09 +0300 >> Vitaliy Makkoveev wrote: >> > On Thu, Jul 30, 2020 at 09:13:46PM +0900, YASUOKA Masahiko wrote: >> >> Hi, >> >> >> >>

Re: pipex(4): kill pipexintr()

2020-07-30 Thread Vitaliy Makkoveev
On Thu, Jul 30, 2020 at 10:05:13PM +0900, YASUOKA Masahiko wrote: > On Thu, 30 Jul 2020 15:34:09 +0300 > Vitaliy Makkoveev wrote: > > On Thu, Jul 30, 2020 at 09:13:46PM +0900, YASUOKA Masahiko wrote: > >> Hi, > >> > >> sys/net/if_ethersubr.c: > >> 372 void > >> 373 ether_input(struct ifnet *ifp,

Re: pipex(4): kill pipexintr()

2020-07-30 Thread YASUOKA Masahiko
On Thu, 30 Jul 2020 15:34:09 +0300 Vitaliy Makkoveev wrote: > On Thu, Jul 30, 2020 at 09:13:46PM +0900, YASUOKA Masahiko wrote: >> Hi, >> >> sys/net/if_ethersubr.c: >> 372 void >> 373 ether_input(struct ifnet *ifp, struct mbuf *m) >> (snip) >> 519 #if NPPPOE > 0 || defined(PIPEX) >> 520

Re: pipex(4): kill pipexintr()

2020-07-30 Thread Vitaliy Makkoveev
On Thu, Jul 30, 2020 at 09:13:46PM +0900, YASUOKA Masahiko wrote: > Hi, > > sys/net/if_ethersubr.c: > 372 void > 373 ether_input(struct ifnet *ifp, struct mbuf *m) > (snip) > 519 #if NPPPOE > 0 || defined(PIPEX) > 520 case ETHERTYPE_PPPOEDISC: > 521 case ETHERTYPE_PPPOE: > 522

Re: pipex(4): kill pipexintr()

2020-07-30 Thread YASUOKA Masahiko
Hi, sys/net/if_ethersubr.c: 372 void 373 ether_input(struct ifnet *ifp, struct mbuf *m) (snip) 519 #if NPPPOE > 0 || defined(PIPEX) 520 case ETHERTYPE_PPPOEDISC: 521 case ETHERTYPE_PPPOE: 522 if (m->m_flags & (M_MCAST | M_BCAST)) 523 goto

Re: pipex(4): kill pipexintr()

2020-07-30 Thread Martin Pieuchot
On 29/07/20(Wed) 23:04, Vitaliy Makkoveev wrote: > Now pipex(4) is fully covered by NET_LOCK() and this is documented. But > we still have an issue with pipex(4) session itself and I guess it's > time to fix it. > > We have `pipexinq' and `pipexoutq' mbuf(9) queues to store mbufs. Each > mbuf(9)

pipex(4): kill pipexintr()

2020-07-29 Thread Vitaliy Makkoveev
Now pipex(4) is fully covered by NET_LOCK() and this is documented. But we still have an issue with pipex(4) session itself and I guess it's time to fix it. We have `pipexinq' and `pipexoutq' mbuf(9) queues to store mbufs. Each mbuf(9) passed to these queues stores the pointer to corresponding

Re: pipex(4): kill pipexintr()

2020-07-11 Thread Vitaliy Makkoveev
On Fri, Jul 10, 2020 at 10:54:44AM +0200, Martin Pieuchot wrote: > On 07/07/20(Tue) 01:01, Vitaliy Makkoveev wrote: > > On Mon, Jul 06, 2020 at 08:47:23PM +0200, Martin Pieuchot wrote: > > > On 06/07/20(Mon) 19:23, Vitaliy Makkoveev wrote: > > > > > On 6 Jul 2020, at 17:36, Martin Pieuchot wrote:

Re: pipex(4): kill pipexintr()

2020-07-10 Thread Martin Pieuchot
On 07/07/20(Tue) 01:01, Vitaliy Makkoveev wrote: > On Mon, Jul 06, 2020 at 08:47:23PM +0200, Martin Pieuchot wrote: > > On 06/07/20(Mon) 19:23, Vitaliy Makkoveev wrote: > > > > On 6 Jul 2020, at 17:36, Martin Pieuchot wrote: > > > [...] > > > Unfortunately you can’t be sure about NET_LOCK()

Re: pipex(4): kill pipexintr()

2020-07-06 Thread Vitaliy Makkoveev
On Mon, Jul 06, 2020 at 08:47:23PM +0200, Martin Pieuchot wrote: > On 06/07/20(Mon) 19:23, Vitaliy Makkoveev wrote: > > > On 6 Jul 2020, at 17:36, Martin Pieuchot wrote: > > [...] > > Unfortunately you can’t be sure about NET_LOCK() status while you are > > in pppac_start(). It was described at

Re: pipex(4): kill pipexintr()

2020-07-06 Thread Vitaliy Makkoveev
> On 6 Jul 2020, at 17:36, Martin Pieuchot wrote: > > On 06/07/20(Mon) 16:42, Vitaliy Makkoveev wrote: >> [...] >> pipex(4) is simultaneously locked by NET_LOCK() and KERNEL_LOCK() but >> with two exceptions: >> >> 1. As you pointed pipex_pppoe_input() called without KERNEL_LOCK() held. >> 2.

Re: pipex(4): kill pipexintr()

2020-07-06 Thread Martin Pieuchot
On 06/07/20(Mon) 19:23, Vitaliy Makkoveev wrote: > > On 6 Jul 2020, at 17:36, Martin Pieuchot wrote: > [...] > Unfortunately you can’t be sure about NET_LOCK() status while you are > in pppac_start(). It was described at this thread [1]. > > We have two cases: > 1. pppac_start() called from

Re: pipex(4): kill pipexintr()

2020-07-06 Thread Vitaliy Makkoveev
On Mon, Jul 06, 2020 at 10:59:17AM +0200, Martin Pieuchot wrote: > On 01/07/20(Wed) 22:42, Vitaliy Makkoveev wrote: > > pipex(4) has 2 mbuf queues: `pipexinq' and `pipexoutq'. When mbuf passed > > to pipex it goes to one of these queues and pipexintr() will be > > scheduled to process them.

Re: pipex(4): kill pipexintr()

2020-07-06 Thread Martin Pieuchot
On 06/07/20(Mon) 16:42, Vitaliy Makkoveev wrote: > [...] > pipex(4) is simultaneously locked by NET_LOCK() and KERNEL_LOCK() but > with two exceptions: > > 1. As you pointed pipex_pppoe_input() called without KERNEL_LOCK() held. > 2. pppac_start() called without NET_LOCK() held. Or with

Re: pipex(4): kill pipexintr()

2020-07-06 Thread Martin Pieuchot
On 01/07/20(Wed) 22:42, Vitaliy Makkoveev wrote: > pipex(4) has 2 mbuf queues: `pipexinq' and `pipexoutq'. When mbuf passed > to pipex it goes to one of these queues and pipexintr() will be > scheduled to process them. pipexintr() called from `netisr' context. > > It's true for pppac(4) but for

pipex(4): kill pipexintr()

2020-07-01 Thread Vitaliy Makkoveev
pipex(4) has 2 mbuf queues: `pipexinq' and `pipexoutq'. When mbuf passed to pipex it goes to one of these queues and pipexintr() will be scheduled to process them. pipexintr() called from `netisr' context. It's true for pppac(4) but for pppx(4) only incoming mbufs go to `pipexinq. Outgoing mbufs