New frag6_in function (was: CVS commit: src/sys/netinet6)

[Bernd Ernesti]

Hi,

Module Name: src
Committed By: zoltan
Date: Fri Nov 4 00:22:34 UTC 2011
src/sys/netinet6: frag6.c ip6_var.h
Change the IPv6 reassembly mechanism to use mutex(9).
Also add ip6_reass_packet() to be used by NPF.
cvs rdiff -u -r1.49 -r1.50 src/sys/netinet6/frag6.c
cvs rdiff -u -r1.55 -r1.56 src/sys/netinet6/ip6_var.h

I can't comment on the actual mutex changes but the return
handling in the 'new' frag6_in function is questionable for me.

Why did you create a new wrapper around the old frag6_input
and replace the return IPPROTO_DONE with a 'return -1' inside
it?

If you really need a special handling for NPF I would have not
gone this way by creating two wrappers around a renamed
frag6_input. Just create one wrapper (ip6_reass_packet) around
the old frag6_input and describe a little bit in that function
why you need this handling there.
Do the new function belongs in frag6.c or where should it be placed?

Btw searching for frag6_in would also match frag6_insque
so the shortened name is not a good one either.

Bernd


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Zoltan Arnold NAGY]

Hi,

Post by Bernd Ernesti
I can't comment on the actual mutex changes but the return
handling in the 'new' frag6_in function is questionable for me.
Why did you create a new wrapper around the old frag6_input
and replace the return IPPROTO_DONE with a 'return -1' inside
it?

Returning IPPROTO_DONE even in the cases where there was an error
was really questionable. The upper-level caller needs to know if something
happened, usually. The original code was a special case, where ip6_input did not
care about this mostly. But we do.

Post by Bernd Ernesti
If you really need a special handling for NPF I would have not
gone this way by creating two wrappers around a renamed
frag6_input. Just create one wrapper (ip6_reass_packet) around
the old frag6_input and describe a little bit in that function
why you need this handling there.
Do the new function belongs in frag6.c or where should it be placed?

I can't do that, since ip6_input depends on functioning like this.
I'm only half-happy with this change; however, I must retain the
original return-behaviour for the v6 code, but that does not enough
information for NPF.

As always, suggestions welcome.

Post by Bernd Ernesti
Btw searching for frag6_in would also match frag6_insque
so the shortened name is not a good one either.

I'm ok with changing the name, if needed. However, it's static
and not accessible from outside, so should not be a problem.
Someone poking inside the frag6 code should see it immediately.

Best,
Zoltan

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de