inpcb: Introduce INP_HASH_ITERATOR

[markj]

Summary

Use it in the raw IP code instead of mucking around with iterator
entrails. This makes it a bit easier to find places where we use this
method of iterating over the pcbinfo hash table.

No functional change intended.

[glebius]

My original idea about the iterator was that it should have all fields const, except the inp pointer. That's why the initializers were without a cast, to force their use only when iterator is originally declared and do not allow reassignment. The iterator could be reused if it finished the iteration (inpi.inp == NULL). The raw_ip.c violates the plan somewhat. The problem with raw_ip.c is not that it is mucking around inpi.hash value. It is heavily mucking around the pcbinfo hash itself as it installs and removes entries. This user of inpcb info is already an exclusion and a violator of many rules. I don't think that it is worth to adopt the KPI to fit it. If you hate the inpi.hash assignment in rip_input() maybe just declare two iterators in this function?

In principle, I think, the raw_ip should just stop using inpcb layer at all, and use its own private pcbs. Raw IP uses only allocation/free KPI of the inpcb KPI. The only is exception the call to in_pcbladdr(). But this function is an exception itself, as it doesn't work with PCB database, it is an address selection function that should be moved to netinet/in.c. Given that, I don't think it is worth to adopt the inpcb KPI for raw IP.

[markj]

In D38390#873224, @glebius wrote:

My original idea about the iterator was that it should have all fields const, except the inp pointer. That's why the initializers were without a cast, to force their use only when iterator is originally declared and do not allow reassignment. The iterator could be reused if it finished the iteration (inpi.inp == NULL). The raw_ip.c violates the plan somewhat. The problem with raw_ip.c is not that it is mucking around inpi.hash value. It is heavily mucking around the pcbinfo hash itself as it installs and removes entries. This user of inpcb info is already an exclusion and a violator of many rules. I don't think that it is worth to adopt the KPI to fit it. If you hate the inpi.hash assignment in rip_input() maybe just declare two iterators in this function?

In principle, I think, the raw_ip should just stop using inpcb layer at all, and use its own private pcbs. Raw IP uses only allocation/free KPI of the inpcb KPI. The only is exception the call to in_pcbladdr(). But this function is an exception itself, as it doesn't work with PCB database, it is an address selection function that should be moved to netinet/in.c. Given that, I don't think it is worth to adopt the inpcb KPI for raw IP.

The reason I looked at this is that I would like to split each hash bucket into two chains, for connected and unconnected inpcbs. This makes inp_next() more complex, but in practice, it never really traverses hash chains. raw_ip is the only thing that tries to do that, but as you say, it abuses the PCB database somewhat, it is not a real consumer.

So the question is, should I make inp_next() more complicated for the sake of raw_ip? I'd prefer not to.

[glebius]

In D38390#873580, @markj wrote:

The reason I looked at this is that I would like to split each hash bucket into two chains, for connected and unconnected inpcbs. This makes inp_next() more complex, but in practice, it never really traverses hash chains. raw_ip is the only thing that tries to do that, but as you say, it abuses the PCB database somewhat, it is not a real consumer.

Got it, thanks!

So the question is, should I make inp_next() more complicated for the sake of raw_ip? I'd prefer not to.

Can you please put whatever hack is needed to make raw_ip work but leave it isolated to raw ip only? I think I will come with a patch that would make raw IP use its own hash. And also do that for raw IPv6, which right now also uses inpcb KPI for allocation/free, but doesn't use the hash. And there is review that makes it use the hash.

[markj]

In D38390#873754, @glebius wrote:

In D38390#873580, @markj wrote:

So the question is, should I make inp_next() more complicated for the sake of raw_ip? I'd prefer not to.

Can you please put whatever hack is needed to make raw_ip work but leave it isolated to raw ip only? I think I will come with a patch that would make raw IP use its own hash. And also do that for raw IPv6, which right now also uses inpcb KPI for allocation/free, but doesn't use the hash. And there is review that makes it use the hash.

That works for me!