route_protosw.pr_input

[Maxime Villard]

In net/rtsock.c, I don't understand where we use the 'pr_input' field of
COMPATNAME(route_protosw)[]. To me it can't be used: .pr_input takes a
variadic function, but if you grep through the tree, you can see that
the variadic .pr_input functions we call always take three arguments, as
opposed to raw_input() which takes five.

Therefore raw_input() is never called via .pr_input, and we can set the
field to NULL. Then we can switch all the .pr_input functions to be static
and not variadic.

This will clear a lot of confusion, and will allow us to find problems
related to types -- like this one.

Comments?

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

[Taylor R Campbell]

Date: Wed, 5 Sep 2018 08:21:12 +0200
In net/rtsock.c, I don't understand where we use the 'pr_input' field of
COMPATNAME(route_protosw)[]. To me it can't be used: .pr_input takes a
variadic function, but if you grep through the tree, you can see that
the variadic .pr_input functions we call always take three arguments, as
opposed to raw_input() which takes five.
Therefore raw_input() is never called via .pr_input, and we can set the
field to NULL. Then we can switch all the .pr_input functions to be static
and not variadic.
This will clear a lot of confusion, and will allow us to find problems
related to types -- like this one.

I have been sitting on a patch (joint effort by me and rtr@, mostly
rtr@, a couple years ago) to kill all the variadics in *_input and put
proto-specific types with no casts on them. Haven't tried it in a
while but it worked at some point. There was some objection on
tech-net at the time but I forget the details now.

[Maxime Villard]

Post by Taylor R Campbell

Date: Wed, 5 Sep 2018 08:21:12 +0200
In net/rtsock.c, I don't understand where we use the 'pr_input' field of
COMPATNAME(route_protosw)[]. To me it can't be used: .pr_input takes a
variadic function, but if you grep through the tree, you can see that
the variadic .pr_input functions we call always take three arguments, as
opposed to raw_input() which takes five.
Therefore raw_input() is never called via .pr_input, and we can set the
field to NULL. Then we can switch all the .pr_input functions to be static
and not variadic.
This will clear a lot of confusion, and will allow us to find problems
related to types -- like this one.

proto-specific types with no casts on them. Haven't tried it in a
while but it worked at some point. There was some objection on
tech-net at the time but I forget the details now.

