Re: RTF_LOCAL and permanent ARP

2015-06-06 Thread Martin Pieuchot 
On 05/06/15(Fri) 15:04, Claudio Jeker wrote:
 On Thu, Jun 04, 2015 at 12:19:10PM +0200, Martin Pieuchot wrote:
  I'd like to put the link-layer address back into the gateway field of
  RTF_LOCAL addresses.  The problem is that RTF_LOCAL routes are also
  marked as RTF_LLINFO and a lot of code assume (correctly) that such
  routes contain valid ARP or ND information.
  
  I believe we decided to use an ``empty'' lladdr because previously all
  the routes created via rt_ifa_add(9) were using the same code and we
  needed the exact same gateway to remove MPATH routes.  But now that
  only RTF_LOCAL routes use this code and taking into consideration that
  such route *cannot* be MPATH, we can simply use ifp-if_sadl instead
  of a blank sockaddr_dl.
  
  This should also fix the (imcomplete) output in arp(8) and ndp(8).
  
  Ok?
 
 If I see this correctly rt_ifa_del is now doing deletes without a gateway
 specified for the RTF_LLINFO flag is set. Are you sure that there will be
 never multipath routes in that case?

Yes I'm sure because in this case RTF_LLINFO implies RTF_LOCAL.  And we
only create one RTF_LOCAL route per IP address.

I should probably add some KASSERT()s. to auto-document that.

 Also do we need to set RTAX_LABEL on remove? I think that is not needed.
 This is unrelated and should be handled independently.

