Hi Janne, I think the way this patch is, it's ghetto, I don't even know if OpenBSD wants to take it on, hence I sent it as a hint. What really would be cool is to find out why exactly the trap 2 happens, because the pppoe code works on a lot of other archs.
My effort is selfish because I want my octeon router doing gateway and not an old i386 router :-), I was in a race with myself to get this going again. But if OpenBSD for whatever reason feels the code is almost acceptable, then the magic number 0x5, 1 can be replaced with a couple of defines perhaps: #define SPPP_AUTH_SEND_FIRST_ROW 0x1 #define SPPP_AUTH_SEND_SECOND_ROW 0x2 #define SPPP_AUTH_SEND_THIRD_ROW 0x4 and then the 0x5 would become (SPPP_AUTH_SEND_FIRST_ROW|SPPP_AUTH_SEND_THIRD_ROW) and the one 1 would become SPPP_AUTH_SEND_FIRST_ROW. We can also rename the seq to row, to make it ever more obvious that the varargs are in pairs and one pair is a row. Looking at this I agree it's ugly. Perhaps it needs to be refactored again or downright fixed (ie. like the other archs, or are they somehow broken too but behave differently?). Best Regards, -peter On Wed, Oct 23, 2019 at 11:47:03AM +0200, Janne Johansson wrote: > Den ons 23 okt. 2019 kl 09:15 skrev Peter J. Philipp <[email protected]>: > > > Hi Holger & Tech, > > > > I have made my octeon router work again and I have a patch. > > > > > Truncated it a lot, leaving the things I reacted on: > > > > - sppp_auth_send(&chap, sp, CHAP_RESPONSE, h->ident, > > - sizeof dsize, (const char *)&dsize, > > + sppp_auth_send(&chap, sp, CHAP_RESPONSE, h->ident, 0x1, > > + 1, dsize, > > > > > > - sppp_auth_send(&chap, sp, CHAP_CHALLENGE, sp->confid[IDX_CHAP], > > - sizeof clen, (const char *)&clen, > > + sppp_auth_send(&chap, sp, CHAP_CHALLENGE, sp->confid[IDX_CHAP], > > 0x1, > > + 1, clen, > > > > > > - sizeof mlen, (const char *)&mlen, > > + 0x1, > > + 1, mlen, > > > > > > - sizeof idlen, (const char *)&idlen, > > - (size_t)idlen, sp->myauth.name, > > - sizeof pwdlen, (const char *)&pwdlen, > > - (size_t)pwdlen, sp->myauth.secret, > > + 0x5, > > + 1, s_id, > > + s_id, sp->myauth.name, > > + 1, s_pwd, > > + s_pwd, sp->myauth.secret, > > > > For all the good this patch might do, it still replaces a lot of things in > the source with names like XYZlen with 0x1 or 0x5, > so it will be much harder later on to figure out why some part is sending 5 > over to the function it calls. 8-/ > Magic constants makes peoples eyes hurt. > > -- > May the most significant bit of your life be positive.
