On Fri, Jun 26, 2015 at 04:34:06PM +0200, Martin Pieuchot wrote: > On 26/06/15(Fri) 16:00, Alexandr Nedvedicky wrote: > > Hello Martin, > > > > I accept or your comments. I just have few quick notes/questions now. > > > > > 2) I saw that you found some ALTQ leftovers, you have some Solaris > > (2) I think ALTQs leftovers are still in CVS repo, will double check > > anyway. Stack alignment is not Solaris compatibility hack it's sparc > > compatibility. May be your C compiler takes care of this and grants > > 16/32/64 bit stack alignment. I have not examined build process > > that closely yet. > > By "Solaris compatibility" I'm referring to the size of ``sa_family_t'' > and the corresponding changes in "struct pfr_table". > I see. sa_family_t is kind of surprise it's defined as uint16_t on Solaris. PF at various places mixes sa_family_t with u_int8_t, so all af variables on Solaris had to be turned to sa_family_t. Some of those changes leaked backed during merge to current.
> > (3) > > > use atomic operations rather than per-CPU counters or any other > > > solution? I'm also raising this question because some counters are > > can you point me to manual page or source code sample so I can have a look > > how > > to use per-CPU counter? > > There's no such manual. I was more asking about the reason for using > atomic operations. Is it because you're trying to use existing APIs? I'm taking what's available. > > > 5) I'm not sure to understand the goal of the new "pf_refcnt_t" type > > (5) Solaris defines pf_refcnt_t as 64-bit unsigned integer, pf_refcnt_t > > hopes > > to make porting easier. It can be defined as 32-bit on 32-bit machines. > > Using a long on OpenBSD will grantee that the value fits in a register, > so it should be fine. O.K. thanks for clarification. > > > > 7) The PF_SMP_INSERT_WQ() macro to replace SLIST_INSERT() seems over- > > PF_SMP_INSERT_WQ() purpose of those is to allow every CPU/thread to > > operate on its own work-queue of ktables/kentries. The current pf > > uses 'intrusive' link members pfrkt_workq/pfrke_workq in > > pfr_ktable/pfr_kentry. > > The only idea is to stay as much close to current version as possible. > > I understand that you want to stay close to the current version. I'm > just saying that we can also modify the current version to reduce the > size of your diff. I understand. I'll try to restructure the patches to make them easier for review. > > Regards, > Martin