I don't think so.

 Apart from that OK claudio@
 
  
  Index: net/route.c
  ===
  RCS file: /cvs/src/sys/net/route.c,v
  retrieving revision 1.212
  diff -u -p -r1.212 route.c
  --- net/route.c 26 May 2015 12:19:51 -  1.212
  +++ net/route.c 4 Jun 2015 10:03:51 -
  @@ -1121,27 +1121,23 @@ rt_maskedcopy(struct sockaddr *src, stru
   int
   rt_ifa_add(struct ifaddr *ifa, int flags, struct sockaddr *dst)
   {
  +   struct ifnet*ifp = ifa-ifa_ifp;
  struct rtentry  *rt, *nrt = NULL;
  struct sockaddr_rtlabel  sa_rl;
  -   struct sockaddr_dl   sa_dl = { sizeof(sa_dl), AF_LINK };
  struct rt_addrinfo   info;
  -   u_short  rtableid = ifa-ifa_ifp-if_rdomain;
  -   u_int8_t prio = ifa-ifa_ifp-if_priority + RTP_STATIC;
  +   u_short  rtableid = ifp-if_rdomain;
  +   u_int8_t prio = ifp-if_priority + RTP_STATIC;
  int  error;
   
  -   sa_dl.sdl_type = ifa-ifa_ifp-if_type;
  -   sa_dl.sdl_index = ifa-ifa_ifp-if_index;
  -
  memset(info, 0, sizeof(info));
  info.rti_ifa = ifa;
  info.rti_flags = flags | RTF_MPATH;
  info.rti_info[RTAX_DST] = dst;
  if (flags  RTF_LLINFO)
  -   info.rti_info[RTAX_GATEWAY] = (struct sockaddr *)sa_dl;
  +   info.rti_info[RTAX_GATEWAY] = (struct sockaddr *)ifp-if_sadl;
  else
  info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
  -   info.rti_info[RTAX_LABEL] =
  -   rtlabel_id2sa(ifa-ifa_ifp-if_rtlabelid, sa_rl);
  +   info.rti_info[RTAX_LABEL] = rtlabel_id2sa(ifp-if_rtlabelid, sa_rl);
   
   #ifdef MPLS
  if ((flags  RTF_MPLS) == RTF_MPLS) {
  @@ -1189,14 +1185,14 @@ rt_ifa_add(struct ifaddr *ifa, int flags
   int
   rt_ifa_del(struct ifaddr *ifa, int flags, struct sockaddr *dst)
   {
  +   struct ifnet*ifp = ifa-ifa_ifp;
  struct rtentry  *rt, *nrt = NULL;
  struct mbuf *m = NULL;
  struct sockaddr *deldst;
  struct rt_addrinfo   info;
  struct sockaddr_rtlabel  sa_rl;
  -   struct sockaddr_dl   sa_dl = { sizeof(sa_dl), AF_LINK };
  -   u_short  rtableid = ifa-ifa_ifp-if_rdomain;
  -   u_int8_t prio = ifa-ifa_ifp-if_priority + RTP_STATIC;
  +   u_short  rtableid = ifp-if_rdomain;
  +   u_int8_t prio = ifp-if_priority + RTP_STATIC;
  int  error;
   
   #ifdef MPLS
  @@ -1227,19 +1223,13 @@ rt_ifa_del(struct ifaddr *ifa, int flags
  }
  }
   
  -   sa_dl.sdl_type = ifa-ifa_ifp-if_type;
  -   sa_dl.sdl_index = ifa-ifa_ifp-if_index;
  -
  memset(info, 0, sizeof(info));
  info.rti_ifa = ifa;
  info.rti_flags = flags;
  info.rti_info[RTAX_DST] = dst;
  -   if (flags  RTF_LLINFO)
  -   info.rti_info[RTAX_GATEWAY] = (struct sockaddr *)sa_dl;
  -   else
  +   if ((flags  RTF_LLINFO) == 0)
  info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
  -   info.rti_info[RTAX_LABEL] =
  -   rtlabel_id2sa(ifa-ifa_ifp-if_rtlabelid, sa_rl);
  +   info.rti_info[RTAX_LABEL] = rtlabel_id2sa(ifp-if_rtlabelid, sa_rl);
   
  if ((flags  RTF_HOST) == 0)
  info.rti_info[RTAX_NETMASK] = ifa-ifa_netmask;
  Index: netinet6/nd6.c
  ===
  RCS file: /cvs/src/sys/netinet6/nd6.c,v
  retrieving revision 1.136
  diff -u -p -r1.136 nd6.c
  --- netinet6/nd6.c  15 May 2015 12:00:57 -  1.136
  +++ netinet6/nd6.c  4 Jun 2015 09:51:54 -
  @@ -651,7 +651,6 @@

Re: RTF_LOCAL and permanent ARP

2015-06-05 Thread Claudio Jeker 
On Thu, Jun 04, 2015 at 12:19:10PM +0200, Martin Pieuchot wrote:
 I'd like to put the link-layer address back into the gateway field of
 RTF_LOCAL addresses.  The problem is that RTF_LOCAL routes are also
 marked as RTF_LLINFO and a lot of code assume (correctly) that such
 routes contain valid ARP or ND information.
 
 I believe we decided to use an ``empty'' lladdr because previously all
 the routes created via rt_ifa_add(9) were using the same code and we
 needed the exact same gateway to remove MPATH routes.  But now that
 only RTF_LOCAL routes use this code and taking into consideration that
 such route *cannot* be MPATH, we can simply use ifp-if_sadl instead
 of a blank sockaddr_dl.
 
 This should also fix the (imcomplete) output in arp(8) and ndp(8).
 
 Ok?

If I see this correctly rt_ifa_del is now doing deletes without a gateway
specified for the RTF_LLINFO flag is set. Are you sure that there will be
never multipath routes in that case?

Also do we need to set RTAX_LABEL on remove? I think that is not needed.
This is unrelated and should be handled independently.

Apart from that OK claudio@

 
 Index: net/route.c
 ===
 RCS file: /cvs/src/sys/net/route.c,v
 retrieving revision 1.212
 diff -u -p -r1.212 route.c
 --- net/route.c   26 May 2015 12:19:51 -  1.212
 +++ net/route.c   4 Jun 2015 10:03:51 -
 @@ -1121,27 +1121,23 @@ rt_maskedcopy(struct sockaddr *src, stru
  int
  rt_ifa_add(struct ifaddr *ifa, int flags, struct sockaddr *dst)
  {
 + struct ifnet*ifp = ifa-ifa_ifp;
   struct rtentry  *rt, *nrt = NULL;
   struct sockaddr_rtlabel  sa_rl;
 - struct sockaddr_dl   sa_dl = { sizeof(sa_dl), AF_LINK };
   struct rt_addrinfo   info;
 - u_short  rtableid = ifa-ifa_ifp-if_rdomain;
 - u_int8_t prio = ifa-ifa_ifp-if_priority + RTP_STATIC;
 + u_short  rtableid = ifp-if_rdomain;
 + u_int8_t prio = ifp-if_priority + RTP_STATIC;
   int  error;
  
 - sa_dl.sdl_type = ifa-ifa_ifp-if_type;
 - sa_dl.sdl_index = ifa-ifa_ifp-if_index;
 -
   memset(info, 0, sizeof(info));
   info.rti_ifa = ifa;
   info.rti_flags = flags | RTF_MPATH;
   info.rti_info[RTAX_DST] = dst;
   if (flags  RTF_LLINFO)
 - info.rti_info[RTAX_GATEWAY] = (struct sockaddr *)sa_dl;
 + info.rti_info[RTAX_GATEWAY] = (struct sockaddr *)ifp-if_sadl;
   else
   info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
 - info.rti_info[RTAX_LABEL] =
 - rtlabel_id2sa(ifa-ifa_ifp-if_rtlabelid, sa_rl);
 + info.rti_info[RTAX_LABEL] = rtlabel_id2sa(ifp-if_rtlabelid, sa_rl);
  
  #ifdef MPLS
   if ((flags  RTF_MPLS) == RTF_MPLS) {
 @@ -1189,14 +1185,14 @@ rt_ifa_add(struct ifaddr *ifa, int flags
  int
  rt_ifa_del(struct ifaddr *ifa, int flags, struct sockaddr *dst)
  {
 + struct ifnet*ifp = ifa-ifa_ifp;
   struct rtentry  *rt, *nrt = NULL;
   struct mbuf *m = NULL;
   struct sockaddr *deldst;
   struct rt_addrinfo   info;
   struct sockaddr_rtlabel  sa_rl;
 - struct sockaddr_dl   sa_dl = { sizeof(sa_dl), AF_LINK };
 - u_short  rtableid = ifa-ifa_ifp-if_rdomain;
 - u_int8_t prio = ifa-ifa_ifp-if_priority + RTP_STATIC;
 + u_short  rtableid = ifp-if_rdomain;
 + u_int8_t prio = ifp-if_priority + RTP_STATIC;
   int  error;
  
  #ifdef MPLS
 @@ -1227,19 +1223,13 @@ rt_ifa_del(struct ifaddr *ifa, int flags
   }
   }
  
 - sa_dl.sdl_type = ifa-ifa_ifp-if_type;
 - sa_dl.sdl_index = ifa-ifa_ifp-if_index;
 -
   memset(info, 0, sizeof(info));
   info.rti_ifa = ifa;
   info.rti_flags = flags;
   info.rti_info[RTAX_DST] = dst;
 - if (flags  RTF_LLINFO)
 - info.rti_info[RTAX_GATEWAY] = (struct sockaddr *)sa_dl;
 - else
 + if ((flags  RTF_LLINFO) == 0)
   info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
 - info.rti_info[RTAX_LABEL] =
 - rtlabel_id2sa(ifa-ifa_ifp-if_rtlabelid, sa_rl);
 + info.rti_info[RTAX_LABEL] = rtlabel_id2sa(ifp-if_rtlabelid, sa_rl);
  
   if ((flags  RTF_HOST) == 0)
   info.rti_info[RTAX_NETMASK] = ifa-ifa_netmask;
 Index: netinet6/nd6.c
 ===
 RCS file: /cvs/src/sys/netinet6/nd6.c,v
 retrieving revision 1.136
 diff -u -p -r1.136 nd6.c
 --- netinet6/nd6.c15 May 2015 12:00:57 -  1.136
 +++ netinet6/nd6.c4 Jun 2015 09:51:54 -
 @@ -651,7 +651,6 @@ nd6_lookup(struct in6_addr *addr6, int c
   }
   if (!rt) {
   if (create  ifp) {
 - struct sockaddr_dl sa_dl = { sizeof(sa_dl), AF_LINK };
   struct rt_addrinfo info;

Re: RTF_LOCAL and permanent ARP

2015-06-04 Thread mxb 


Yes, incomplete is gone as well as arpresolve: unresolved and 
rt_expire == 0.


//mxb

On 2015-06-04 12:19, Martin Pieuchot wrote:

I'd like to put the link-layer address back into the gateway field of
RTF_LOCAL addresses.  The problem is that RTF_LOCAL routes are also
marked as RTF_LLINFO and a lot of code assume (correctly) that such
routes contain valid ARP or ND information.

I believe we decided to use an ``empty'' lladdr because previously all
the routes created via rt_ifa_add(9) were using the same code and we
needed the exact same gateway to remove MPATH routes.  But now that
only RTF_LOCAL routes use this code and taking into consideration that
such route *cannot* be MPATH, we can simply use ifp-if_sadl instead
of a blank sockaddr_dl.

This should also fix the (imcomplete) output in arp(8) and ndp(8).

Ok?

Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.212
diff -u -p -r1.212 route.c
--- net/route.c 26 May 2015 12:19:51 -  1.212
+++ net/route.c 4 Jun 2015 10:03:51 -
@@ -1121,27 +1121,23 @@ rt_maskedcopy(struct sockaddr *src, stru
  int
  rt_ifa_add(struct ifaddr *ifa, int flags, struct sockaddr *dst)
  {
+   struct ifnet*ifp = ifa-ifa_ifp;
struct rtentry  *rt, *nrt = NULL;
struct sockaddr_rtlabel  sa_rl;
-   struct sockaddr_dl   sa_dl = { sizeof(sa_dl), AF_LINK };
struct rt_addrinfo   info;
-   u_short  rtableid = ifa-ifa_ifp-if_rdomain;
-   u_int8_t prio = ifa-ifa_ifp-if_priority + RTP_STATIC;
+   u_short  rtableid = ifp-if_rdomain;
+   u_int8_t prio = ifp-if_priority + RTP_STATIC;
int  error;
  
-	sa_dl.sdl_type = ifa-ifa_ifp-if_type;

-   sa_dl.sdl_index = ifa-ifa_ifp-if_index;
-
memset(info, 0, sizeof(info));
info.rti_ifa = ifa;
info.rti_flags = flags | RTF_MPATH;
info.rti_info[RTAX_DST] = dst;
if (flags  RTF_LLINFO)
-   info.rti_info[RTAX_GATEWAY] = (struct sockaddr *)sa_dl;
+   info.rti_info[RTAX_GATEWAY] = (struct sockaddr *)ifp-if_sadl;
else
info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
-   info.rti_info[RTAX_LABEL] =
-   rtlabel_id2sa(ifa-ifa_ifp-if_rtlabelid, sa_rl);
+   info.rti_info[RTAX_LABEL] = rtlabel_id2sa(ifp-if_rtlabelid, sa_rl);
  
  #ifdef MPLS

if ((flags  RTF_MPLS) == RTF_MPLS) {
@@ -1189,14 +1185,14 @@ rt_ifa_add(struct ifaddr *ifa, int flags
  int
  rt_ifa_del(struct ifaddr *ifa, int flags, struct sockaddr *dst)
  {
+   struct ifnet*ifp = ifa-ifa_ifp;
struct rtentry  *rt, *nrt = NULL;
struct mbuf *m = NULL;
struct sockaddr *deldst;
struct rt_addrinfo   info;
struct sockaddr_rtlabel  sa_rl;
-   struct sockaddr_dl   sa_dl = { sizeof(sa_dl), AF_LINK };
-   u_short  rtableid = ifa-ifa_ifp-if_rdomain;
-   u_int8_t prio = ifa-ifa_ifp-if_priority + RTP_STATIC;
+   u_short  rtableid = ifp-if_rdomain;
+   u_int8_t prio = ifp-if_priority + RTP_STATIC;
int  error;
  
  #ifdef MPLS

@@ -1227,19 +1223,13 @@ rt_ifa_del(struct ifaddr *ifa, int flags
}
}
  
-	sa_dl.sdl_type = ifa-ifa_ifp-if_type;

-   sa_dl.sdl_index = ifa-ifa_ifp-if_index;
-
memset(info, 0, sizeof(info));
info.rti_ifa = ifa;
info.rti_flags = flags;
info.rti_info[RTAX_DST] = dst;
-   if (flags  RTF_LLINFO)
-   info.rti_info[RTAX_GATEWAY] = (struct sockaddr *)sa_dl;
-   else
+   if ((flags  RTF_LLINFO) == 0)
info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
-   info.rti_info[RTAX_LABEL] =
-   rtlabel_id2sa(ifa-ifa_ifp-if_rtlabelid, sa_rl);
+   info.rti_info[RTAX_LABEL] = rtlabel_id2sa(ifp-if_rtlabelid, sa_rl);
  
  	if ((flags  RTF_HOST) == 0)

info.rti_info[RTAX_NETMASK] = ifa-ifa_netmask;
Index: netinet6/nd6.c
===
RCS file: /cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.136
diff -u -p -r1.136 nd6.c
--- netinet6/nd6.c  15 May 2015 12:00:57 -  1.136
+++ netinet6/nd6.c  4 Jun 2015 09:51:54 -
@@ -651,7 +651,6 @@ nd6_lookup(struct in6_addr *addr6, int c
}
if (!rt) {
if (create  ifp) {
-   struct sockaddr_dl sa_dl = { sizeof(sa_dl), AF_LINK };
struct rt_addrinfo info;
int e;
  
@@ -667,9 +666,6 @@ nd6_lookup(struct in6_addr *addr6, int c

if (ifa == NULL)
return (NULL);
  
-			sa_dl.sdl_type = ifp-if_type;

-   sa_dl.sdl_index = ifp-if_index;
-

RTF_LOCAL and permanent ARP

2015-06-04 Thread Martin Pieuchot 
I'd like to put the link-layer address back into the gateway field of
RTF_LOCAL addresses.  The problem is that RTF_LOCAL routes are also
marked as RTF_LLINFO and a lot of code assume (correctly) that such
routes contain valid ARP or ND information.

I believe we decided to use an ``empty'' lladdr because previously all
the routes created via rt_ifa_add(9) were using the same code and we
needed the exact same gateway to remove MPATH routes.  But now that
only RTF_LOCAL routes use this code and taking into consideration that
such route *cannot* be MPATH, we can simply use ifp-if_sadl instead
of a blank sockaddr_dl.

This should also fix the (imcomplete) output in arp(8) and ndp(8).

Ok?

Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.212
diff -u -p -r1.212 route.c
--- net/route.c 26 May 2015 12:19:51 -  1.212
+++ net/route.c 4 Jun 2015 10:03:51 -
@@ -1121,27 +1121,23 @@ rt_maskedcopy(struct sockaddr *src, stru
 int
 rt_ifa_add(struct ifaddr *ifa, int flags, struct sockaddr *dst)
 {
+   struct ifnet*ifp = ifa-ifa_ifp;
struct rtentry  *rt, *nrt = NULL;
struct sockaddr_rtlabel  sa_rl;
-   struct sockaddr_dl   sa_dl = { sizeof(sa_dl), AF_LINK };
struct rt_addrinfo   info;
-   u_short  rtableid = ifa-ifa_ifp-if_rdomain;
-   u_int8_t prio = ifa-ifa_ifp-if_priority + RTP_STATIC;
+   u_short  rtableid = ifp-if_rdomain;
+   u_int8_t prio = ifp-if_priority + RTP_STATIC;
int  error;
 
-   sa_dl.sdl_type = ifa-ifa_ifp-if_type;
-   sa_dl.sdl_index = ifa-ifa_ifp-if_index;
-
memset(info, 0, sizeof(info));
info.rti_ifa = ifa;
info.rti_flags = flags | RTF_MPATH;
info.rti_info[RTAX_DST] = dst;
if (flags  RTF_LLINFO)
-   info.rti_info[RTAX_GATEWAY] = (struct sockaddr *)sa_dl;
+   info.rti_info[RTAX_GATEWAY] = (struct sockaddr *)ifp-if_sadl;
else
info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
-   info.rti_info[RTAX_LABEL] =
-   rtlabel_id2sa(ifa-ifa_ifp-if_rtlabelid, sa_rl);
+   info.rti_info[RTAX_LABEL] = rtlabel_id2sa(ifp-if_rtlabelid, sa_rl);
 
 #ifdef MPLS
if ((flags  RTF_MPLS) == RTF_MPLS) {
@@ -1189,14 +1185,14 @@ rt_ifa_add(struct ifaddr *ifa, int flags
 int
 rt_ifa_del(struct ifaddr *ifa, int flags, struct sockaddr *dst)
 {
+   struct ifnet*ifp = ifa-ifa_ifp;
struct rtentry  *rt, *nrt = NULL;
struct mbuf *m = NULL;
struct sockaddr *deldst;
struct rt_addrinfo   info;
struct sockaddr_rtlabel  sa_rl;
-   struct sockaddr_dl   sa_dl = { sizeof(sa_dl), AF_LINK };
-   u_short  rtableid = ifa-ifa_ifp-if_rdomain;
-   u_int8_t prio = ifa-ifa_ifp-if_priority + RTP_STATIC;
+   u_short  rtableid = ifp-if_rdomain;
+   u_int8_t prio = ifp-if_priority + RTP_STATIC;
int  error;
 
 #ifdef MPLS
@@ -1227,19 +1223,13 @@ rt_ifa_del(struct ifaddr *ifa, int flags
}
}
 
-   sa_dl.sdl_type = ifa-ifa_ifp-if_type;
-   sa_dl.sdl_index = ifa-ifa_ifp-if_index;
-
memset(info, 0, sizeof(info));
info.rti_ifa = ifa;
info.rti_flags = flags;
info.rti_info[RTAX_DST] = dst;
-   if (flags  RTF_LLINFO)
-   info.rti_info[RTAX_GATEWAY] = (struct sockaddr *)sa_dl;
-   else
+   if ((flags  RTF_LLINFO) == 0)
info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
-   info.rti_info[RTAX_LABEL] =
-   rtlabel_id2sa(ifa-ifa_ifp-if_rtlabelid, sa_rl);
+   info.rti_info[RTAX_LABEL] = rtlabel_id2sa(ifp-if_rtlabelid, sa_rl);
 
if ((flags  RTF_HOST) == 0)
info.rti_info[RTAX_NETMASK] = ifa-ifa_netmask;
Index: netinet6/nd6.c
===
RCS file: /cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.136
diff -u -p -r1.136 nd6.c
--- netinet6/nd6.c  15 May 2015 12:00:57 -  1.136
+++ netinet6/nd6.c  4 Jun 2015 09:51:54 -
@@ -651,7 +651,6 @@ nd6_lookup(struct in6_addr *addr6, int c
}
if (!rt) {
if (create  ifp) {
-   struct sockaddr_dl sa_dl = { sizeof(sa_dl), AF_LINK };
struct rt_addrinfo info;
int e;
 
@@ -667,9 +666,6 @@ nd6_lookup(struct in6_addr *addr6, int c
if (ifa == NULL)
return (NULL);
 
-   sa_dl.sdl_type = ifp-if_type;
-   sa_dl.sdl_index = ifp-if_index;
-
/*
 * Create a new route.  RTF_LLINFO is necessary
 * to