Re: git: fcb3f813f379 - main - netinet*: remove PRC_ constants and streamline ICMP processing
Date: Tue, 04 Oct 2022 11:12:47 UTC
Dmitry:
Yeah I hit the same issue.. I did find that if you add
nooptions IPSEC_SUPPORT
to your build it will work. Also making LINT works
so I suspect its something minor in Gleb’s re-arrangement.
Though its like 4am right now in California so he won’t
be up for a while I suspect :)
R
> On Oct 4, 2022, at 7:10 AM, Dmitry Chagin <dchagin@heemeyer.club> wrote:
>
> On Tue, Oct 04, 2022 at 03:57:37AM +0000, Gleb Smirnoff wrote:
>> The branch main has been updated by glebius:
>>
>> URL: https://www.google.com/url?q=https://cgit.FreeBSD.org/src/commit/?id%3Dfcb3f813f379f544f9cd2a10d18045588da0e132&source=gmail-imap&ust=1665486631000000&usg=AOvVaw2VvUECoWSpNqOYdx0tDraU
>>
>> 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://www.google.com/url?q=https://reviews.freebsd.org/D36731&source=gmail-imap&ust=1665486631000000&usg=AOvVaw1JtS6e8srYG7hprLpt68zn
>> ---
>> 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
>
>
------
Randall Stewart
rrs@netflix.com