I was more talking about removing the only user (that doesn't actually use) of
the pr_input field, rather than using a different structure.

Your idea is not wrong, but in the end the field is still unused, so let's set
it to NULL, this eliminates the only obstacle to removing variadic functions

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

[Joerg Sonnenberger]

Post by Maxime Villard
In net/rtsock.c, I don't understand where we use the 'pr_input' field of
COMPATNAME(route_protosw)[]. To me it can't be used: .pr_input takes a
variadic function, but if you grep through the tree, you can see that
the variadic .pr_input functions we call always take three arguments, as
opposed to raw_input() which takes five.

ip_encap.c line 363?

Joerg

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

[Maxime Villard]

Post by Joerg Sonnenberger

Post by Maxime Villard
In net/rtsock.c, I don't understand where we use the 'pr_input' field of
COMPATNAME(route_protosw)[]. To me it can't be used: .pr_input takes a
variadic function, but if you grep through the tree, you can see that
the variadic .pr_input functions we call always take three arguments, as
opposed to raw_input() which takes five.

ip_encap.c line 363?
Joerg

That's a different pr_input, this one is in struct encapsw4, which is not
variadic; so it's not related to the problem

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

[Joerg Sonnenberger]

Post by Maxime Villard

Post by Joerg Sonnenberger

Post by Maxime Villard
In net/rtsock.c, I don't understand where we use the 'pr_input' field of
COMPATNAME(route_protosw)[]. To me it can't be used: .pr_input takes a
variadic function, but if you grep through the tree, you can see that
the variadic .pr_input functions we call always take three arguments, as
opposed to raw_input() which takes five.

ip_encap.c line 363?
Joerg

That's a different pr_input, this one is in struct encapsw4, which is not
variadic; so it's not related to the problem

Ah, right. The 5 argument usage was part of netiso.

Joerg

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

[Ryota Ozaki]

Post by Maxime Villard
In net/rtsock.c, I don't understand where we use the 'pr_input' field of
COMPATNAME(route_protosw)[]. To me it can't be used: .pr_input takes a
variadic function, but if you grep through the tree, you can see that
the variadic .pr_input functions we call always take three arguments, as
opposed to raw_input() which takes five.
Therefore raw_input() is never called via .pr_input, and we can set the
field to NULL. Then we can switch all the .pr_input functions to be static
and not variadic.
This will clear a lot of confusion, and will allow us to find problems
related to types -- like this one.
Comments?

No objection.

And a next step would be to change the signature of protosw#pr_input
to that of ip6protosw#pr_input and then we can merge protosw and
ip6protosw into one.

ozaki-r

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

[Maxime Villard]

Post by Ryota Ozaki

Post by Maxime Villard
In net/rtsock.c, I don't understand where we use the 'pr_input' field of
COMPATNAME(route_protosw)[]. To me it can't be used: .pr_input takes a
variadic function, but if you grep through the tree, you can see that
the variadic .pr_input functions we call always take three arguments, as
opposed to raw_input() which takes five.
Therefore raw_input() is never called via .pr_input, and we can set the
field to NULL. Then we can switch all the .pr_input functions to be static
and not variadic.
This will clear a lot of confusion, and will allow us to find problems
related to types -- like this one.
Comments?

No objection.
And a next step would be to change the signature of protosw#pr_input
to that of ip6protosw#pr_input and then we can merge protosw and
ip6protosw into one.

Is it a really good idea? To me it would be better if the IPv4 pr_input
stayed as

void x_input(struct mbuf *m, int off, int proto)

I don't see a lot of point in wanting to mimic IPv6; IPv6 has constraints
that don't apply to IPv4. Eg the second arg is a pointer, because of the
format of the IPv6 options -- but that doesn't apply to IPv4.

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

[Ryota Ozaki]

Post by Maxime Villard

Post by Ryota Ozaki

Post by Maxime Villard
In net/rtsock.c, I don't understand where we use the 'pr_input' field of
COMPATNAME(route_protosw)[]. To me it can't be used: .pr_input takes a
variadic function, but if you grep through the tree, you can see that
the variadic .pr_input functions we call always take three arguments, as
opposed to raw_input() which takes five.
Therefore raw_input() is never called via .pr_input, and we can set the
field to NULL. Then we can switch all the .pr_input functions to be static
and not variadic.
This will clear a lot of confusion, and will allow us to find problems
related to types -- like this one.
Comments?

No objection.
And a next step would be to change the signature of protosw#pr_input
to that of ip6protosw#pr_input and then we can merge protosw and
ip6protosw into one.

Is it a really good idea? To me it would be better if the IPv4 pr_input
stayed as
void x_input(struct mbuf *m, int off, int proto)
I don't see a lot of point in wanting to mimic IPv6; IPv6 has constraints
that don't apply to IPv4. Eg the second arg is a pointer, because of the
format of the IPv6 options -- but that doesn't apply to IPv4.

I just prefer having just one structure to having two similar structures,
which reduces some codes.

(Well, and FreeBSD / OpenBSD / Dragonfly BSD did the change.)

ozaki-r

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

[Maxime Villard]

Post by Ryota Ozaki

Post by Maxime Villard

Post by Ryota Ozaki

Post by Maxime Villard
In net/rtsock.c, I don't understand where we use the 'pr_input' field of
COMPATNAME(route_protosw)[]. To me it can't be used: .pr_input takes a
variadic function, but if you grep through the tree, you can see that
the variadic .pr_input functions we call always take three arguments, as
opposed to raw_input() which takes five.
Therefore raw_input() is never called via .pr_input, and we can set the
field to NULL. Then we can switch all the .pr_input functions to be static
and not variadic.
This will clear a lot of confusion, and will allow us to find problems
related to types -- like this one.
Comments?

No objection.
And a next step would be to change the signature of protosw#pr_input
to that of ip6protosw#pr_input and then we can merge protosw and
ip6protosw into one.

Is it a really good idea? To me it would be better if the IPv4 pr_input
stayed as
void x_input(struct mbuf *m, int off, int proto)
I don't see a lot of point in wanting to mimic IPv6; IPv6 has constraints
that don't apply to IPv4. Eg the second arg is a pointer, because of the
format of the IPv6 options -- but that doesn't apply to IPv4.

I just prefer having just one structure to having two similar structures,
which reduces some codes.
(Well, and FreeBSD / OpenBSD / Dragonfly BSD did the change.)

In this particular case I don't think it's a really good move. The IPv6
code is already a bit difficult to track because of *offp, so if we start
doing the same for IPv4 there will be blood.

We can still make the change later if it turns out to be really better.

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