Date: Tue, 4 Oct 2022 14:10:15 +0300 From: Dmitry Chagin <dchagin@heemeyer.club> To: Gleb Smirnoff <glebius@freebsd.org> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: fcb3f813f379 - main - netinet*: remove PRC_ constants and streamline ICMP processing Message-ID: <YzwUl9Rd/xBRzu%2BP@heemeyer.club> In-Reply-To: <202210040357.2943vbxw015435@gitrepo.freebsd.org> References: <202210040357.2943vbxw015435@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Oct 04, 2022 at 03:57:37AM +0000, Gleb Smirnoff wrote: > The branch main has been updated by glebius: > > URL: https://cgit.FreeBSD.org/src/commit/?id=fcb3f813f379f544f9cd2a10d18045588da0e132 > > commit fcb3f813f379f544f9cd2a10d18045588da0e132 > Author: Gleb Smirnoff <glebius@FreeBSD.org> > AuthorDate: 2022-10-04 03:53:04 +0000 > Commit: Gleb Smirnoff <glebius@FreeBSD.org> > CommitDate: 2022-10-04 03:53:04 +0000 > > netinet*: remove PRC_ constants and streamline ICMP processing > > In the original design of the network stack from the protocol control > input method pr_ctlinput was used notify the protocols about two very > different kinds of events: internal system events and receival of an > ICMP messages from outside. These events were coded with PRC_ codes. > Today these methods are removed from the protosw(9) and are isolated > to IPv4 and IPv6 stacks and are called only from icmp*_input(). The > PRC_ codes now just create a shim layer between ICMP codes and errors > or actions taken by protocols. > > - Change ipproto_ctlinput_t to pass just pointer to ICMP header. This > allows protocols to not deduct it from the internal IP header. > - Change ip6proto_ctlinput_t to pass just struct ip6ctlparam pointer. > It has all the information needed to the protocols. In the structure, > change ip6c_finaldst fields to sockaddr_in6. The reason is that > icmp6_input() already has this address wrapped in sockaddr, and the > protocols want this address as sockaddr. > - For UDP tunneling control input, as well as for IPSEC control input, > change the prototypes to accept a transparent union of either ICMP > header pointer or struct ip6ctlparam pointer. > - In icmp_input() and icmp6_input() do only validation of ICMP header and > count bad packets. The translation of ICMP codes to errors/actions is > done by protocols. > - Provide icmp_errmap() and icmp6_errmap() as substitute to inetctlerrmap, > inet6ctlerrmap arrays. > - In protocol ctlinput methods either trust what icmp_errmap() recommend, > or do our own logic based on the ICMP header. > > Differential revision: https://reviews.freebsd.org/D36731 > --- > sys/netinet/icmp6.h | 1 + > sys/netinet/in_var.h | 2 - > sys/netinet/ip_icmp.c | 106 +++++++++++++++++++---------------- > sys/netinet/ip_icmp.h | 1 + > sys/netinet/ip_input.c | 17 ------ > sys/netinet/ip_var.h | 3 +- > sys/netinet/raw_ip.c | 11 +--- > sys/netinet/sctp_usrreq.c | 10 ++-- > sys/netinet/sctp_var.h | 2 +- > sys/netinet/sctputil.c | 13 ++--- > sys/netinet/tcp_subr.c | 128 +++++++++++++++++++++++++------------------ > sys/netinet/udp_usrreq.c | 24 ++++---- > sys/netinet/udp_var.h | 6 +- > sys/netinet6/icmp6.c | 100 ++++++++++++++++++--------------- > sys/netinet6/in6_pcb.c | 26 +-------- > sys/netinet6/in6_var.h | 1 - > sys/netinet6/ip6_input.c | 20 ------- > sys/netinet6/ip6_var.h | 11 ++-- > sys/netinet6/raw_ip6.c | 31 ++--------- > sys/netinet6/sctp6_usrreq.c | 8 +-- > sys/netinet6/udp6_usrreq.c | 26 ++++----- > sys/netipsec/ipsec_input.c | 24 ++++---- > sys/netipsec/ipsec_support.h | 17 ++++-- > sys/sys/protosw.h | 43 --------------- > 24 files changed, 274 insertions(+), 357 deletions(-) > > diff --git a/sys/netinet/icmp6.h b/sys/netinet/icmp6.h > index 9628c0957c4a..7429b8173b6a 100644 > --- a/sys/netinet/icmp6.h > +++ b/sys/netinet/icmp6.h > @@ -701,6 +701,7 @@ struct rttimer; > struct in6_multi; > # endif > void icmp6_paramerror(struct mbuf *, int); > +int icmp6_errmap(const struct icmp6_hdr *); > void icmp6_error(struct mbuf *, int, int, int); > void icmp6_error2(struct mbuf *, int, int, int, struct ifnet *); > int icmp6_input(struct mbuf **, int *, int); > diff --git a/sys/netinet/in_var.h b/sys/netinet/in_var.h > index 40e1c1a23c40..c4cfeea66ba8 100644 > --- a/sys/netinet/in_var.h > +++ b/sys/netinet/in_var.h > @@ -100,8 +100,6 @@ struct in_ifaddr { > #define IN_LNAOF(in, ifa) \ > ((ntohl((in).s_addr) & ~((struct in_ifaddr *)(ifa)->ia_subnetmask)) > > -extern u_char inetctlerrmap[]; > - > #define LLTABLE(ifp) \ > ((struct in_ifinfo *)(ifp)->if_afdata[AF_INET])->ii_llt > /* > diff --git a/sys/netinet/ip_icmp.c b/sys/netinet/ip_icmp.c > index f0cc703c2757..fdde24fd94be 100644 > --- a/sys/netinet/ip_icmp.c > +++ b/sys/netinet/ip_icmp.c > @@ -403,6 +403,55 @@ freeit: > m_freem(n); > } > > +int > +icmp_errmap(const struct icmp *icp) > +{ > + > + switch (icp->icmp_type) { > + case ICMP_UNREACH: > + switch (icp->icmp_code) { > + case ICMP_UNREACH_NET: > + case ICMP_UNREACH_HOST: > + case ICMP_UNREACH_SRCFAIL: > + case ICMP_UNREACH_NET_UNKNOWN: > + case ICMP_UNREACH_HOST_UNKNOWN: > + case ICMP_UNREACH_ISOLATED: > + case ICMP_UNREACH_TOSNET: > + case ICMP_UNREACH_TOSHOST: > + case ICMP_UNREACH_HOST_PRECEDENCE: > + case ICMP_UNREACH_PRECEDENCE_CUTOFF: > + return (EHOSTUNREACH); > + case ICMP_UNREACH_NEEDFRAG: > + return (EMSGSIZE); > + case ICMP_UNREACH_PROTOCOL: > + case ICMP_UNREACH_PORT: > + case ICMP_UNREACH_NET_PROHIB: > + case ICMP_UNREACH_HOST_PROHIB: > + case ICMP_UNREACH_FILTER_PROHIB: > + return (ECONNREFUSED); > + default: > + return (0); > + } > + case ICMP_TIMXCEED: > + switch (icp->icmp_code) { > + case ICMP_TIMXCEED_INTRANS: > + return (EHOSTUNREACH); > + default: > + return (0); > + } > + case ICMP_PARAMPROB: > + switch (icp->icmp_code) { > + case ICMP_PARAMPROB_ERRATPTR: > + case ICMP_PARAMPROB_OPTABSENT: > + return (ENOPROTOOPT); > + default: > + return (0); > + } > + default: > + return (0); > + } > +} > + > /* > * Process a received ICMP message. > */ > @@ -484,56 +533,21 @@ icmp_input(struct mbuf **mp, int *offp, int proto) > code = icp->icmp_code; > switch (icp->icmp_type) { > case ICMP_UNREACH: > - switch (code) { > - case ICMP_UNREACH_NET: > - case ICMP_UNREACH_HOST: > - case ICMP_UNREACH_SRCFAIL: > - case ICMP_UNREACH_NET_UNKNOWN: > - case ICMP_UNREACH_HOST_UNKNOWN: > - case ICMP_UNREACH_ISOLATED: > - case ICMP_UNREACH_TOSNET: > - case ICMP_UNREACH_TOSHOST: > - case ICMP_UNREACH_HOST_PRECEDENCE: > - case ICMP_UNREACH_PRECEDENCE_CUTOFF: > - code = PRC_UNREACH_NET; > - break; > - > - case ICMP_UNREACH_NEEDFRAG: > - code = PRC_MSGSIZE; > - break; > - > - /* > - * RFC 1122, Sections 3.2.2.1 and 4.2.3.9. > - * Treat subcodes 2,3 as immediate RST > - */ > - case ICMP_UNREACH_PROTOCOL: > - code = PRC_UNREACH_PROTOCOL; > - break; > - case ICMP_UNREACH_PORT: > - code = PRC_UNREACH_PORT; > - break; > - > - case ICMP_UNREACH_NET_PROHIB: > - case ICMP_UNREACH_HOST_PROHIB: > - case ICMP_UNREACH_FILTER_PROHIB: > - code = PRC_UNREACH_ADMIN_PROHIB; > - break; > - > - default: > - goto badcode; > - } > - goto deliver; > + if (code > ICMP_UNREACH_PRECEDENCE_CUTOFF) > + goto badcode; > + else > + goto deliver; > > case ICMP_TIMXCEED: > - if (code > 1) > + if (code > ICMP_TIMXCEED_REASS) > goto badcode; > - code += PRC_TIMXCEED_INTRANS; > - goto deliver; > + else > + goto deliver; > > case ICMP_PARAMPROB: > - if (code > 1) > + if (code > ICMP_PARAMPROB_LENGTH) > goto badcode; > - code = PRC_PARAMPROB; > + > deliver: > /* > * Problem with datagram; advise higher level routines. > @@ -553,7 +567,6 @@ icmp_input(struct mbuf **mp, int *offp, int proto) > if (icmpprintfs) > printf("deliver to protocol %d\n", icp->icmp_ip.ip_p); > #endif > - icmpsrc.sin_addr = icp->icmp_ip.ip_dst; > /* > * XXX if the packet contains [IPv4 AH TCP], we can't make a > * notification to TCP layer. > @@ -576,8 +589,7 @@ icmp_input(struct mbuf **mp, int *offp, int proto) > * ICMP_ADVLENPREF. See its definition in ip_icmp.h. > */ > if (ip_ctlprotox[icp->icmp_ip.ip_p] != NULL) > - ip_ctlprotox[icp->icmp_ip.ip_p](code, &icmpsrc, > - &icp->icmp_ip); > + ip_ctlprotox[icp->icmp_ip.ip_p](icp); > break; > > badcode: > diff --git a/sys/netinet/ip_icmp.h b/sys/netinet/ip_icmp.h > index 0303a09509c7..fefece665a00 100644 > --- a/sys/netinet/ip_icmp.h > +++ b/sys/netinet/ip_icmp.h > @@ -216,6 +216,7 @@ struct icmp { > (type) == ICMP_MASKREQ || (type) == ICMP_MASKREPLY) > > #ifdef _KERNEL > +int icmp_errmap(const struct icmp *); > void icmp_error(struct mbuf *, int, int, uint32_t, int); > int icmp_input(struct mbuf **, int *, int); > int ip_next_mtu(int, int); > diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c > index 145c4464b855..88fd4f5e4def 100644 > --- a/sys/netinet/ip_input.c > +++ b/sys/netinet/ip_input.c > @@ -873,23 +873,6 @@ ipproto_unregister(uint8_t proto) > return (ENOENT); > } > > -/* (x) - issued by icmp_input() */ > -u_char inetctlerrmap[PRC_NCMDS] = { > - [PRC_MSGSIZE] = EMSGSIZE, /* (x) */ > - [PRC_HOSTDEAD] = EHOSTDOWN, > - [PRC_HOSTUNREACH] = EHOSTUNREACH, > - [PRC_UNREACH_NET] = EHOSTUNREACH, /* (x) */ > - [PRC_UNREACH_HOST] = EHOSTUNREACH, > - [PRC_UNREACH_PROTOCOL] = ECONNREFUSED, /* (x) */ > - [PRC_UNREACH_PORT] = ECONNREFUSED, /* (x) */ > - [12] = EMSGSIZE, > - [PRC_UNREACH_SRCFAIL] = EHOSTUNREACH, > - [PRC_TIMXCEED_INTRANS] = EHOSTUNREACH, /* (x) */ > - [PRC_TIMXCEED_REASS] = 0, /* (x) */ > - [PRC_PARAMPROB] = ENOPROTOOPT, /* (x) */ > - [PRC_UNREACH_ADMIN_PROHIB] = ECONNREFUSED, /* (x) */ > -}; > - > /* > * Forward a packet. If some error occurs return the sender > * an icmp packet. Note we can't always generate a meaningful > diff --git a/sys/netinet/ip_var.h b/sys/netinet/ip_var.h > index 070c82677150..0a2d915b12b3 100644 > --- a/sys/netinet/ip_var.h > +++ b/sys/netinet/ip_var.h > @@ -238,7 +238,8 @@ extern void (*ip_rsvp_force_done)(struct socket *); > extern int (*rsvp_input_p)(struct mbuf **, int *, int); > > typedef int ipproto_input_t(struct mbuf **, int *, int); > -typedef void ipproto_ctlinput_t(int, struct sockaddr_in *, struct ip *); > +struct icmp; > +typedef void ipproto_ctlinput_t(struct icmp *); > int ipproto_register(uint8_t, ipproto_input_t, ipproto_ctlinput_t); > int ipproto_unregister(uint8_t); > #define IPPROTO_REGISTER(prot, input, ctl) do { \ > diff --git a/sys/netinet/raw_ip.c b/sys/netinet/raw_ip.c > index fb692e0822cf..2065b47883bb 100644 > --- a/sys/netinet/raw_ip.c > +++ b/sys/netinet/raw_ip.c > @@ -804,17 +804,12 @@ rip_ctloutput(struct socket *so, struct sockopt *sopt) > } > > void > -rip_ctlinput(int cmd, struct sockaddr_in *sin, struct ip *ip) > +rip_ctlinput(struct icmp *icmp) > { > - > - switch (cmd) { > #if defined(IPSEC) || defined(IPSEC_SUPPORT) > - case PRC_MSGSIZE: > - if (IPSEC_ENABLED(ipv4)) > - IPSEC_CTLINPUT(ipv4, cmd, (struct sockaddr *)sin, ip); > - break; > + if (IPSEC_ENABLED(ipv4)) > + IPSEC_CTLINPUT(ipv4, icmp); > #endif hi, me/dchagin/freebsd/sys/netinet/raw_ip.c:811:3: error: too few arguments to function call, expected 4, have 2 IPSEC_CTLINPUT(ipv4, icmp); /home/dchagin/freebsd/sys/netipsec/ipsec_support.h:222:61: note: expanded from macro 'IPSEC_CTLINPUT' ipsec_kmod_ctlinput(proto ## _ipsec_support, __VA_ARGS__) /home/dchagin/freebsd/sys/netipsec/ipsec_support.h:196:5: note: 'ipsec_kmod_ctlinput' declared here int ipsec_kmod_ctlinput(struct ipsec_support * const, int, 1 error generated. *** [raw_ip.o] Error code 1
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?YzwUl9Rd/xBRzu%2BP>