Resend because of nasty typo :/
On Mon, Dec 24, 2018 at 08:43:10PM -0200, Martin Pieuchot wrote:
> I'm not happy with adding the IFF_MULTICAST flag and SIOC{ADD,DEL}MULTI
> ioctls. It seems to be a common pattern between in existing pseudo-driver,
> so this shouldn't block you. However I'd greatly appreciate if you
> could explain to us which code is asking for this and if this could be
> improved.
>
Interface needs IFF_MULTICAST when enabling IPv6 as per in6_ifattach().
When an address is configured, the interface joins multicast groups with
in6_joingroup() (hence use SIOCADDMULTI) but only if IFF_MULTICAST is set. It is
useful for interfaces extern-facing interfaces, not so much for internal (like
mpe) as they won't process all-{nodes, routers} packets.
We may remove the test in in6_ifattach() because there are many tests through
the stack. It works if I disable the test in in6_ifattach() and remove the
IFF_MULTICAST. However I haven't tested further so it may break elsewhere.
And here is a new version of the icmp_reflect() diff, it is closer to what is
done with icmp6_reflect().
Index: netinet/ip_icmp.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_icmp.c,v
retrieving revision 1.181
diff -u -p -r1.181 ip_icmp.c
--- netinet/ip_icmp.c 28 Nov 2018 08:15:29 -0000 1.181
+++ netinet/ip_icmp.c 26 Dec 2018 14:03:50 -0000
@@ -676,11 +676,12 @@ freeit:
* Reflect the ip packet back to the source
*/
int
-icmp_reflect(struct mbuf *m, struct mbuf **op, struct in_ifaddr *ia)
+icmp_reflect(struct mbuf *m, struct mbuf **op, struct sockaddr *sa)
{
struct ip *ip = mtod(m, struct ip *);
struct mbuf *opts = NULL;
struct sockaddr_in sin;
+ struct in_ifaddr *ia = NULL;
struct rtentry *rt = NULL;
int optlen = (ip->ip_hl << 2) - sizeof(struct ip);
u_int rtableid;
@@ -700,12 +701,12 @@ icmp_reflect(struct mbuf *m, struct mbuf
m_resethdr(m);
m->m_pkthdr.ph_rtableid = rtableid;
- /*
- * If the incoming packet was addressed directly to us,
- * use dst as the src for the reply. For broadcast, use
- * the address which corresponds to the incoming interface.
- */
- if (ia == NULL) {
+ if (sa == NULL) {
+ /*
+ * If the incoming packet was addressed directly to us,
+ * use dst as the src for the reply. For broadcast, use
+ * the address which corresponds to the incoming interface.
+ */
memset(&sin, 0, sizeof(sin));
sin.sin_len = sizeof(sin);
sin.sin_family = AF_INET;
@@ -715,8 +716,15 @@ icmp_reflect(struct mbuf *m, struct mbuf
if (rtisvalid(rt) &&
ISSET(rt->rt_flags, RTF_LOCAL|RTF_BROADCAST))
ia = ifatoia(rt->rt_ifa);
- }
+ else {
+ memset(&sin, 0, sizeof(sin));
+ sin.sin_len = sizeof(sin);
+ sin.sin_family = AF_INET;
+ sin.sin_addr = ip->ip_src;
+ sa = sintosa(&sin);
+ }
+ }
/*
* The following happens if the packet was not addressed to us.
* Use the new source address and do a route lookup. If it fails
@@ -725,19 +733,14 @@ icmp_reflect(struct mbuf *m, struct mbuf
if (ia == NULL) {
rtfree(rt);
- memset(&sin, 0, sizeof(sin));
- sin.sin_len = sizeof(sin);
- sin.sin_family = AF_INET;
- sin.sin_addr = ip->ip_src;
-
/* keep packet in the original virtual instance */
- rt = rtalloc(sintosa(&sin), RT_RESOLVE, rtableid);
- if (rt == NULL) {
+ rt = rtalloc(sa, RT_RESOLVE, rtableid);
+ if (!rtisvalid(rt) &&
+ rt->rt_ifa->ifa_addr->sa_family == AF_INET) {
ipstat_inc(ips_noroute);
m_freem(m);
return (EHOSTUNREACH);
}
-
ia = ifatoia(rt->rt_ifa);
}
Index: netinet/ip_icmp.h
===================================================================
RCS file: /cvs/src/sys/netinet/ip_icmp.h,v
retrieving revision 1.31
diff -u -p -r1.31 ip_icmp.h
--- netinet/ip_icmp.h 5 Nov 2018 21:50:39 -0000 1.31
+++ netinet/ip_icmp.h 26 Dec 2018 14:03:50 -0000
@@ -235,7 +235,7 @@ struct mbuf *
void icmp_error(struct mbuf *, int, int, u_int32_t, int);
int icmp_input(struct mbuf **, int *, int, int);
void icmp_init(void);
-int icmp_reflect(struct mbuf *, struct mbuf **, struct in_ifaddr *);
+int icmp_reflect(struct mbuf *, struct mbuf **, struct sockaddr *);
void icmp_send(struct mbuf *, struct mbuf *);
int icmp_sysctl(int *, u_int, void *, size_t *, void *, size_t);
struct rtentry *
Index: netmpls/mpls_input.c
===================================================================
RCS file: /cvs/src/sys/netmpls/mpls_input.c,v
retrieving revision 1.68
diff -u -p -r1.68 mpls_input.c
--- netmpls/mpls_input.c 12 Jan 2018 06:57:56 -0000 1.68
+++ netmpls/mpls_input.c 26 Dec 2018 14:03:50 -0000
@@ -339,9 +339,7 @@ mpls_do_error(struct mbuf *m, int type,
struct shim_hdr stack[MPLS_INKERNEL_LOOP_MAX];
struct sockaddr_mpls sa_mpls;
struct sockaddr_mpls *smpls;
- struct rtentry *rt = NULL;
struct shim_hdr *shim;
- struct in_ifaddr *ia;
struct icmp *icp;
struct ip *ip;
int nstk, error;
@@ -380,26 +378,7 @@ mpls_do_error(struct mbuf *m, int type,
smpls->smpls_len = sizeof(*smpls);
smpls->smpls_label = shim->shim_label & MPLS_LABEL_MASK;
- rt = rtalloc(smplstosa(smpls), RT_RESOLVE, 0);
- if (rt == NULL) {
- /* no entry for this label */
- m_freem(m);
- return (NULL);
- }
- if (rt->rt_ifa->ifa_addr->sa_family == AF_INET)
- ia = ifatoia(rt->rt_ifa);
- else {
- /* XXX this needs fixing, if the MPLS is on an IP
- * less interface we need to find some other IP to
- * use as source.
- */
- rtfree(rt);
- m_freem(m);
- return (NULL);
- }
- /* It is safe to dereference ``ia'' iff ``rt'' is valid. */
- error = icmp_reflect(m, NULL, ia);
- rtfree(rt);
+ error = icmp_reflect(m, NULL, smplstosa(smpls));
if (error)
return (NULL);
