Re: move PRU_BIND request to (*pru_bind)() handler
On Sat, Aug 20, 2022 at 05:56:51PM +0300, Vitaliy Makkoveev wrote:
> We have 15 PRU_ requests to split. Is the one request per diff fine?
Yes. OK bluhm@
> Index: sys/kern/uipc_usrreq.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> retrieving revision 1.168
> diff -u -p -r1.168 uipc_usrreq.c
> --- sys/kern/uipc_usrreq.c15 Aug 2022 09:11:38 - 1.168
> +++ sys/kern/uipc_usrreq.c20 Aug 2022 14:42:47 -
> @@ -130,6 +130,7 @@ const struct pr_usrreqs uipc_usrreqs = {
> .pru_usrreq = uipc_usrreq,
> .pru_attach = uipc_attach,
> .pru_detach = uipc_detach,
> + .pru_bind = uipc_bind,
> };
>
> void
> @@ -222,10 +223,6 @@ uipc_usrreq(struct socket *so, int req,
>
> switch (req) {
>
> - case PRU_BIND:
> - error = unp_bind(unp, nam, p);
> - break;
> -
> case PRU_LISTEN:
> if (unp->unp_vnode == NULL)
> error = EINVAL;
> @@ -535,6 +532,14 @@ uipc_detach(struct socket *so)
> unp_detach(unp);
>
> return (0);
> +}
> +
> +int
> +uipc_bind(struct socket *so, struct mbuf *nam, struct proc *p)
> +{
> + struct unpcb *unp = sotounpcb(so);
> +
> + return unp_bind(unp, nam, p);
> }
>
> int
> Index: sys/net/pfkeyv2.c
> ===
> RCS file: /cvs/src/sys/net/pfkeyv2.c,v
> retrieving revision 1.235
> diff -u -p -r1.235 pfkeyv2.c
> --- sys/net/pfkeyv2.c 15 Aug 2022 09:11:38 - 1.235
> +++ sys/net/pfkeyv2.c 20 Aug 2022 14:42:47 -
> @@ -358,7 +358,6 @@ pfkeyv2_usrreq(struct socket *so, int re
> switch (req) {
> /* no connect, bind, accept. Socket is connected from the start */
> case PRU_CONNECT:
> - case PRU_BIND:
> case PRU_CONNECT2:
> case PRU_LISTEN:
> case PRU_ACCEPT:
> Index: sys/net/rtsock.c
> ===
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.335
> diff -u -p -r1.335 rtsock.c
> --- sys/net/rtsock.c 15 Aug 2022 09:11:38 - 1.335
> +++ sys/net/rtsock.c 20 Aug 2022 14:42:47 -
> @@ -234,7 +234,6 @@ route_usrreq(struct socket *so, int req,
> switch (req) {
> /* no connect, bind, accept. Socket is connected from the start */
> case PRU_CONNECT:
> - case PRU_BIND:
> case PRU_CONNECT2:
> case PRU_LISTEN:
> case PRU_ACCEPT:
> Index: sys/netinet/ip_divert.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_divert.c,v
> retrieving revision 1.69
> diff -u -p -r1.69 ip_divert.c
> --- sys/netinet/ip_divert.c 15 Aug 2022 09:11:39 - 1.69
> +++ sys/netinet/ip_divert.c 20 Aug 2022 14:42:47 -
> @@ -66,6 +66,7 @@ const struct pr_usrreqs divert_usrreqs =
> .pru_usrreq = divert_usrreq,
> .pru_attach = divert_attach,
> .pru_detach = divert_detach,
> + .pru_bind = divert_bind,
> };
>
> int divbhashsize = DIVERTHASHSIZE;
> @@ -274,10 +275,6 @@ divert_usrreq(struct socket *so, int req
> }
> switch (req) {
>
> - case PRU_BIND:
> - error = in_pcbbind(inp, addr, p);
> - break;
> -
> case PRU_SHUTDOWN:
> socantsendmore(so);
> break;
> @@ -362,6 +359,15 @@ divert_detach(struct socket *so)
>
> in_pcbdetach(inp);
> return (0);
> +}
> +
> +int
> +divert_bind(struct socket *so, struct mbuf *addr, struct proc *p)
> +{
> + struct inpcb *inp = sotoinpcb(so);
> +
> + soassertlocked(so);
> + return in_pcbbind(inp, addr, p);
> }
>
> int
> Index: sys/netinet/ip_divert.h
> ===
> RCS file: /cvs/src/sys/netinet/ip_divert.h,v
> retrieving revision 1.16
> diff -u -p -r1.16 ip_divert.h
> --- sys/netinet/ip_divert.h 15 Aug 2022 09:11:39 - 1.16
> +++ sys/netinet/ip_divert.h 20 Aug 2022 14:42:47 -
> @@ -74,5 +74,6 @@ int divert_usrreq(struct socket *,
> int, struct mbuf *, struct mbuf *, struct mbuf *, struct proc *);
> int divert_attach(struct socket *, int);
> int divert_detach(struct socket *);
> +int divert_bind(struct socket *, struct mbuf *, struct proc *);
> #endif /* _KERNEL */
> #endif /* _IP_DIVERT_H_ */
> Index: sys/netinet/ip_gre.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_gre.c,v
> retrieving revision 1.75
> diff -u -p -r1.75 ip_gre.c
> --- sys/netinet/ip_gre.c 15 Aug 2022 09:11:39 - 1.75
> +++ sys/netinet/ip_gre.c 20 Aug 2022 14:42:47 -
> @@ -65,6 +65,7 @@ const struct pr_usrreqs gre_usrreqs = {
> .pru_usrreq = gre_usrreq,
> .pru_attach = rip_attach,
> .pru_detach = rip_detach,
> + .pru_bind = rip_bind,
> };
>
> int
> Index:
Re: move PRU_BIND request to (*pru_bind)() handler
On Sat, Aug 20, 2022 at 03:49:08PM +0200, Alexander Bluhm wrote:
> On Fri, Aug 19, 2022 at 04:28:24PM -0900, Philip Guenther wrote:
> > On Fri, Aug 19, 2022 at 12:42 PM Vitaliy Makkoveev wrote:
> >
> > > bluhm@ pointed, that many KASSERT()s are not welcomed, so I didn't
> > > insert them into newly introduced handlers. Anyway except the tcp(4)
> > > protocol, `so_pcb' cant be NULL here. But the socket lock assertion
> > > looks reasonable.
> > >
> > > Some unp_*() functions could be merged with newly introduced uipc_*(),
> > > but I want to do this after (*pru_usrreq)() split finished.
> > >
> >
> > Having multiple PROTO_bind() routines that just return EOPNOTSUPP seems
> > like overkill to me. I think I would tend to just have the pru_bind()
> > inline do a NULL test and return EOPNOTSUPP if it is and leave the callback
> > NULL for all those protocols,
>
> I think a generic return(EOPNOTSUPP) in pru_bind() is good.
>
I also like this way. Since we use pru_() wrappers we don't mess the
code paths with the "if (...->pr_usrreqs->pru_something)" checks.
We have 15 PRU_ requests to split. Is the one request per diff fine?
Index: sys/kern/uipc_usrreq.c
===
RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
retrieving revision 1.168
diff -u -p -r1.168 uipc_usrreq.c
--- sys/kern/uipc_usrreq.c 15 Aug 2022 09:11:38 - 1.168
+++ sys/kern/uipc_usrreq.c 20 Aug 2022 14:42:47 -
@@ -130,6 +130,7 @@ const struct pr_usrreqs uipc_usrreqs = {
.pru_usrreq = uipc_usrreq,
.pru_attach = uipc_attach,
.pru_detach = uipc_detach,
+ .pru_bind = uipc_bind,
};
void
@@ -222,10 +223,6 @@ uipc_usrreq(struct socket *so, int req,
switch (req) {
- case PRU_BIND:
- error = unp_bind(unp, nam, p);
- break;
-
case PRU_LISTEN:
if (unp->unp_vnode == NULL)
error = EINVAL;
@@ -535,6 +532,14 @@ uipc_detach(struct socket *so)
unp_detach(unp);
return (0);
+}
+
+int
+uipc_bind(struct socket *so, struct mbuf *nam, struct proc *p)
+{
+ struct unpcb *unp = sotounpcb(so);
+
+ return unp_bind(unp, nam, p);
}
int
Index: sys/net/pfkeyv2.c
===
RCS file: /cvs/src/sys/net/pfkeyv2.c,v
retrieving revision 1.235
diff -u -p -r1.235 pfkeyv2.c
--- sys/net/pfkeyv2.c 15 Aug 2022 09:11:38 - 1.235
+++ sys/net/pfkeyv2.c 20 Aug 2022 14:42:47 -
@@ -358,7 +358,6 @@ pfkeyv2_usrreq(struct socket *so, int re
switch (req) {
/* no connect, bind, accept. Socket is connected from the start */
case PRU_CONNECT:
- case PRU_BIND:
case PRU_CONNECT2:
case PRU_LISTEN:
case PRU_ACCEPT:
Index: sys/net/rtsock.c
===
RCS file: /cvs/src/sys/net/rtsock.c,v
retrieving revision 1.335
diff -u -p -r1.335 rtsock.c
--- sys/net/rtsock.c15 Aug 2022 09:11:38 - 1.335
+++ sys/net/rtsock.c20 Aug 2022 14:42:47 -
@@ -234,7 +234,6 @@ route_usrreq(struct socket *so, int req,
switch (req) {
/* no connect, bind, accept. Socket is connected from the start */
case PRU_CONNECT:
- case PRU_BIND:
case PRU_CONNECT2:
case PRU_LISTEN:
case PRU_ACCEPT:
Index: sys/netinet/ip_divert.c
===
RCS file: /cvs/src/sys/netinet/ip_divert.c,v
retrieving revision 1.69
diff -u -p -r1.69 ip_divert.c
--- sys/netinet/ip_divert.c 15 Aug 2022 09:11:39 - 1.69
+++ sys/netinet/ip_divert.c 20 Aug 2022 14:42:47 -
@@ -66,6 +66,7 @@ const struct pr_usrreqs divert_usrreqs =
.pru_usrreq = divert_usrreq,
.pru_attach = divert_attach,
.pru_detach = divert_detach,
+ .pru_bind = divert_bind,
};
int divbhashsize = DIVERTHASHSIZE;
@@ -274,10 +275,6 @@ divert_usrreq(struct socket *so, int req
}
switch (req) {
- case PRU_BIND:
- error = in_pcbbind(inp, addr, p);
- break;
-
case PRU_SHUTDOWN:
socantsendmore(so);
break;
@@ -362,6 +359,15 @@ divert_detach(struct socket *so)
in_pcbdetach(inp);
return (0);
+}
+
+int
+divert_bind(struct socket *so, struct mbuf *addr, struct proc *p)
+{
+ struct inpcb *inp = sotoinpcb(so);
+
+ soassertlocked(so);
+ return in_pcbbind(inp, addr, p);
}
int
Index: sys/netinet/ip_divert.h
===
RCS file: /cvs/src/sys/netinet/ip_divert.h,v
retrieving revision 1.16
diff -u -p -r1.16 ip_divert.h
--- sys/netinet/ip_divert.h 15 Aug 2022 09:11:39 - 1.16
+++ sys/netinet/ip_divert.h 20 Aug 2022 14:42:47 -
@@ -74,5 +74,6 @@ intdivert_usrreq(struct socket *,
Re: move PRU_BIND request to (*pru_bind)() handler
On Fri, Aug 19, 2022 at 04:28:24PM -0900, Philip Guenther wrote: > On Fri, Aug 19, 2022 at 12:42 PM Vitaliy Makkoveev wrote: > > > bluhm@ pointed, that many KASSERT()s are not welcomed, so I didn't > > insert them into newly introduced handlers. Anyway except the tcp(4) > > protocol, `so_pcb' cant be NULL here. But the socket lock assertion > > looks reasonable. > > > > Some unp_*() functions could be merged with newly introduced uipc_*(), > > but I want to do this after (*pru_usrreq)() split finished. > > > > Having multiple PROTO_bind() routines that just return EOPNOTSUPP seems > like overkill to me. I think I would tend to just have the pru_bind() > inline do a NULL test and return EOPNOTSUPP if it is and leave the callback > NULL for all those protocols, I think a generic return(EOPNOTSUPP) in pru_bind() is good. > but I could also see having a single > prubind_eopnotsupp() (or whatever you think is a clear name) > implementation in some sys/kern/ file which could then be used by all the > protocols that don't implement it. I am not a fan of generic stubs. bluhm
Re: move PRU_BIND request to (*pru_bind)() handler
On Fri, Aug 19, 2022 at 12:42 PM Vitaliy Makkoveev wrote: > bluhm@ pointed, that many KASSERT()s are not welcomed, so I didn't > insert them into newly introduced handlers. Anyway except the tcp(4) > protocol, `so_pcb' cant be NULL here. But the socket lock assertion > looks reasonable. > > Some unp_*() functions could be merged with newly introduced uipc_*(), > but I want to do this after (*pru_usrreq)() split finished. > Having multiple PROTO_bind() routines that just return EOPNOTSUPP seems like overkill to me. I think I would tend to just have the pru_bind() inline do a NULL test and return EOPNOTSUPP if it is and leave the callback NULL for all those protocols, but I could also see having a single prubind_eopnotsupp() (or whatever you think is a clear name) implementation in some sys/kern/ file which could then be used by all the protocols that don't implement it. Consider how many protocols and callbacks you're going to be working through and how many stubs that would be if there's no sharing... Philip
move PRU_BIND request to (*pru_bind)() handler
Subj. Start the (*pru_usrreq)() split.
bluhm@ pointed, that many KASSERT()s are not welcomed, so I didn't
insert them into newly introduced handlers. Anyway except the tcp(4)
protocol, `so_pcb' cant be NULL here. But the socket lock assertion
looks reasonable.
Some unp_*() functions could be merged with newly introduced uipc_*(),
but I want to do this after (*pru_usrreq)() split finished.
Index: sys/kern/uipc_usrreq.c
===
RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
retrieving revision 1.168
diff -u -p -r1.168 uipc_usrreq.c
--- sys/kern/uipc_usrreq.c 15 Aug 2022 09:11:38 - 1.168
+++ sys/kern/uipc_usrreq.c 19 Aug 2022 21:34:26 -
@@ -130,6 +130,7 @@ const struct pr_usrreqs uipc_usrreqs = {
.pru_usrreq = uipc_usrreq,
.pru_attach = uipc_attach,
.pru_detach = uipc_detach,
+ .pru_bind = uipc_bind,
};
void
@@ -222,10 +223,6 @@ uipc_usrreq(struct socket *so, int req,
switch (req) {
- case PRU_BIND:
- error = unp_bind(unp, nam, p);
- break;
-
case PRU_LISTEN:
if (unp->unp_vnode == NULL)
error = EINVAL;
@@ -535,6 +532,14 @@ uipc_detach(struct socket *so)
unp_detach(unp);
return (0);
+}
+
+int
+uipc_bind(struct socket *so, struct mbuf *nam, struct proc *p)
+{
+ struct unpcb *unp = sotounpcb(so);
+
+ return unp_bind(unp, nam, p);
}
int
Index: sys/net/pfkeyv2.c
===
RCS file: /cvs/src/sys/net/pfkeyv2.c,v
retrieving revision 1.235
diff -u -p -r1.235 pfkeyv2.c
--- sys/net/pfkeyv2.c 15 Aug 2022 09:11:38 - 1.235
+++ sys/net/pfkeyv2.c 19 Aug 2022 21:34:26 -
@@ -171,6 +171,7 @@ void pfkey_init(void);
int pfkeyv2_attach(struct socket *, int);
int pfkeyv2_detach(struct socket *);
+int pfkeyv2_bind(struct socket *, struct mbuf *, struct proc *);
int pfkeyv2_usrreq(struct socket *, int, struct mbuf *, struct mbuf *,
struct mbuf *, struct proc *);
int pfkeyv2_output(struct mbuf *, struct socket *, struct sockaddr *,
@@ -203,6 +204,7 @@ const struct pr_usrreqs pfkeyv2_usrreqs
.pru_usrreq = pfkeyv2_usrreq,
.pru_attach = pfkeyv2_attach,
.pru_detach = pfkeyv2_detach,
+ .pru_bind = pfkeyv2_bind,
};
const struct protosw pfkeysw[] = {
@@ -333,6 +335,12 @@ pfkeyv2_detach(struct socket *so)
}
int
+pfkeyv2_bind(struct socket *so, struct mbuf *nam, struct proc *p)
+{
+ return (EOPNOTSUPP);
+}
+
+int
pfkeyv2_usrreq(struct socket *so, int req, struct mbuf *m,
struct mbuf *nam, struct mbuf *control, struct proc *p)
{
@@ -358,7 +366,6 @@ pfkeyv2_usrreq(struct socket *so, int re
switch (req) {
/* no connect, bind, accept. Socket is connected from the start */
case PRU_CONNECT:
- case PRU_BIND:
case PRU_CONNECT2:
case PRU_LISTEN:
case PRU_ACCEPT:
Index: sys/net/rtsock.c
===
RCS file: /cvs/src/sys/net/rtsock.c,v
retrieving revision 1.335
diff -u -p -r1.335 rtsock.c
--- sys/net/rtsock.c15 Aug 2022 09:11:38 - 1.335
+++ sys/net/rtsock.c19 Aug 2022 21:34:26 -
@@ -114,6 +114,7 @@ int route_output(struct mbuf *, struct s
introute_ctloutput(int, struct socket *, int, int, struct mbuf *);
introute_usrreq(struct socket *, int, struct mbuf *, struct mbuf *,
struct mbuf *, struct proc *);
+introute_bind(struct socket *, struct mbuf *, struct proc *);
void route_input(struct mbuf *m0, struct socket *, sa_family_t);
introute_arp_conflict(struct rtentry *, struct rt_addrinfo *);
introute_cleargateway(struct rtentry *, void *, unsigned int);
@@ -234,7 +235,6 @@ route_usrreq(struct socket *so, int req,
switch (req) {
/* no connect, bind, accept. Socket is connected from the start */
case PRU_CONNECT:
- case PRU_BIND:
case PRU_CONNECT2:
case PRU_LISTEN:
case PRU_ACCEPT:
@@ -367,6 +367,12 @@ route_detach(struct socket *so)
}
int
+route_bind(struct socket *so, struct mbuf *nam, struct proc *p)
+{
+ return (EOPNOTSUPP);
+}
+
+int
route_ctloutput(int op, struct socket *so, int level, int optname,
struct mbuf *m)
{
@@ -2405,6 +2411,7 @@ const struct pr_usrreqs route_usrreqs =
.pru_usrreq = route_usrreq,
.pru_attach = route_attach,
.pru_detach = route_detach,
+ .pru_bind = route_bind,
};
const struct protosw routesw[] = {
Index: sys/netinet/ip_divert.c
===
RCS file: /cvs/src/sys/netinet/ip_divert.c,v
retrieving revision 1.69
diff -u -p -r1.69 ip_divert.c
--- sys/netinet/ip_divert.c 15 Aug 2022 09:11:39 - 1.69
+++ sys/netinet/ip_divert.c 19 Aug 2022 21:34:26 -
@@
