svn commit: r364257 - head/sys/net

[Qing Li]

Author: qingli
Date: Sat Aug 15 16:48:58 2020
New Revision: 364257
URL: https://svnweb.freebsd.org/changeset/base/364257

Log:
  Correct the mask byte order when checking for reserved bits.
  
  Reviewed by:  gnn
  Approved by:  gnn
  MFC after:2 weeks
  Differential Revision:https://reviews.freebsd.org/D26071

Modified:
  head/sys/net/if_vxlan.c

Modified: head/sys/net/if_vxlan.c
==
--- head/sys/net/if_vxlan.c Sat Aug 15 16:15:34 2020(r364256)
+++ head/sys/net/if_vxlan.c Sat Aug 15 16:48:58 2020(r364257)
@@ -2545,7 +2545,7 @@ vxlan_rcv_udp_packet(struct mbuf *m, int offset, struc
 * the behavior of the Linux implementation.
 */
if (vxh->vxlh_flags != htonl(VXLAN_HDR_FLAGS_VALID_VNI) ||
-   vxh->vxlh_vni & ~htonl(VXLAN_VNI_MASK))
+   vxh->vxlh_vni & ~VXLAN_VNI_MASK)
goto out;
 
vni = ntohl(vxh->vxlh_vni) >> VXLAN_HDR_VNI_SHIFT;
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r301217 - in head/sys: net netinet netinet6

[Qing Li]

On Sun, Jun 5, 2016 at 11:06 PM, Alexander V. Chernikov <
melif...@freebsd.org> wrote:

> 06.06.2016, 04:40, "George Neville-Neil" :
> > On 4 Jun 2016, at 15:05, Alexander V. Chernikov wrote:
> >
> >>  02.06.2016, 20:51, "George V. Neville-Neil" :
> >>>  Author: gnn
> >>>  Date: Thu Jun 2 17:51:29 2016
> >>>  New Revision: 301217
> >>>  URL: https://svnweb.freebsd.org/changeset/base/301217
> >>>
> >>>  Log:
> >>>This change re-adds L2 caching for TCP and UDP, as originally
> >>>  added in D4306
> >>>but removed due to other changes in the system. Restore the
> >>>  llentry pointer
> >>>to the "struct route", and use it to cache the L2 lookup (ARP or
> >>>  ND6) as
> >>>appropriate.
> >>
> >>  I have several comments regarding this commit.
> >>
> >>  1 Architecturally, there was quite a lot of efforts to eliminate
> >>  layering violation between lltable and other places in network stack.
> >>  It ended by committing D4102, which allowed both to cleanup lower
> >>  level, provide abstract “prepend” framework which could be used to
> >>  provide cached data to _otuput() functions.
> >>  This change brings these violations back in a really invasive way.
> >>
> >>  Additionally, implementing L2 PCB caching at the other subsystem
> >>  expense is really a bad idea.
> >>  If one wants caching in some subsystem, it should be implemented in
> >>  that subsystem not polluting other things.
> >>  Current implementation permits this by filling in “ro_prepend” /
> >>  ro_plen fields.
> >>
> >>  In general, this change looks more like a local hack and not like the
> >>  code that should be included in the tree.
> >>
> >>  2 There was no benchmarks proving the effectiveness of this change.
> >>  (For example, it is not obvious if it could significantly improve TCP
> >>  since we still have per-session TCP wlock + (typically) per-ring
> >>  mutex, so removing lltable rock might not help things here). Given
> >>  that the patch complicates existing code, there should be adequate
> >>  benefits to consider whether this change is worth implementing.
> >>
> >>  3 The “network” group was not included to the review despite the
> >>  fact that most of the changes were related to the L2 layer which is
> >>  not “transport”, so some people might have missed this review.
> >>
> >>  4 This change DOES NOT WORK. really.
> >>  (which raises questions on both review and benchmarking process).
> >>
> >>  The reason is that “plle” argument is filled only in “heavy”
> >>  lltable lookup functions (e.g. when we don’t have neighbour
> >>  adjacency). 99.9% of the time arpresolve/nd6_resolve() returns the
> >>  result w/o calling their heavy versions, and the returned “plle”
> >>  is NULL.
> >>
> >>  This can be easily verified by calling something like
> >>  dtrace -n 'fbt:kernel:ether_output:entry /arg3!=NULL&&((struct route
> >>  *)arg3)->ro_lle != NULL/ { stack(); }'
> >>
> >>  Given that, I kindly ask you to backout this change.
> >
> > Hi,
> >
> > I'm going to limit the scope of this reply to just you, me and Mike
> > Karels, from whom this originated.
> No, please keep the discussion open. The decision on having that
> particular L2 caching implementation (and L2 caching in general) is quite
> important, so it would be great if all technical arguments were saved so
> other people can (now or later) understand the decision details.
>
> Thanks for understanding.
> >
> > Best,
> > George
>
> This commit does seem to undo the non-trivial layer separation efforts
invested previously.
The flow-table construction was meant to help accelerate TCP/UDP route
lookups. The various
aspects of the routing code took flow-table into consideration, and those
code are still present
even after this change.
--Qing
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r252184 - head/sys/net

[Qing Li]

Author: qingli
Date: Tue Jun 25 00:10:49 2013
New Revision: 252184
URL: http://svnweb.freebsd.org/changeset/base/252184

Log:
  Due to the routing related networking kernel redesign work
  in FBSD 8.0, interface routes have been returened to the
  applications without the RTF_GATEWAY bit. This incompatibility
  has caused some issues with Zebra, Qugga and the like.
  This patch provides the RTF_GATEWAY flag bit in returned interface
  routes so to behave similarly to pre 8.0 systems.
  
  Reviewed by:  hrs
  Verified by:  mackn at opendns dot com

Modified:
  head/sys/net/route.h
  head/sys/net/rtsock.c

Modified: head/sys/net/route.h
==
--- head/sys/net/route.hMon Jun 24 23:41:16 2013(r252183)
+++ head/sys/net/route.hTue Jun 25 00:10:49 2013(r252184)
@@ -185,6 +185,9 @@ struct ortentry {
 
 #defineRTF_RNH_LOCKED   0x4000 /* radix node head is locked */
 
+#defineRTF_GWFLAG_COMPAT 0x8000/* a compatibility bit for 
interacting
+  with existing routing apps */
+
 /* Mask of RTF flags that are allowed to be modified by RTM_CHANGE. */
 #define RTF_FMASK  \
(RTF_PROTO1 | RTF_PROTO2 | RTF_PROTO3 | RTF_BLACKHOLE | \

Modified: head/sys/net/rtsock.c
==
--- head/sys/net/rtsock.c   Mon Jun 24 23:41:16 2013(r252183)
+++ head/sys/net/rtsock.c   Tue Jun 25 00:10:49 2013(r252184)
@@ -652,8 +652,10 @@ route_output(struct mbuf *m, struct sock
 */
if (gw_ro.ro_rt != NULL 
gw_ro.ro_rt-rt_gateway-sa_family == AF_LINK 
-   gw_ro.ro_rt-rt_ifp-if_flags  IFF_LOOPBACK)
+   gw_ro.ro_rt-rt_ifp-if_flags  IFF_LOOPBACK) {
info.rti_flags = ~RTF_GATEWAY;
+   info.rti_flags |= RTF_GWFLAG_COMPAT;
+   }
if (gw_ro.ro_rt != NULL)
RTFREE(gw_ro.ro_rt);
}
@@ -853,7 +855,11 @@ route_output(struct mbuf *m, struct sock
Free(rtm); rtm = new_rtm;
}
(void)rt_msg2(rtm-rtm_type, info, (caddr_t)rtm, NULL);
-   rtm-rtm_flags = rt-rt_flags;
+   if (rt-rt_flags  RTF_GWFLAG_COMPAT)
+   rtm-rtm_flags = RTF_GATEWAY | 
+   (rt-rt_flags  ~RTF_GWFLAG_COMPAT);
+   else
+   rtm-rtm_flags = rt-rt_flags;
rt_getmetrics(rt-rt_rmx, rtm-rtm_rmx);
rtm-rtm_addrs = info.rti_addrs;
break;
@@ -905,6 +911,7 @@ route_output(struct mbuf *m, struct sock
RT_UNLOCK(rt);
senderr(error);
}
+   rt-rt_flags = ~RTF_GATEWAY;
rt-rt_flags |= (RTF_GATEWAY  info.rti_flags);
}
if (info.rti_ifa != NULL 
@@ -1591,7 +1598,11 @@ sysctl_dumpentry(struct radix_node *rn, 
if (w-w_req  w-w_tmem) {
struct rt_msghdr *rtm = (struct rt_msghdr *)w-w_tmem;
 
-   rtm-rtm_flags = rt-rt_flags;
+   if (rt-rt_flags  RTF_GWFLAG_COMPAT)
+   rtm-rtm_flags = RTF_GATEWAY | 
+   (rt-rt_flags  ~RTF_GWFLAG_COMPAT);
+   else
+   rtm-rtm_flags = rt-rt_flags;
/*
 * let's be honest about this being a retarded hack
 */
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r252141 - head/sys/netinet6

[Qing Li]

Author: qingli
Date: Mon Jun 24 05:01:13 2013
New Revision: 252141
URL: http://svnweb.freebsd.org/changeset/base/252141

Log:
  Delete the nd6 entries associated with an off-link prefix
  if the same prefix cannot be found on an alternative
  interface.
  
  Reviewed by:  hrs
  MFC after:1 week

Modified:
  head/sys/netinet6/nd6_rtr.c

Modified: head/sys/netinet6/nd6_rtr.c
==
--- head/sys/netinet6/nd6_rtr.c Mon Jun 24 05:00:31 2013(r252140)
+++ head/sys/netinet6/nd6_rtr.c Mon Jun 24 05:01:13 2013(r252141)
@@ -1720,6 +1720,7 @@ nd6_prefix_offlink(struct nd_prefix *pr)
}
}
error = a_failure;
+   a_failure = 1;
if (error == 0) {
pr-ndpr_stateflags = ~NDPRF_ONLINK;
 
@@ -1758,7 +1759,8 @@ nd6_prefix_offlink(struct nd_prefix *pr)
opr-ndpr_prefix.sin6_addr),
opr-ndpr_plen, if_name(ifp),
if_name(opr-ndpr_ifp), e));
-   }
+   } else
+   a_failure = 0;
}
}
} else {
@@ -1770,6 +1772,10 @@ nd6_prefix_offlink(struct nd_prefix *pr)
if_name(ifp), error));
}
 
+   if (a_failure)
+   lltable_prefix_free(AF_INET6, (struct sockaddr *)sa6,
+   (struct sockaddr *)mask6, LLE_STATIC);
+
return (error);
 }
 
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r233773 - head/usr.sbin/arp

[Qing Li]

You missed my points.

That if check as part of r201282 was meant to resolve a couple of
issues related
to PPP links, as noted in my commit message. In this PPP/proxy
resolution context
the error message applies, which is why I actually used the word context in my
previous reply.

Your removing of that code will break the fixes committed in r201282.

You don't need to try so hard to be pedantic ...

When I say let's put in the code to support RFC 3012 properly, I
mean exactly that,
i.e., put code in which makes RFC3012 work without breaking the previous fixes.
And if there are other cases we need to cover, well, let's make sure we do.
Do you have another definition of a proper fix ?

I can't quite decipher the example you described in this email.

Could you please give me a bit more information in a private email so I can have
a better look at the issue, and possibly make a suggestion for an alternative
patch ?

--Qing


2012/4/8 Gleb Smirnoff gleb...@freebsd.org:
  Qing,

 On Sun, Apr 08, 2012 at 10:41:11AM -0700, Qing Li wrote:
 Q This is not the right way to support RFC3021.
 Q
 Q The code you removed is used for checking against attempt at adding
 Q duplicate entry.
 Q Both the message and the code apply in that context. I tried to state
 Q clearly and concisely
 Q what r201282 was intended in solving and was verified by actual users
 Q who ran into the
 Q described problems.

 How does the message apply?

 On a 10.0/9.0 prior to my commit:

 #ifconfig em0
 em0: flags=8843UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST metric 0 mtu 1500
        
 options=4219bRXCSUM,TXCSUM,VLAN_MTU,VLAN_HWTAGGING,VLAN_HWCSUM,TSO4,WOL_MAGIC,VLAN_HWTSO
        ether f0:de:f1:6c:5b:fa
        inet x.x.x.111 netmask 0xffe0 broadcast x.x.x.127
        nd6 options=29PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL
        media: Ethernet autoselect (100baseTX full-duplex)
        status: active
 # arp -an
 ? (x.x.x.97) at 00:00:5e:00:01:61 on em0 expires in 1198 seconds [ethernet]
 ? (x.x.x.101) at 00:e0:81:5a:22:49 on em0 expires in 618 seconds [ethernet]
 ? (x.x.x.111) at f0:de:f1:6c:5b:fa on em0 permanent [ethernet]
 ? (x.x.x.116) at 00:26:18:6a:ea:02 on em0 expires in 1128 seconds [ethernet]
 # # arp -s 81.19.64.96 0:0:0:0:0:0
 set: proxy entry exists for non 802 device

 And how does this apply? Where is the proxy entry mentioned? Where is the
 non 802 device?

 Look at the code before r201282 and see that the message was for absolutely
 unrelated case.

 And here is behavior of 6.1-RELEASE, that is prior to your new ARP work:

 # ifconfig fxp0
 fxp0: flags=8843UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST mtu 1500
        options=8VLAN_MTU
        inet x.x.x.134 netmask 0xfffc broadcast x.x.x.135
        ether 00:20:ed:6e:9c:f9
        media: Ethernet autoselect (10baseT/UTP)
        status: active
 # arp -s x.x.x.132 0:0:0:0:0:0
 set: can only proxy for x.x.x.132

 As you see, the error message was an other one.

 Q If we actually need to support RFC 3021, then better do it properly.

 What do you mean here under properly? RFC3021 says that network address
 in a /31 network is a common address. Thus it should be possible to have
 an ARP entry for it.

 Anyway this change isn't about RFC3021. A /31 network is just a case when we
 need to set ARP entry for network address.

 --
 Totus tuus, Glebius.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r233773 - head/usr.sbin/arp

[Qing Li]

This is not the right way to support RFC3021.

The code you removed is used for checking against attempt at adding
duplicate entry.
Both the message and the code apply in that context. I tried to state
clearly and concisely
what r201282 was intended in solving and was verified by actual users
who ran into the
described problems.

If we actually need to support RFC 3021, then better do it properly.

--Qing


On Mon, Apr 2, 2012 at 3:44 AM, Gleb Smirnoff gleb...@freebsd.org wrote:
 Author: glebius
 Date: Mon Apr  2 10:44:25 2012
 New Revision: 233773
 URL: http://svn.freebsd.org/changeset/base/233773

 Log:
  Historically arp(8) did a route lookup for the entry it is
  about to add, and failed if it exist and had invalid data
  link type.

  Later on, in r201282, this check morphed to other code, but
  message proxy entry exists for non 802 device still left,
  and now it is printed in a case if route prefix found is
  equal to current address being added. In other words, when
  we are trying to add ARP entry for a network address. The
  message is absolutely unrelated and disappointing in this
  case.

  I don't see anything bad with setting ARP entries for
  network addresses. While useless in usual network,
  in a /31 RFC3021 it may be necessary. This, remove this code.

 Modified:
  head/usr.sbin/arp/arp.c

 Modified: head/usr.sbin/arp/arp.c
 ==
 --- head/usr.sbin/arp/arp.c     Mon Apr  2 10:24:50 2012        (r233772)
 +++ head/usr.sbin/arp/arp.c     Mon Apr  2 10:44:25 2012        (r233773)
 @@ -387,10 +387,6 @@ set(int argc, char **argv)
        }
        addr = (struct sockaddr_inarp *)(rtm + 1);
        sdl = (struct sockaddr_dl *)(SA_SIZE(addr) + (char *)addr);
 -       if (addr-sin_addr.s_addr == dst-sin_addr.s_addr) {
 -               printf(set: proxy entry exists for non 802 device\n);
 -               return (1);
 -       }

        if ((sdl-sdl_family != AF_LINK) ||
            (rtm-rtm_flags  RTF_GATEWAY) ||
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r227791 - head/sys/netinet

[Qing Li]

  first I'd like to notice that we are speaking about obsoleted interfaces.


Yup, that's why you don't see me commenting on your other commits around
ia_netmask stuff, do you ?

snip


 Back to your comments:

 I have made a test case that proves, that usage of deleted address isn't
 prevented when it is removed, but loopback route still exists.

 The test is the following run a race between this program:

        struct ifreq ifr;
        int s;

        bzero(ifr, sizeof(struct ifreq));

        strncpy(ifr.ifr_name, igb1, sizeof ifr.ifr_name);
        ifr.ifr_addr.sa_family = AF_INET;
        ifr.ifr_addr.sa_len = sizeof(struct sockaddr_in);
        ((struct sockaddr_in *)ifr.ifr_addr)-sin_addr.s_addr = 
 inet_addr(10.0.0.1);

        s = socket(AF_INET, SOCK_DGRAM, 0);

        for (;;)
                ioctl(s, SIOCSIFADDR, ifr);

 And this script:

        while (true); do nc -z 10.0.0.1 22 || echo Fail; done


I am not sure if this test scenario is valid.

The loopback route is wiped at line #853 and then quickly inserted back at
line #936 because you are SIOCSIFADDR the same address over and over again.

snip


 Application writers don't know this in-kernel things. They usually write code
 this way:

        if (ioctl(foo)  0) {
                /* fatal error: can't configure address! */
        }

 And this is correct way. Look into this from viewpoint of say quagga 
 developer.
 One doesn't need to know about this tricks in FreeBSD kernel.


Yes, you have a good point there and I agree completely.

--Qing
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r227791 - head/sys/netinet

[Qing Li]

Logically speaking the prefix route should not be removed until all of the
address related housing keeping tasks have been completed successfully.

Putting in_scrubprefix() at the top does not gain you anything at
all, but can
potentially be problematic if additional tasks are in fact performed
in if_ioctl()
that may in fact affect the logic in in_ifinit().

Case in point, I had to perform additional routing related tasks so I
re-introduced nd6_rtrequest() in r227460.

You are not simplifying much logic by removing the error recovery code from
the return of if_ioctl(). So why removing the flexibility of what
if_ioctl() is
intended for as part of the address configuration logic ?

--Qing

On Mon, Nov 21, 2011 at 6:10 AM, Gleb Smirnoff gleb...@freebsd.org wrote:
 Author: glebius
 Date: Mon Nov 21 14:10:13 2011
 New Revision: 227791
 URL: http://svn.freebsd.org/changeset/base/227791

 Log:
  Historically in_control() did not check sockaddrs supplied with
  structs ifreq/in_aliasreq and there've been several panics due
  to that problem. All these panics were fixed just a couple of
  lines above the panicing code.

  Take a more general approach: sanity check sockaddrs supplied
  with SIOCAIFADDR and SIOCSIF*ADDR at the beggining of the
  function and drop all checks below.

  One check is now disabled due to strange code in ifconfig(8)
  that I've removed recently. I'm going to enable it with next
  __FreeBSD_version bump.

  Historically in_ifinit() was able to recover from an error
  and restore old address. Nowadays this feature isn't working
  for all error cases, but for some of them. I suppose no software
  relies on this behavior, so I'd like to remove it, since this
  simplifies code a lot.

  Also, move if_scrub() earlier in the in_ifinit(). It is more
  correct to wipe routes before removing address from local
  address list, and interface address list.

  Silence from: bz, brooks, andre, rwatson, 3 weeks

 Modified:
  head/sys/netinet/in.c

 Modified: head/sys/netinet/in.c
 ==
 --- head/sys/netinet/in.c       Mon Nov 21 13:40:35 2011        (r227790)
 +++ head/sys/netinet/in.c       Mon Nov 21 14:10:13 2011        (r227791)
 @@ -234,16 +234,42 @@ in_control(struct socket *so, u_long cmd
         * in_lifaddr_ioctl() and ifp-if_ioctl().
         */
        switch (cmd) {
 -       case SIOCAIFADDR:
 -       case SIOCDIFADDR:
        case SIOCGIFADDR:
        case SIOCGIFBRDADDR:
        case SIOCGIFDSTADDR:
        case SIOCGIFNETMASK:
 +       case SIOCDIFADDR:
 +               break;
 +       case SIOCAIFADDR:
 +               /*
 +                * ifra_addr must be present and be of INET family.
 +                * ifra_broadaddr and ifra_mask are optional.
 +                */
 +               if (ifra-ifra_addr.sin_len != sizeof(struct sockaddr_in) ||
 +                   ifra-ifra_addr.sin_family != AF_INET)
 +                       return (EINVAL);
 +               if (ifra-ifra_broadaddr.sin_len != 0 
 +                   (ifra-ifra_broadaddr.sin_len != sizeof(struct 
 sockaddr_in) ||
 +                   ifra-ifra_broadaddr.sin_family != AF_INET))
 +                       return (EINVAL);
 +#if 0
 +               /*
 +                * ifconfig(8) historically doesn't set af_family for mask
 +                * for unknown reason.
 +                */
 +               if (ifra-ifra_mask.sin_len != 0 
 +                   (ifra-ifra_mask.sin_len != sizeof(struct sockaddr_in) ||
 +                   ifra-ifra_mask.sin_family != AF_INET))
 +                       return (EINVAL);
 +#endif
 +               break;
        case SIOCSIFADDR:
        case SIOCSIFBRDADDR:
        case SIOCSIFDSTADDR:
        case SIOCSIFNETMASK:
 +               if (ifr-ifr_addr.sa_family != AF_INET ||
 +                   ifr-ifr_addr.sa_len != sizeof(struct sockaddr_in))
 +                       return (EINVAL);
                break;

        case SIOCALIFADDR:
 @@ -349,7 +375,7 @@ in_control(struct socket *so, u_long cmd
        switch (cmd) {
        case SIOCAIFADDR:
        case SIOCDIFADDR:
 -               if (ifra-ifra_addr.sin_family == AF_INET) {
 +               {
                        struct in_ifaddr *oia;

                        IN_IFADDR_RLOCK();
 @@ -506,7 +532,7 @@ in_control(struct socket *so, u_long cmd
                goto out;

        case SIOCSIFNETMASK:
 -               ia-ia_sockmask.sin_addr = ifra-ifra_addr.sin_addr;
 +               ia-ia_sockmask = *(struct sockaddr_in *)ifr-ifr_addr;
                ia-ia_subnetmask = ntohl(ia-ia_sockmask.sin_addr.s_addr);
                goto out;

 @@ -514,14 +540,12 @@ in_control(struct socket *so, u_long cmd
                maskIsNew = 0;
                hostIsNew = 1;
                error = 0;
 -               if (ia-ia_addr.sin_family == AF_INET) {
 -                       if (ifra-ifra_addr.sin_len == 0) {
 -                               ifra-ifra_addr = 

Re: svn commit: r227791 - head/sys/netinet

[Qing Li]

 From my point of view logically speaking, we should first remove route,
 then remove address. Otherwise, for a short time we've got an invalid
 route in table.


For a short time you have an invalid address, it is faster to remove the
address from the list to prevent usage, then to flush the route, which would
also trigger L2 table flushes as well.


 Q Putting in_scrubprefix() at the top does not gain you anything at
 Q all, but can
 Q potentially be problematic if additional tasks are in fact performed
 Q in if_ioctl()
 Q that may in fact affect the logic in in_ifinit().
 Q
 Q Case in point, I had to perform additional routing related tasks so I
 Q re-introduced nd6_rtrequest() in r227460.

 Pardon, can you please elaborate on this? I don't see any problems that
 I intoroduced, but if ther are any, we can either push in_scrubprefix()
 down the function as it was before, of fix them some other way.


The point being, perhaps the address related tasks to be performed in
if_ioctl() today is limited, however, we may need to perform additional
housekeeping tasks in if_ioctl(), and if there is error, we would want to
revert the process.

There are so many different L2 and pseudo interface functions that map
to if_ioctl(), it is Not safe to ignore its error and not reflect that error
back to the consistency of the routing table and the interface address
list.

Again, removing the associated route at the beginning of in_ifinit() does
not gain much in terms of performance or code cleanness, nor does it
improve the logic, so why limit what we could do in the future in if_ioctl()
by making such a change ??


 Q You are not simplifying much logic by removing the error recovery code from
 Q the return of if_ioctl(). So why removing the flexibility of what
 Q if_ioctl() is
 Q intended for as part of the address configuration logic ?

 Because in_ifinit() was inconsistent. It tried to recover in case
 of (*ifp-if_ioctl) failure, but did not try to recover in case
 of failure of:

 in_addprefix()
 ifa_add_loopback_route()


The inconsistency is due to the fact failures from these two functions
are not fatal, and does not necessarily affect the actual operations.

In function in_addprefix(), there are 2 main possible errors. EEXIST is
non fatal, it just means the same prefix route is already covered by
another address, and what is being inserted is just an alias.

The other error is due to RTM_ADD failure, however, since IFA_ROUTE is
not set on the address when failure occurs, no route will ever be directed
to either the interface or that address. Therefore no harm is done.

In the case of function ifa_add_loopback_route(), again, a failure is just
an indication of an EEXIST.

You may ask why not check for EEXIST before calling these functions, and
I can tell you having worked those code regions for quite some time, the
actual work amounts to the same efforts, but complicates the logic WRT
the callers. It's actually better just let the failure occur.

Unless you have strong reasons otherwise, I'd appreciate you put
back the original logic in function in_ifinit().

--Qing
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r227460 - head/sys/netinet6

[Qing Li]

Author: qingli
Date: Fri Nov 11 23:22:38 2011
New Revision: 227460
URL: http://svn.freebsd.org/changeset/base/227460

Log:
  A default route learned from the RAs could be deleted manually
  after its installation. This removal may be accidental and can
  prevent the default route from being installed in the future if
  the associated default router has the best preference. The cause
  is the lack of status update in the default router on the state
  of its route installation in the kernel FIB. This patch fixes
  the described problem.
  
  Reviewed by:  hrs, discussed with hrs
  MFC after:5 days

Modified:
  head/sys/netinet6/in6.c
  head/sys/netinet6/nd6.c
  head/sys/netinet6/nd6.h
  head/sys/netinet6/nd6_rtr.c

Modified: head/sys/netinet6/in6.c
==
--- head/sys/netinet6/in6.c Fri Nov 11 22:57:52 2011(r227459)
+++ head/sys/netinet6/in6.c Fri Nov 11 23:22:38 2011(r227460)
@@ -152,7 +152,7 @@ in6_ifaddloop(struct ifaddr *ifa)
ia = ifa2ia6(ifa);
ifp = ifa-ifa_ifp;
IF_AFDATA_LOCK(ifp);
-   ifa-ifa_rtrequest = NULL;
+   ifa-ifa_rtrequest = nd6_rtrequest;
 
/* XXX QL
 * we need to report rt_newaddrmsg

Modified: head/sys/netinet6/nd6.c
==
--- head/sys/netinet6/nd6.c Fri Nov 11 22:57:52 2011(r227459)
+++ head/sys/netinet6/nd6.c Fri Nov 11 23:22:38 2011(r227460)
@@ -1174,6 +1174,46 @@ done:
 }
 
 
+/*
+ * Rejuvenate this function for routing operations related
+ * processing.
+ */
+void
+nd6_rtrequest(int req, struct rtentry *rt, struct rt_addrinfo *info)
+{
+   struct sockaddr_in6 *gateway = (struct sockaddr_in6 *)rt-rt_gateway;
+   struct nd_defrouter *dr;
+   struct ifnet *ifp = rt-rt_ifp;
+
+   RT_LOCK_ASSERT(rt);
+
+   switch (req) {
+   case RTM_ADD:
+   break;
+
+   case RTM_DELETE:
+   if (!ifp)
+   return;
+   /*
+* Only indirect routes are interesting.
+*/
+   if ((rt-rt_flags  RTF_GATEWAY) == 0)
+   return;
+   /*
+* check for default route
+*/
+   if (IN6_ARE_ADDR_EQUAL(in6addr_any, 
+  SIN6(rt_key(rt))-sin6_addr)) {
+
+   dr = defrouter_lookup(gateway-sin6_addr, ifp);
+   if (dr != NULL)
+   dr-installed = 0;
+   }
+   break;
+   }
+}
+
+
 int
 nd6_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp)
 {

Modified: head/sys/netinet6/nd6.h
==
--- head/sys/netinet6/nd6.h Fri Nov 11 22:57:52 2011(r227459)
+++ head/sys/netinet6/nd6.h Fri Nov 11 23:22:38 2011(r227460)
@@ -406,6 +406,7 @@ void nd6_purge __P((struct ifnet *));
 void nd6_nud_hint __P((struct rtentry *, struct in6_addr *, int));
 int nd6_resolve __P((struct ifnet *, struct rtentry *, struct mbuf *,
struct sockaddr *, u_char *));
+void nd6_rtrequest __P((int, struct rtentry *, struct rt_addrinfo *));
 int nd6_ioctl __P((u_long, caddr_t, struct ifnet *));
 struct llentry *nd6_cache_lladdr __P((struct ifnet *, struct in6_addr *,
char *, int, int, int));

Modified: head/sys/netinet6/nd6_rtr.c
==
--- head/sys/netinet6/nd6_rtr.c Fri Nov 11 22:57:52 2011(r227459)
+++ head/sys/netinet6/nd6_rtr.c Fri Nov 11 23:22:38 2011(r227460)
@@ -751,9 +751,10 @@ defrtrlist_update(struct nd_defrouter *n
 
/*
 * If the preference does not change, there's no need
-* to sort the entries.
+* to sort the entries. Also make sure the selected
+* router is still installed in the kernel.
 */
-   if (rtpref(new) == oldpref) {
+   if (dr-installed  rtpref(new) == oldpref) {
splx(s);
return (dr);
}
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r226710 - head/sys/net

[Qing Li]

Author: qingli
Date: Tue Oct 25 00:34:39 2011
New Revision: 226710
URL: http://svn.freebsd.org/changeset/base/226710

Log:
  The host-id/interface-id can have a specific value and is properly
  masked out when adding a prefix route through the route command.
  However, when deleting the route, simply changing the command keyword
  from add to delete does not work. The failoure is observed in
  both IPv4 and IPv6 route insertion. The patch makes the route command
  behavior consistent between the add and the delete operation.
  
  MFC after:1 week

Modified:
  head/sys/net/route.c

Modified: head/sys/net/route.c
==
--- head/sys/net/route.cMon Oct 24 23:38:11 2011(r226709)
+++ head/sys/net/route.cTue Oct 25 00:34:39 2011(r226710)
@@ -1025,6 +1025,7 @@ rtrequest1_fib(int req, struct rt_addrin
register struct radix_node_head *rnh;
struct ifaddr *ifa;
struct sockaddr *ndst;
+   struct sockaddr_storage mdst;
 #define senderr(x) { error = x ; goto bad; }
 
KASSERT((fibnum  rt_numfibs), (rtrequest1_fib: bad fibnum));
@@ -1051,6 +1052,10 @@ rtrequest1_fib(int req, struct rt_addrin
 
switch (req) {
case RTM_DELETE:
+   if (netmask) {
+   rt_maskedcopy(dst, (struct sockaddr *)mdst, netmask);
+   dst = (struct sockaddr *)mdst;
+   }
 #ifdef RADIX_MPATH
if (rn_mpath_capable(rnh)) {
error = rn_mpath_update(req, info, rnh, ret_nrt);
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r226713 - head/sys/netinet

[Qing Li]

Author: qingli
Date: Tue Oct 25 04:06:29 2011
New Revision: 226713
URL: http://svn.freebsd.org/changeset/base/226713

Log:
  Exclude host routes when checking for prefix coverage on multiple
  interfaces. A host route has a NULL mask so check for that condition.
  I have also been told by developers who customize the packet output
  path with direct manipulation of the route entry (or the outgoing
  interface to be specific). This patch checks for the route mask
  explicitly to make sure custom code will not panic.
  
  PR:   kern/161805
  MFC after:3 days

Modified:
  head/sys/netinet/in.c

Modified: head/sys/netinet/in.c
==
--- head/sys/netinet/in.c   Tue Oct 25 01:47:33 2011(r226712)
+++ head/sys/netinet/in.c   Tue Oct 25 04:06:29 2011(r226713)
@@ -1429,12 +1429,21 @@ in_lltable_rtcheck(struct ifnet *ifp, u_
 * on one interface and the corresponding outgoing packet leaves
 * another interface.
 */
-   if (rt-rt_ifp != ifp) {
+   if (!(rt-rt_flags  RTF_HOST)  rt-rt_ifp != ifp) {
const char *sa, *mask, *addr, *lim;
int len;
 
-   sa = (const char *)rt_key(rt);
mask = (const char *)rt_mask(rt);
+   /*
+* Just being extra cautious to avoid some custom
+* code getting into trouble.
+*/
+   if (mask == NULL) {
+   RTFREE_LOCKED(rt);
+   return (EINVAL);
+   }
+
+   sa = (const char *)rt_key(rt);
addr = (const char *)l3addr;
len = ((const struct sockaddr_in *)l3addr)-sin_len;
lim = addr + len;
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r226451 - head/sys/netinet6

[Qing Li]

Author: qingli
Date: Sun Oct 16 22:15:13 2011
New Revision: 226451
URL: http://svn.freebsd.org/changeset/base/226451

Log:
  The IPv6 code was influx at the time of r196865 due to the L2/L3
  separation rewrite changes. r196865 was committed to fix a scope
  violation problem in the following test scenario:
  
box-1# ifconfig em0 inet6 2001:db8:1:: prefixlen 64 anycast
box-1# ifconfig em1 inet6 2001:db8:2::1 prefixlen 64
  
box-2# ifconfig re0 inet6 2001:db8:1::6 prefixlen 64
  
em0 and re0 are on the same link.
  
box-2# ping6 2001:db8:1::
PING6(56=40+8+8 bytes) 2001:db8:1::6 -- 2001:db8:1::
  
  the ICMPv6 response should have a source address of em1, which
  is 2001:db8:2::1, not the link-local address of em0.
  
  That code is no longer necessary and breaks the IPv6-Ready logo
  testing, so revert it now.
  
  Reviewed by:  hrs
  MFC after:3 days

Modified:
  head/sys/netinet6/icmp6.c

Modified: head/sys/netinet6/icmp6.c
==
--- head/sys/netinet6/icmp6.c   Sun Oct 16 21:30:15 2011(r226450)
+++ head/sys/netinet6/icmp6.c   Sun Oct 16 22:15:13 2011(r226451)
@@ -2244,10 +2244,6 @@ icmp6_reflect(struct mbuf *m, size_t off
}
}
 
-   if ((srcp != NULL)  
-   (in6_addrscope(srcp) != in6_addrscope(ip6-ip6_src)))
-   srcp = NULL;
-
if (srcp == NULL) {
int e;
struct sockaddr_in6 sin6;
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r225947 - head/sys/netinet

[Qing Li]

Okay, now I know what's confusing you ... it's that bug I introduced :-)

The 2nd if() check on the RTF_GATEWAY flag should have been
an else if().

In a nutshell, the logic is any indirect route should fail the check,
except for that special host route.

Attached is the rework of that part of the patch, with some cleaning up
per your suggestion.

Thank you very much for catching that bug.

--Qing


 Q  Well, after third review it is clear, that
 Q  next if() case would definitely be true, and you would proceed
 Q  with return. But that is difficult to see from first glance.
 Q
 Q    Not so, only for an indirect prefix route.
 Q
 Q  I'd suggest to remove error variable, return immediately in
 Q  all error cases, and also the RTF_GATEWAY check can be shifted up,
 Q  since it is the most simple and the most usual to be true.
 Q 
 Q
 Q   No, the RTF_GATEWAY check cannot be shifted up because if we did
 Q   that, the (indirect host route, with destination matching the gateway IP)
 Q   would never be executed, if when that set of conditions are true, which 
 is
 Q   allowed and the reason for the patch in the first place.

 Can you elaborate on that please? As far as I see, any rtentry that has
 RTF_GATEWAY would return with EINVAL. The first if() clause doesn't
 do any actual processing, only checking flags and memcmp()ing. The third
 clause either. The error is never reset to 0. So, I don't see any
 difference in returning EINVAL for RTF_GATEWAY immediately or later
 after other checks.



in.c.diff
Description: Binary data
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r226224 - head/sys/netinet

[Qing Li]

Author: qingli
Date: Mon Oct 10 17:41:11 2011
New Revision: 226224
URL: http://svn.freebsd.org/changeset/base/226224

Log:
  All indirect routes will fail the rtcheck, except for a special host
  route where the destination IP and the gateway IP is the same. This
  special case handling is only meant for backward compatibility reason.
  The last commit introduced a bug in the route check logic, where a
  valid special case is treated as an error. This patch fixes that bug
  along with some code cleanup.
  
  Suggested by: gleb
  Reviewed by:  kmacy, discussed with gleb
  MFC after:1 day

Modified:
  head/sys/netinet/in.c

Modified: head/sys/netinet/in.c
==
--- head/sys/netinet/in.c   Mon Oct 10 17:39:58 2011(r226223)
+++ head/sys/netinet/in.c   Mon Oct 10 17:41:11 2011(r226224)
@@ -1414,8 +1414,6 @@ static int
 in_lltable_rtcheck(struct ifnet *ifp, u_int flags, const struct sockaddr 
*l3addr)
 {
struct rtentry *rt;
-   struct ifnet *xifp;
-   int error = 0;
 
KASSERT(l3addr-sa_family == AF_INET,
(sin_family %d, l3addr-sa_family));
@@ -1432,21 +1430,16 @@ in_lltable_rtcheck(struct ifnet *ifp, u_
 * such as MANET, and the interface is of the correct type, then
 * allow for ARP to proceed.
 */
-   if (rt-rt_flags  (RTF_GATEWAY | RTF_HOST)) {
-   xifp = rt-rt_ifp;
-   
-   if (xifp  (xifp-if_type != IFT_ETHER ||
-(xifp-if_flags  (IFF_NOARP | IFF_STATICARP)) != 0))
-   error = EINVAL;
-
-   if (memcmp(rt-rt_gateway-sa_data, l3addr-sa_data,
-   sizeof(in_addr_t)) != 0)
-   error = EINVAL;
-   }
-
if (rt-rt_flags  RTF_GATEWAY) {
-   RTFREE_LOCKED(rt);
-   return (EINVAL);
+   if (!(rt-rt_flags  RTF_HOST) || !rt-rt_ifp ||
+   rt-rt_ifp-if_type != IFT_ETHER ||
+ (rt-rt_ifp-if_flags  
+  (IFF_NOARP | IFF_STATICARP)) != 0 ||
+ memcmp(rt-rt_gateway-sa_data, l3addr-sa_data,
+sizeof(in_addr_t)) != 0) {
+   RTFREE_LOCKED(rt);
+   return (EINVAL);
+   }
}
 
/*
@@ -1455,32 +1448,31 @@ in_lltable_rtcheck(struct ifnet *ifp, u_
 * interfaces have the same prefix. An incoming packet arrives
 * on one interface and the corresponding outgoing packet leaves
 * another interface.
-* 
 */
if (rt-rt_ifp != ifp) {
-   char *sa, *mask, *addr, *lim;
+   const char *sa, *mask, *addr, *lim;
int len;
 
-   sa = (char *)rt_key(rt);
-   mask = (char *)rt_mask(rt);
-   addr = (char *)__DECONST(struct sockaddr *, l3addr);
-   len = ((struct sockaddr_in *)__DECONST(struct sockaddr *, 
l3addr))-sin_len;
+   sa = (const char *)rt_key(rt);
+   mask = (const char *)rt_mask(rt);
+   addr = (const char *)l3addr;
+   len = ((const struct sockaddr_in *)l3addr)-sin_len;
lim = addr + len;
 
for ( ; addr  lim; sa++, mask++, addr++) {
if ((*sa ^ *addr)  *mask) {
-   error = EINVAL;
 #ifdef DIAGNOSTIC
log(LOG_INFO, IPv4 address: \%s\ is not on 
the network\n,
inet_ntoa(((const struct sockaddr_in 
*)l3addr)-sin_addr));
 #endif
-   break;
+   RTFREE_LOCKED(rt);
+   return (EINVAL);
}
}
}
 
RTFREE_LOCKED(rt);
-   return (error);
+   return (0);
 }
 
 /*
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r226224 - head/sys/netinet

[Qing Li]

MFC 225946 is the original patch, 225947 messed up the logic a bit
while putting in the fix for another issue. 226224 is the fix to 225947,
which I will MFC tomorrow.

--Qing


2011/10/10 Gleb Smirnoff gleb...@freebsd.org:
  Qing,

 On Mon, Oct 10, 2011 at 05:41:11PM +, Qing Li wrote:
 Q Author: qingli
 Q Date: Mon Oct 10 17:41:11 2011
 Q New Revision: 226224
 Q URL: http://svn.freebsd.org/changeset/base/226224
 Q
 Q Log:
 Q   All indirect routes will fail the rtcheck, except for a special host
 Q   route where the destination IP and the gateway IP is the same. This
 Q   special case handling is only meant for backward compatibility reason.
 Q   The last commit introduced a bug in the route check logic, where a
 Q   valid special case is treated as an error. This patch fixes that bug
 Q   along with some code cleanup.
 Q
 Q   Suggested by:      gleb
 Q   Reviewed by:       kmacy, discussed with gleb
 Q   MFC after: 1 day

  Looks like you have committed a slightly different patch to stable/8
 in r226230. Is that okay?

  Also, you haven't awaited even one day, while our policy suggests at
 least 3 days before MFC, and 3 days is actually a delay for critical
 fixes.

 P.S. Now I am not the only Gleb at FreeBSD.org community. Recently
 Gleb Kurtsou joined us, and his login name is exactly gleb, while
 mine is glebius. So, your commit may be confusing to later
 reviewers of VCS history.

 --
 Totus tuus, Glebius.

___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r225947 - head/sys/netinet

[Qing Li]

Hi Gleb,


 On Mon, Oct 03, 2011 at 07:51:19PM +, Qing Li wrote:
 Q Author: qingli
 Q Date: Mon Oct  3 19:51:18 2011
 Q New Revision: 225947
 Q URL: http://svn.freebsd.org/changeset/base/225947
 Q
 Q Log:
 Q   A system may have multiple physical interfaces, all of which are on the
 Q   same prefix. Since a single route entry is installed for the prefix
 Q   (without RADIX_MPATH), incoming packets on the interfaces that are not
 Q   associated with the prefix route may trigger an error message about
 Q   unable to allocation LLE entry, and fails L2. This patch makes sure a
 Q   valid route is present in the system, and allow the aforementioned
 Q   condition to exist and treats as valid.
 Q
 Q   Reviewed by:       bz
 Q   MFC after: 5 days

  this commit together with r225946 makes the in_lltable_rtcheck()
 quite difficult to understand.

  What confuses me most, is that in lines 1435-1445 you are
 assigning error to a positive value, BUT proceeding further
 with function.


   This is what was there before (meaning returning error immediately),
   but I guess a couple of folks felt it looked a bit cluttered.
   This is mostly due to the fact the RTFREE_LOCKED() operation
   has to be performed before returning.


 Well, after third review it is clear, that
 next if() case would definitely be true, and you would proceed
 with return. But that is difficult to see from first glance.


   Not so, only for an indirect prefix route.


 I'd suggest to remove error variable, return immediately in
 all error cases, and also the RTF_GATEWAY check can be shifted up,
 since it is the most simple and the most usual to be true.


  No, the RTF_GATEWAY check cannot be shifted up because if we did
  that, the (indirect host route, with destination matching the gateway IP)
  would never be executed, if when that set of conditions are true, which is
  allowed and the reason for the patch in the first place.


 Also, in this commit you really do not need the __DECONST hacks.


  Okay, I must had a temporary short circuit. I will fix that.

  Thanks

  --Qing
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r226114 - head/sys/netinet

[Qing Li]

Author: qingli
Date: Fri Oct  7 18:01:34 2011
New Revision: 226114
URL: http://svn.freebsd.org/changeset/base/226114

Log:
  Remove the reference held on the loopback route when the interface
  address is being deleted. Only the last reference holder deletes the
  loopback route. All other delete operations just clear the IFA_RTSELF
  flag.
  
  PR:   kern/159601
  Submitted by: pluknet
  Reviewed by:  discussed on net@
  MFC after:3 days

Modified:
  head/sys/netinet/in.c

Modified: head/sys/netinet/in.c
==
--- head/sys/netinet/in.c   Fri Oct  7 16:39:03 2011(r226113)
+++ head/sys/netinet/in.c   Fri Oct  7 18:01:34 2011(r226114)
@@ -1126,8 +1126,10 @@ in_scrubprefix(struct in_ifaddr *target,
RT_LOCK(ia_ro.ro_rt);
if (ia_ro.ro_rt-rt_refcnt = 1)
freeit = 1;
-   else
+   else if (flags  LLE_STATIC) {
RT_REMREF(ia_ro.ro_rt);
+   target-ia_flags = ~IFA_RTSELF;
+   }
RTFREE_LOCKED(ia_ro.ro_rt);
}
if (freeit  (flags  LLE_STATIC)) {
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r226120 - head/sys/netinet

[Qing Li]

Author: qingli
Date: Fri Oct  7 22:22:19 2011
New Revision: 226120
URL: http://svn.freebsd.org/changeset/base/226120

Log:
  Do not try removing an ARP entry associated with a given interface
  address if that interface does not support ARP. Otherwise the
  system will generate error messages unnecessarily due to the missing
  entry.
  
  PR:   kern/159602
  Submitted by: pluknet
  MFC after:3 days

Modified:
  head/sys/netinet/in.c

Modified: head/sys/netinet/in.c
==
--- head/sys/netinet/in.c   Fri Oct  7 22:14:18 2011(r226119)
+++ head/sys/netinet/in.c   Fri Oct  7 22:22:19 2011(r226120)
@@ -1138,7 +1138,8 @@ in_scrubprefix(struct in_ifaddr *target,
if (error == 0)
target-ia_flags = ~IFA_RTSELF;
}
-   if (flags  LLE_STATIC)
+   if ((flags  LLE_STATIC) 
+   !(target-ia_ifp-if_flags  IFF_NOARP))
/* remove arp cache */
arp_ifscrub(target-ia_ifp, 
IA_SIN(target)-sin_addr.s_addr);
}
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r226040 - head/sys/netinet6

[Qing Li]

Author: qingli
Date: Wed Oct  5 16:27:11 2011
New Revision: 226040
URL: http://svn.freebsd.org/changeset/base/226040

Log:
  The IFA_RTSELF instead of the IFA_ROUTE flag should be checked to
  determine if a loopback route should be installed for an interface
  IPv6 address. Another condition is the address must not belong to a
  looopback interface.
  
  Reviewed by:  hrs
  MFC after:3 days

Modified:
  head/sys/netinet6/in6.c

Modified: head/sys/netinet6/in6.c
==
--- head/sys/netinet6/in6.c Wed Oct  5 16:03:47 2011(r226039)
+++ head/sys/netinet6/in6.c Wed Oct  5 16:27:11 2011(r226040)
@@ -1810,9 +1810,9 @@ in6_ifinit(struct ifnet *ifp, struct in6
/*
 * add a loopback route to self
 */
-   if (!(ia-ia_flags  IFA_ROUTE)
+   if (!(ia-ia_flags  IFA_RTSELF)
 (V_nd6_useloopback
-   || (ifp-if_flags  IFF_LOOPBACK))) {
+!(ifp-if_flags  IFF_LOOPBACK))) {
error = ifa_add_loopback_route((struct ifaddr *)ia,
   (struct sockaddr *)ia-ia_addr);
if (error == 0)
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r226040 - head/sys/netinet6

[Qing Li]

Correct, but local addresses assigned to interfaces that support
address resolution are
still reachable. For those addresses mapped to pseduo interfaces,
those are not reachable
anyways.

See ML thread

  http://unix.derkeiler.com/Mailing-Lists/FreeBSD/net/2009-09/msg00241.html

--Qing


On Wed, Oct 5, 2011 at 4:21 PM, Bjoern A. Zeeb
bzeeb-li...@lists.zabbadoz.net wrote:

 On 5. Oct 2011, at 16:27 , Qing Li wrote:

 Author: qingli
 Date: Wed Oct  5 16:27:11 2011
 New Revision: 226040
 URL: http://svn.freebsd.org/changeset/base/226040

 Log:
  The IFA_RTSELF instead of the IFA_ROUTE flag should be checked to
  determine if a loopback route should be installed for an interface
  IPv6 address. Another condition is the address must not belong to a
  looopback interface.

 If I set useloopback to 0 my loopback will no longer have a route to itself 
 anymore now?


  Reviewed by: hrs
  MFC after:   3 days

 Modified:
  head/sys/netinet6/in6.c

 Modified: head/sys/netinet6/in6.c
 ==
 --- head/sys/netinet6/in6.c   Wed Oct  5 16:03:47 2011        (r226039)
 +++ head/sys/netinet6/in6.c   Wed Oct  5 16:27:11 2011        (r226040)
 @@ -1810,9 +1810,9 @@ in6_ifinit(struct ifnet *ifp, struct in6
       /*
        * add a loopback route to self
        */
 -     if (!(ia-ia_flags  IFA_ROUTE)
 +     if (!(ia-ia_flags  IFA_RTSELF)
            (V_nd6_useloopback
 -             || (ifp-if_flags  IFF_LOOPBACK))) {
 +              !(ifp-if_flags  IFF_LOOPBACK))) {
               error = ifa_add_loopback_route((struct ifaddr *)ia,
                                      (struct sockaddr *)ia-ia_addr);
               if (error == 0)

 --
 Bjoern A. Zeeb                                 You have to have visions!
         Stop bit received. Insert coin for new address family.


___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r225946 - head/sys/netinet

[Qing Li]

Author: qingli
Date: Mon Oct  3 19:06:55 2011
New Revision: 225946
URL: http://svn.freebsd.org/changeset/base/225946

Log:
  This patch allows ARP to work properly in the presence of
  self-referencing routes. This patch is a rework of r223862.
  
  Reviewed by:  bz, zec
  MFC after:5 days

Modified:
  head/sys/netinet/in.c

Modified: head/sys/netinet/in.c
==
--- head/sys/netinet/in.c   Mon Oct  3 18:05:51 2011(r225945)
+++ head/sys/netinet/in.c   Mon Oct  3 19:06:55 2011(r225946)
@@ -1411,6 +1411,8 @@ static int
 in_lltable_rtcheck(struct ifnet *ifp, u_int flags, const struct sockaddr 
*l3addr)
 {
struct rtentry *rt;
+   struct ifnet *xifp;
+   int error = 0;
 
KASSERT(l3addr-sa_family == AF_INET,
(sin_family %d, l3addr-sa_family));
@@ -1418,30 +1420,35 @@ in_lltable_rtcheck(struct ifnet *ifp, u_
/* XXX rtalloc1 should take a const param */
rt = rtalloc1(__DECONST(struct sockaddr *, l3addr), 0, 0);
 
+   if (rt == NULL)
+   return (EINVAL);
+
/*
 * If the gateway for an existing host route matches the target L3
-* address, allow for ARP to proceed.
+* address, which is a special route inserted by some implementation
+* such as MANET, and the interface is of the correct type, then
+* allow for ARP to proceed.
 */
-   if (rt != NULL  (rt-rt_flags  (RTF_HOST|RTF_GATEWAY)) 
-   rt-rt_gateway-sa_family == AF_INET 
-   memcmp(rt-rt_gateway-sa_data, l3addr-sa_data, 4) == 0) {
-   RTFREE_LOCKED(rt);
-   return (0);
-   }
-
-   if (rt == NULL || (!(flags  LLE_PUB) 
-  ((rt-rt_flags  RTF_GATEWAY) || 
-   (rt-rt_ifp != ifp {
+   if (rt-rt_flags  (RTF_GATEWAY | RTF_HOST)) {
+   xifp = rt-rt_ifp;
+   
+   if (xifp  (xifp-if_type != IFT_ETHER ||
+(xifp-if_flags  (IFF_NOARP | IFF_STATICARP)) != 0))
+   error = EINVAL;
+
+   if (memcmp(rt-rt_gateway-sa_data, l3addr-sa_data,
+   sizeof(in_addr_t)) != 0)
+   error = EINVAL;
+   } else if (!(flags  LLE_PUB)  ((rt-rt_flags  RTF_GATEWAY) ||
+   (rt-rt_ifp != ifp))) {
 #ifdef DIAGNOSTIC
log(LOG_INFO, IPv4 address: \%s\ is not on the network\n,
inet_ntoa(((const struct sockaddr_in *)l3addr)-sin_addr));
 #endif
-   if (rt != NULL)
-   RTFREE_LOCKED(rt);
-   return (EINVAL);
+   error = EINVAL;
}
RTFREE_LOCKED(rt);
-   return 0;
+   return (error);
 }
 
 /*
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r225947 - head/sys/netinet

[Qing Li]

Author: qingli
Date: Mon Oct  3 19:51:18 2011
New Revision: 225947
URL: http://svn.freebsd.org/changeset/base/225947

Log:
  A system may have multiple physical interfaces, all of which are on the
  same prefix. Since a single route entry is installed for the prefix
  (without RADIX_MPATH), incoming packets on the interfaces that are not
  associated with the prefix route may trigger an error message about
  unable to allocation LLE entry, and fails L2. This patch makes sure a
  valid route is present in the system, and allow the aforementioned
  condition to exist and treats as valid.
  
  Reviewed by:  bz
  MFC after:5 days

Modified:
  head/sys/netinet/in.c

Modified: head/sys/netinet/in.c
==
--- head/sys/netinet/in.c   Mon Oct  3 19:06:55 2011(r225946)
+++ head/sys/netinet/in.c   Mon Oct  3 19:51:18 2011(r225947)
@@ -1439,14 +1439,43 @@ in_lltable_rtcheck(struct ifnet *ifp, u_
if (memcmp(rt-rt_gateway-sa_data, l3addr-sa_data,
sizeof(in_addr_t)) != 0)
error = EINVAL;
-   } else if (!(flags  LLE_PUB)  ((rt-rt_flags  RTF_GATEWAY) ||
-   (rt-rt_ifp != ifp))) {
+   }
+
+   if (rt-rt_flags  RTF_GATEWAY) {
+   RTFREE_LOCKED(rt);
+   return (EINVAL);
+   }
+
+   /*
+* Make sure that at least the destination address is covered 
+* by the route. This is for handling the case where 2 or more 
+* interfaces have the same prefix. An incoming packet arrives
+* on one interface and the corresponding outgoing packet leaves
+* another interface.
+* 
+*/
+   if (rt-rt_ifp != ifp) {
+   char *sa, *mask, *addr, *lim;
+   int len;
+
+   sa = (char *)rt_key(rt);
+   mask = (char *)rt_mask(rt);
+   addr = (char *)__DECONST(struct sockaddr *, l3addr);
+   len = ((struct sockaddr_in *)__DECONST(struct sockaddr *, 
l3addr))-sin_len;
+   lim = addr + len;
+
+   for ( ; addr  lim; sa++, mask++, addr++) {
+   if ((*sa ^ *addr)  *mask) {
+   error = EINVAL;
 #ifdef DIAGNOSTIC
-   log(LOG_INFO, IPv4 address: \%s\ is not on the network\n,
-   inet_ntoa(((const struct sockaddr_in *)l3addr)-sin_addr));
+   log(LOG_INFO, IPv4 address: \%s\ is not on 
the network\n,
+   inet_ntoa(((const struct sockaddr_in 
*)l3addr)-sin_addr));
 #endif
-   error = EINVAL;
+   break;
+   }
+   }
}
+
RTFREE_LOCKED(rt);
return (error);
 }
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r225405 - head/sys/dev/ixgbe

[Qing Li]

Author: qingli
Date: Mon Sep  5 17:54:19 2011
New Revision: 225405
URL: http://svn.freebsd.org/changeset/base/225405

Log:
  The maximum read size of incoming packets is done in 1024-byte increments.
  The current code was rounding down the maximum frame size instead of
  routing up, resulting in a read size of 1024 bytes, in the non-jumbo
  frame case, and splitting the packets across multiple mbufs.
  
  Consequently the above problem exposed another issue, which is when
  packets were splitted across multiple mbufs, and all of the mbufs in the
  chain have the M_PKTHDR flag set.
  
  Submitted by: original patch by Ray Ruvinskiy at BlueCoat dot com
  Reviewed by:  jfv, kmacy, rwatson
  Approved by:  re (rwatson)
  MFC after:5 days

Modified:
  head/sys/dev/ixgbe/ixgbe.c

Modified: head/sys/dev/ixgbe/ixgbe.c
==
--- head/sys/dev/ixgbe/ixgbe.c  Mon Sep  5 17:45:24 2011(r225404)
+++ head/sys/dev/ixgbe/ixgbe.c  Mon Sep  5 17:54:19 2011(r225405)
@@ -3849,6 +3849,8 @@ fail:
  **/
 #define IXGBE_SRRCTL_BSIZEHDRSIZE_SHIFT 2
 
+#define BSIZEPKT_ROUNDUP ((1IXGBE_SRRCTL_BSIZEPKT_SHIFT)-1)
+   
 static void
 ixgbe_initialize_receive_units(struct adapter *adapter)
 {
@@ -3882,7 +3884,7 @@ ixgbe_initialize_receive_units(struct ad
hlreg = ~IXGBE_HLREG0_JUMBOEN;
IXGBE_WRITE_REG(hw, IXGBE_HLREG0, hlreg);
 
-   bufsz = adapter-rx_mbuf_sz   IXGBE_SRRCTL_BSIZEPKT_SHIFT;
+   bufsz = (adapter-rx_mbuf_sz + BSIZEPKT_ROUNDUP)  
IXGBE_SRRCTL_BSIZEPKT_SHIFT;
 
for (int i = 0; i  adapter-num_queues; i++, rxr++) {
u64 rdba = rxr-rxdma.dma_paddr;
@@ -4300,9 +4302,10 @@ ixgbe_rxeof(struct ix_queue *que, int co
sendmp = rbuf-fmp;
rbuf-m_pack = rbuf-fmp = NULL;
 
-   if (sendmp != NULL) /* secondary frag */
+   if (sendmp != NULL) {  /* secondary frag */
+   mp-m_flags = ~M_PKTHDR;
sendmp-m_pkthdr.len += mp-m_len;
-   else {
+   } else {
/* first desc of a non-ps chain */
sendmp = mp;
sendmp-m_flags |= M_PKTHDR;
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r225223 - head/sys/netinet

[Qing Li]

Author: qingli
Date: Sun Aug 28 00:14:40 2011
New Revision: 225223
URL: http://svn.freebsd.org/changeset/base/225223

Log:
  When an interface address route is removed from the system, another
  route with the same prefix is searched for as a replacement. The
  current code did not bypass routes that have non-operational
  interfaces. This patch fixes that bug and will find a replacement
  route with an active interface.
  
  PR:   kern/159603
  Submitted by: pluknet, ambrisko at ambrisko dot com
  Reviewed by:  discussed on net@
  Approved by:  re (bz)
  MFC after:3 days

Modified:
  head/sys/netinet/in.c

Modified: head/sys/netinet/in.c
==
--- head/sys/netinet/in.c   Sat Aug 27 22:10:53 2011(r225222)
+++ head/sys/netinet/in.c   Sun Aug 28 00:14:40 2011(r225223)
@@ -1163,7 +1163,8 @@ in_scrubprefix(struct in_ifaddr *target,
p.s_addr = ia-ia_sockmask.sin_addr.s_addr;
}
 
-   if (prefix.s_addr != p.s_addr)
+   if ((prefix.s_addr != p.s_addr) ||
+   !(ia-ia_ifp-if_flags  IFF_UP))
continue;
 
/*
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r225163 - head/sys/net

[Qing Li]

Author: qingli
Date: Thu Aug 25 04:31:20 2011
New Revision: 225163
URL: http://svn.freebsd.org/changeset/base/225163

Log:
  When the RADIX_MPATH kernel option is enabled, the RADIX_MPATH code tries
  to find the first route node of an ECMP chain before executing the route
  command. If the system has a default route, and the specific route argument
  to the command does not exist in the routing table, then the default route
  would be reached. The current code does not verify the reached node matches
  the given route argument, therefore erroneous removed the entry. This patch
  fixes that bug.
  
  Approved by:  re
  MFC after:3 days

Modified:
  head/sys/net/radix_mpath.c

Modified: head/sys/net/radix_mpath.c
==
--- head/sys/net/radix_mpath.c  Thu Aug 25 01:47:26 2011(r225162)
+++ head/sys/net/radix_mpath.c  Thu Aug 25 04:31:20 2011(r225163)
@@ -96,10 +96,7 @@ rt_mpath_matchgate(struct rtentry *rt, s
 {
struct radix_node *rn;
 
-   if (!rn_mpath_next((struct radix_node *)rt))
-   return rt;
-
-   if (!gate)
+   if (!gate || !rt-rt_gateway)
return NULL;
 
/* beyond here, we use rn as the master copy */
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r222438 - head/sys/netinet

[Qing Li]

Author: qingli
Date: Sun May 29 02:21:35 2011
New Revision: 222438
URL: http://svn.freebsd.org/changeset/base/222438

Log:
  Supply the LLE_STATIC flag bit to in_ifscurb() when scrubbing interface
  address so that proper clean up will take place in the routing code.
  This patch fixes the bootp panic on startup problem. Also, added more
  error handling and logging code in function in_scrubprefix().
  
  MFC after:5 days

Modified:
  head/sys/netinet/in.c

Modified: head/sys/netinet/in.c
==
--- head/sys/netinet/in.c   Sun May 29 02:10:57 2011(r222437)
+++ head/sys/netinet/in.c   Sun May 29 02:21:35 2011(r222438)
@@ -548,7 +548,7 @@ in_control(struct socket *so, u_long cmd
 * is the same as before, then the call is 
 * un-necessarily executed here.
 */
-   in_ifscrub(ifp, ia, 0);
+   in_ifscrub(ifp, ia, LLE_STATIC);
ia-ia_sockmask = ifra-ifra_mask;
ia-ia_sockmask.sin_family = AF_INET;
ia-ia_subnetmask =
@@ -557,7 +557,7 @@ in_control(struct socket *so, u_long cmd
}
if ((ifp-if_flags  IFF_POINTOPOINT) 
(ifra-ifra_dstaddr.sin_family == AF_INET)) {
-   in_ifscrub(ifp, ia, 0);
+   in_ifscrub(ifp, ia, LLE_STATIC);
ia-ia_dstaddr = ifra-ifra_dstaddr;
maskIsNew  = 1; /* We lie; but the effect's the same */
}
@@ -1179,14 +1179,20 @@ in_scrubprefix(struct in_ifaddr *target,
 (ia-ia_ifp-if_type != IFT_CARP)) {
ifa_ref(ia-ia_ifa);
IN_IFADDR_RUNLOCK();
-   rtinit((target-ia_ifa), (int)RTM_DELETE,
+   error = rtinit((target-ia_ifa), (int)RTM_DELETE,
rtinitflags(target));
-   target-ia_flags = ~IFA_ROUTE;
-
+   if (error == 0)
+   target-ia_flags = ~IFA_ROUTE;
+   else
+   log(LOG_INFO, in_scrubprefix: err=%d, old 
prefix delete failed\n,
+   error);
error = rtinit(ia-ia_ifa, (int)RTM_ADD,
rtinitflags(ia) | RTF_UP);
if (error == 0)
ia-ia_flags |= IFA_ROUTE;
+   else
+   log(LOG_INFO, in_scrubprefix: err=%d, new 
prefix add failed\n,
+   error);
ifa_free(ia-ia_ifa);
return (error);
}
@@ -1210,9 +1216,12 @@ in_scrubprefix(struct in_ifaddr *target,
/*
 * As no-one seem to have this prefix, we can remove the route.
 */
-   rtinit((target-ia_ifa), (int)RTM_DELETE, rtinitflags(target));
-   target-ia_flags = ~IFA_ROUTE;
-   return (0);
+   error = rtinit((target-ia_ifa), (int)RTM_DELETE, rtinitflags(target));
+   if (error == 0)
+   target-ia_flags = ~IFA_ROUTE;
+   else
+   log(LOG_INFO, in_scrubprefix: err=%d, prefix delete failed\n, 
error);
+   return (error);
 }
 
 #undef rtinitflags
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r222143 - in head/sys: net netinet netinet6

[Qing Li]

Author: qingli
Date: Fri May 20 19:12:20 2011
New Revision: 222143
URL: http://svn.freebsd.org/changeset/base/222143

Log:
  The statically configured (permanent) ARP entries are removed when an
  interface is brought down, even though the interface address is still
  valid. This patch maintains the permanent ARP entries as long as the
  interface address (having the same prefix as that of the ARP entries)
  is valid.
  
  Reviewed by:  delphij
  MFC after:5 days

Modified:
  head/sys/net/if_llatbl.c
  head/sys/net/if_llatbl.h
  head/sys/netinet/in.c
  head/sys/netinet/in_var.h
  head/sys/netinet/raw_ip.c
  head/sys/netinet6/in6.c

Modified: head/sys/net/if_llatbl.c
==
--- head/sys/net/if_llatbl.cFri May 20 18:27:13 2011(r222142)
+++ head/sys/net/if_llatbl.cFri May 20 19:12:20 2011(r222143)
@@ -228,7 +228,8 @@ lltable_drain(int af)
 #endif
 
 void
-lltable_prefix_free(int af, struct sockaddr *prefix, struct sockaddr *mask)
+lltable_prefix_free(int af, struct sockaddr *prefix, struct sockaddr *mask,
+   u_int flags)
 {
struct lltable *llt;
 
@@ -237,7 +238,7 @@ lltable_prefix_free(int af, struct socka
if (llt-llt_af != af)
continue;
 
-   llt-llt_prefix_free(llt, prefix, mask);
+   llt-llt_prefix_free(llt, prefix, mask, flags);
}
LLTABLE_RUNLOCK();
 }

Modified: head/sys/net/if_llatbl.h
==
--- head/sys/net/if_llatbl.hFri May 20 18:27:13 2011(r222142)
+++ head/sys/net/if_llatbl.hFri May 20 19:12:20 2011(r222143)
@@ -155,7 +155,8 @@ struct lltable {
void(*llt_free)(struct lltable *, struct llentry *);
void(*llt_prefix_free)(struct lltable *,
const struct sockaddr *prefix,
-   const struct sockaddr *mask);
+   const struct sockaddr *mask,
+   u_int flags);
struct llentry *(*llt_lookup)(struct lltable *, u_int flags,
const struct sockaddr *l3addr);
int (*llt_rtcheck)(struct ifnet *, u_int flags,
@@ -184,7 +185,7 @@ MALLOC_DECLARE(M_LLTABLE);
 struct lltable *lltable_init(struct ifnet *, int);
 void   lltable_free(struct lltable *);
 void   lltable_prefix_free(int, struct sockaddr *, 
-   struct sockaddr *);
+   struct sockaddr *, u_int);
 #if 0
 void   lltable_drain(int);
 #endif

Modified: head/sys/netinet/in.c
==
--- head/sys/netinet/in.c   Fri May 20 18:27:13 2011(r222142)
+++ head/sys/netinet/in.c   Fri May 20 19:12:20 2011(r222143)
@@ -70,7 +70,7 @@ static int in_lifaddr_ioctl(struct socke
struct ifnet *, struct thread *);
 
 static int in_addprefix(struct in_ifaddr *, int);
-static int in_scrubprefix(struct in_ifaddr *);
+static int in_scrubprefix(struct in_ifaddr *, u_int);
 static voidin_socktrim(struct sockaddr_in *);
 static int in_ifinit(struct ifnet *,
struct in_ifaddr *, struct sockaddr_in *, int);
@@ -548,7 +548,7 @@ in_control(struct socket *so, u_long cmd
 * is the same as before, then the call is 
 * un-necessarily executed here.
 */
-   in_ifscrub(ifp, ia);
+   in_ifscrub(ifp, ia, 0);
ia-ia_sockmask = ifra-ifra_mask;
ia-ia_sockmask.sin_family = AF_INET;
ia-ia_subnetmask =
@@ -557,7 +557,7 @@ in_control(struct socket *so, u_long cmd
}
if ((ifp-if_flags  IFF_POINTOPOINT) 
(ifra-ifra_dstaddr.sin_family == AF_INET)) {
-   in_ifscrub(ifp, ia);
+   in_ifscrub(ifp, ia, 0);
ia-ia_dstaddr = ifra-ifra_dstaddr;
maskIsNew  = 1; /* We lie; but the effect's the same */
}
@@ -585,7 +585,7 @@ in_control(struct socket *so, u_long cmd
/*
 * in_ifscrub kills the interface route.
 */
-   in_ifscrub(ifp, ia);
+   in_ifscrub(ifp, ia, LLE_STATIC);
 
/*
 * in_ifadown gets rid of all the rest of
@@ -829,10 +829,10 @@ in_lifaddr_ioctl(struct socket *so, u_lo
  * Delete any existing route for an interface.
  */
 void
-in_ifscrub(struct ifnet *ifp, struct in_ifaddr *ia)
+in_ifscrub(struct ifnet *ifp, struct in_ifaddr *ia, u_int flags)
 {
 
-   in_scrubprefix(ia);
+   in_scrubprefix(ia, flags);
 }
 
 /*
@@ -887,7 +887,7 

svn commit: r212502 - head/sys/netinet

[Qing Li]

Author: qingli
Date: Sun Sep 12 18:04:47 2010
New Revision: 212502
URL: http://svn.freebsd.org/changeset/base/212502

Log:
  Adding an address on an interface also requires the loopback route to
  that address be installed.
  
  PR:   kern/150481
  Submitted by: Ingo Flaschberger if at xip.at
  MFC after:5 days

Modified:
  head/sys/netinet/raw_ip.c

Modified: head/sys/netinet/raw_ip.c
==
--- head/sys/netinet/raw_ip.c   Sun Sep 12 17:55:56 2010(r212501)
+++ head/sys/netinet/raw_ip.c   Sun Sep 12 18:04:47 2010(r212502)
@@ -741,6 +741,8 @@ rip_ctlinput(int cmd, struct sockaddr *s
if (err == 0)
ia-ia_flags |= IFA_ROUTE;
err = ifa_add_loopback_route((struct ifaddr *)ia, sa);
+   if (err == 0)
+   ia-ia_flags |= IFA_RTSELF;
ifa_free(ia-ia_ifa);
break;
}
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r208553 - in head/sys: net netinet

[Qing Li]

Author: qingli
Date: Tue May 25 20:42:35 2010
New Revision: 208553
URL: http://svn.freebsd.org/changeset/base/208553

Log:
  This patch fixes the problem where proxy ARP entries cannot be added
  over the if_ng interface.
  
  MFC after:3 days

Modified:
  head/sys/net/if.c
  head/sys/net/if_var.h
  head/sys/net/route.c
  head/sys/net/rtsock.c
  head/sys/netinet/in.c
  head/sys/netinet/in_pcb.c
  head/sys/netinet/ip_options.c
  head/sys/netinet/ip_output.c

Modified: head/sys/net/if.c
==
--- head/sys/net/if.c   Tue May 25 20:35:39 2010(r208552)
+++ head/sys/net/if.c   Tue May 25 20:42:35 2010(r208553)
@@ -1607,7 +1607,7 @@ done:
  * is most specific found.
  */
 struct ifaddr *
-ifa_ifwithnet(struct sockaddr *addr)
+ifa_ifwithnet(struct sockaddr *addr, int ignore_ptp)
 {
struct ifnet *ifp;
struct ifaddr *ifa;
@@ -1639,7 +1639,8 @@ ifa_ifwithnet(struct sockaddr *addr)
 
if (ifa-ifa_addr-sa_family != af)
 next:  continue;
-   if (af == AF_INET  ifp-if_flags  IFF_POINTOPOINT) {
+   if (af == AF_INET  
+   ifp-if_flags  IFF_POINTOPOINT  !ignore_ptp) {
/*
 * This is a bit broken as it doesn't
 * take into account that the remote end may

Modified: head/sys/net/if_var.h
==
--- head/sys/net/if_var.h   Tue May 25 20:35:39 2010(r208552)
+++ head/sys/net/if_var.h   Tue May 25 20:42:35 2010(r208553)
@@ -873,7 +873,7 @@ struct  ifaddr *ifa_ifwithaddr(struct soc
 intifa_ifwithaddr_check(struct sockaddr *);
 struct ifaddr *ifa_ifwithbroadaddr(struct sockaddr *);
 struct ifaddr *ifa_ifwithdstaddr(struct sockaddr *);
-struct ifaddr *ifa_ifwithnet(struct sockaddr *);
+struct ifaddr *ifa_ifwithnet(struct sockaddr *, int);
 struct ifaddr *ifa_ifwithroute(int, struct sockaddr *, struct sockaddr *);
 struct ifaddr *ifa_ifwithroute_fib(int, struct sockaddr *, struct sockaddr *, 
u_int);
 

Modified: head/sys/net/route.c
==
--- head/sys/net/route.cTue May 25 20:35:39 2010(r208552)
+++ head/sys/net/route.cTue May 25 20:42:35 2010(r208553)
@@ -519,7 +519,7 @@ rtredirect_fib(struct sockaddr *dst,
}
 
/* verify the gateway is directly reachable */
-   if ((ifa = ifa_ifwithnet(gateway)) == NULL) {
+   if ((ifa = ifa_ifwithnet(gateway, 0)) == NULL) {
error = ENETUNREACH;
goto out;
}
@@ -686,7 +686,7 @@ ifa_ifwithroute_fib(int flags, struct so
ifa = ifa_ifwithdstaddr(gateway);
}
if (ifa == NULL)
-   ifa = ifa_ifwithnet(gateway);
+   ifa = ifa_ifwithnet(gateway, 0);
if (ifa == NULL) {
struct rtentry *rt = rtalloc1_fib(gateway, 0, RTF_RNH_LOCKED, 
fibnum);
if (rt == NULL)
@@ -797,7 +797,7 @@ rt_getifa_fib(struct rt_addrinfo *info, 
 */
if (info-rti_ifp == NULL  ifpaddr != NULL 
ifpaddr-sa_family == AF_LINK 
-   (ifa = ifa_ifwithnet(ifpaddr)) != NULL) {
+   (ifa = ifa_ifwithnet(ifpaddr, 0)) != NULL) {
info-rti_ifp = ifa-ifa_ifp;
ifa_free(ifa);
}

Modified: head/sys/net/rtsock.c
==
--- head/sys/net/rtsock.c   Tue May 25 20:35:39 2010(r208552)
+++ head/sys/net/rtsock.c   Tue May 25 20:42:35 2010(r208553)
@@ -55,6 +55,7 @@
 #include net/if.h
 #include net/if_dl.h
 #include net/if_llatbl.h
+#include net/if_types.h
 #include net/netisr.h
 #include net/raw_cb.h
 #include net/route.h
@@ -673,12 +674,22 @@ route_output(struct mbuf *m, struct sock
 * another search to retrieve the prefix route of
 * the local end point of the PPP link.
 */
-   if ((rtm-rtm_flags  RTF_ANNOUNCE) 
-   (rt-rt_ifp-if_flags  IFF_POINTOPOINT)) {
+   if (rtm-rtm_flags  RTF_ANNOUNCE) {
struct sockaddr laddr;
-   rt_maskedcopy(rt-rt_ifa-ifa_addr,
- laddr,
- rt-rt_ifa-ifa_netmask);
+
+   if (rt-rt_ifp != NULL  
+   rt-rt_ifp-if_type == IFT_PROPVIRTUAL) {
+   struct ifaddr *ifa;
+
+   ifa = ifa_ifwithnet(info.rti_info[RTAX_DST], 1);
+   if (ifa != NULL)
+   rt_maskedcopy(ifa-ifa_addr,
+ 

svn commit: r205268 - head/sys/dev/mii

[Qing Li]

Author: qingli
Date: Wed Mar 17 22:12:12 2010
New Revision: 205268
URL: http://svn.freebsd.org/changeset/base/205268

Log:
  Set the device capabilities to include dynamic link-state for
  those modern drivers.
  
  Reviewed by:  imp (and suggested by imp)
  MFC after:3 days

Modified:
  head/sys/dev/mii/mii.c

Modified: head/sys/dev/mii/mii.c
==
--- head/sys/dev/mii/mii.c  Wed Mar 17 21:19:30 2010(r205267)
+++ head/sys/dev/mii/mii.c  Wed Mar 17 22:12:12 2010(r205268)
@@ -173,6 +173,8 @@ miibus_attach(device_t dev)
 * XXX: EVIL HACK!
 */
mii-mii_ifp = *(struct 
ifnet**)device_get_softc(device_get_parent(dev));
+   mii-mii_ifp-if_capabilities |= IFCAP_LINKSTATE;
+   mii-mii_ifp-if_capenable |= IFCAP_LINKSTATE;
ivars = device_get_ivars(dev);
ifmedia_init(mii-mii_media, IFM_IMASK, ivars-ifmedia_upd,
ivars-ifmedia_sts);
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r205272 - head/usr.sbin/ppp

[Qing Li]

Author: qingli
Date: Thu Mar 18 00:23:39 2010
New Revision: 205272
URL: http://svn.freebsd.org/changeset/base/205272

Log:
  Need to set the proper flag bit when inserting ARP
  entries into the kernel.
  
  MFC after:3 days

Modified:
  head/usr.sbin/ppp/arp.c

Modified: head/usr.sbin/ppp/arp.c
==
--- head/usr.sbin/ppp/arp.c Wed Mar 17 22:57:58 2010(r205271)
+++ head/usr.sbin/ppp/arp.c Thu Mar 18 00:23:39 2010(r205272)
@@ -119,7 +119,7 @@ arp_ProxySub(struct bundle *bundle, stru
 return 0;
   }
   arpmsg.hdr.rtm_type = add ? RTM_ADD : RTM_DELETE;
-  arpmsg.hdr.rtm_flags = RTF_ANNOUNCE | RTF_HOST | RTF_STATIC;
+  arpmsg.hdr.rtm_flags = RTF_ANNOUNCE | RTF_HOST | RTF_STATIC | RTF_LLDATA;
   arpmsg.hdr.rtm_version = RTM_VERSION;
   arpmsg.hdr.rtm_seq = ++bundle-routing_seq;
   arpmsg.hdr.rtm_addrs = RTA_DST | RTA_GATEWAY;
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r205222 - in head: sbin/ifconfig sys/net

[Qing Li]

Author: qingli
Date: Tue Mar 16 17:59:12 2010
New Revision: 205222
URL: http://svn.freebsd.org/changeset/base/205222

Log:
  Verify interface up status using its link state only
  if the interface has such capability. The interface
  capability flag indicates whether such capability
  exists. This approach is much more backward compatible.
  Physical device driver changes will be part of another
  commit.
  
  Also updated the ifconfig utility to show the LINKSTATE
  capability if present.
  
  Reviewed by:  rwatson, imp, juli
  MFC after:3 days

Modified:
  head/sbin/ifconfig/ifconfig.c
  head/sys/net/if.h
  head/sys/net/if_tap.c
  head/sys/net/if_tun.c
  head/sys/net/route.h

Modified: head/sbin/ifconfig/ifconfig.c
==
--- head/sbin/ifconfig/ifconfig.c   Tue Mar 16 17:45:16 2010
(r205221)
+++ head/sbin/ifconfig/ifconfig.c   Tue Mar 16 17:59:12 2010
(r205222)
@@ -881,7 +881,7 @@ unsetifdescr(const char *val, int value,
 #defineIFCAPBITS \
 \020\1RXCSUM\2TXCSUM\3NETCONS\4VLAN_MTU\5VLAN_HWTAGGING\6JUMBO_MTU\7POLLING \
 \10VLAN_HWCSUM\11TSO4\12TSO6\13LRO\14WOL_UCAST\15WOL_MCAST\16WOL_MAGIC \
-\21VLAN_HWFILTER\23VLAN_HWTSO
+\21VLAN_HWFILTER\23VLAN_HWTSO\24LINKSTATE
 
 /*
  * Print the status of the interface.  If an address family was

Modified: head/sys/net/if.h
==
--- head/sys/net/if.h   Tue Mar 16 17:45:16 2010(r205221)
+++ head/sys/net/if.h   Tue Mar 16 17:59:12 2010(r205222)
@@ -219,6 +219,7 @@ struct if_data {
 #defineIFCAP_VLAN_HWFILTER 0x1 /* interface hw can filter vlan 
tag */
 #defineIFCAP_POLLING_NOCOUNT   0x2 /* polling ticks cannot be 
fragmented */
 #defineIFCAP_VLAN_HWTSO0x4 /* can do IFCAP_TSO on VLANs */
+#defineIFCAP_LINKSTATE 0x8 /* the runtime link state is 
dynamic */
 
 #define IFCAP_HWCSUM   (IFCAP_RXCSUM | IFCAP_TXCSUM)
 #defineIFCAP_TSO   (IFCAP_TSO4 | IFCAP_TSO6)

Modified: head/sys/net/if_tap.c
==
--- head/sys/net/if_tap.c   Tue Mar 16 17:45:16 2010(r205221)
+++ head/sys/net/if_tap.c   Tue Mar 16 17:59:12 2010(r205222)
@@ -443,6 +443,8 @@ tapcreate(struct cdev *dev)
ifp-if_mtu = ETHERMTU;
ifp-if_flags = (IFF_BROADCAST|IFF_SIMPLEX|IFF_MULTICAST);
ifp-if_snd.ifq_maxlen = ifqmaxlen;
+   ifp-if_capabilities |= IFCAP_LINKSTATE;
+   ifp-if_capenable |= IFCAP_LINKSTATE;
 
dev-si_drv1 = tp;
tp-tap_dev = dev;

Modified: head/sys/net/if_tun.c
==
--- head/sys/net/if_tun.c   Tue Mar 16 17:45:16 2010(r205221)
+++ head/sys/net/if_tun.c   Tue Mar 16 17:59:12 2010(r205222)
@@ -386,6 +386,8 @@ tuncreate(const char *name, struct cdev 
ifp-if_snd.ifq_drv_maxlen = 0;
IFQ_SET_READY(ifp-if_snd);
knlist_init_mtx(sc-tun_rsel.si_note, NULL);
+   ifp-if_capabilities |= IFCAP_LINKSTATE;
+   ifp-if_capenable |= IFCAP_LINKSTATE;
 
if_attach(ifp);
bpfattach(ifp, DLT_NULL, sizeof(u_int32_t));

Modified: head/sys/net/route.h
==
--- head/sys/net/route.hTue Mar 16 17:45:16 2010(r205221)
+++ head/sys/net/route.hTue Mar 16 17:59:12 2010(r205222)
@@ -319,8 +319,7 @@ struct rt_addrinfo {
 
 #ifdef _KERNEL
 
-#define RT_LINK_IS_UP(ifp) (((ifp)-if_flags  \
- (IFF_LOOPBACK | IFF_POINTOPOINT)) \
+#define RT_LINK_IS_UP(ifp) (!((ifp)-if_capabilities  IFCAP_LINKSTATE) \
 || (ifp)-if_link_state == LINK_STATE_UP)
 
 #defineRT_LOCK_INIT(_rt) \
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r205024 - head/sys/net

[Qing Li]

Nope, I meant Julian, and what he proposed, and if I understood
correctly, is the simplest
approach and easily done.

-- Qing


On Fri, Mar 12, 2010 at 12:29 AM, Robert N. M. Watson
rwat...@freebsd.org wrote:

 On Mar 12, 2010, at 8:11 AM, Qing Li wrote:

 I like Julian's suggestion because it is simple and very low risk.
 And there isn't a need to check for interface type any more.
 Here is why:

 Re-reading this e-mail: perhaps you mean Juli, not Julian?

___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r205077 - head/sys/net

[Qing Li]

Author: qingli
Date: Fri Mar 12 10:24:58 2010
New Revision: 205077
URL: http://svn.freebsd.org/changeset/base/205077

Log:
  The flow-table module retrieves the destination and source
  address as well as the transport protocol port information
  from the outbound packets. The routing code is generic and
  compares every byte in the given sockaddr object. Therefore
  the temporary sockaddr objects must be cleared due to padding
  bytes. In addition, the port information must be stripped
  or the route search will either fail or return the incorrect
  route entry.
  
  Unit testing is done using OpenVPN over the if_tun interface.
  
  MFC after:7 days

Modified:
  head/sys/net/flowtable.c

Modified: head/sys/net/flowtable.c
==
--- head/sys/net/flowtable.cFri Mar 12 10:01:06 2010(r205076)
+++ head/sys/net/flowtable.cFri Mar 12 10:24:58 2010(r205077)
@@ -593,6 +593,8 @@ flowtable_lookup_mbuf4(struct flowtable 
 
dsin = (struct sockaddr_in *)dsa;
ssin = (struct sockaddr_in *)ssa;
+   bzero(dsin, sizeof(*dsin));
+   bzero(ssin, sizeof(*ssin));
flags = ft-ft_flags;
if (ipv4_mbuf_demarshal(ft, m, ssin, dsin, flags) != 0)
return (NULL);
@@ -796,6 +798,8 @@ flowtable_lookup_mbuf6(struct flowtable 
 
dsin6 = (struct sockaddr_in6 *)dsa;
ssin6 = (struct sockaddr_in6 *)ssa;
+   bzero(dsin6, sizeof(*dsin6));
+   bzero(ssin6, sizeof(*ssin6));
flags = ft-ft_flags;

if (ipv6_mbuf_demarshal(ft, m, ssin6, dsin6, flags) != 0)
@@ -1088,6 +1092,14 @@ flowtable_lookup(struct flowtable *ft, s
 
ro = sro;
memcpy(ro-ro_dst, dsa, sizeof(struct sockaddr_in));
+   /*
+* The harvested source and destination addresses
+* may contain port information if the packet is 
+* from a transport protocol (e.g. TCP/UDP). The 
+* port field must be cleared before performing 
+* a route lookup.
+*/
+   ((struct sockaddr_in *)ro-ro_dst)-sin_port = 0;
dsin = (struct sockaddr_in *)dsa;
ssin = (struct sockaddr_in *)ssa;
if ((dsin-sin_addr.s_addr == ssin-sin_addr.s_addr) ||
@@ -1105,6 +1117,7 @@ flowtable_lookup(struct flowtable *ft, s
ro = (struct route *)sro6;
memcpy(sro6.ro_dst, dsa,
sizeof(struct sockaddr_in6));
+   ((struct sockaddr_in6 *)ro-ro_dst)-sin6_port = 0;
dsin6 = (struct sockaddr_in6 *)dsa;
ssin6 = (struct sockaddr_in6 *)ssa;
 
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r205024 - head/sys/net

[Qing Li]

 We've got LINK_STATE_UNKNOWN, we can just initialize if_link_state to
 this value in ether_ifattach(). And Qing should treat this value as
 LINK_STATE_UP in routing decision until better times.


Thanks everyone for your input.

I generally try to avoid overloading a variable to have more than 1 meaning.
For example, the if_tun interface is in LINK_STATE_UNKNOWN until
it's opened, then the link can be one of the other two states.
Also, some of the pseudo drivers such as if_tun do not call ether_ifattach().

Right now I like to implement Robert's suggestion of using if_capabilities
field. I'd like to create a new flag, IFCAP_LINKSTATE_NOTIFY flag.
The routing decision will check against the if_link_state if this bit
is set.

Only a handful of drivers have this capability. So updating those drivers
is a small task.

Do we agree on this approach?

-- Qing
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

svn commit: r205024 - head/sys/net

[Qing Li]

Author: qingli
Date: Thu Mar 11 17:56:46 2010
New Revision: 205024
URL: http://svn.freebsd.org/changeset/base/205024

Log:
  The if_tap interface is of IFT_ETHERNET type, but it
  does not set or update the if_link_state variable.
  As such RT_LINK_IS_UP() fails for the if_tap interface.
  
  Also, the RT_LINK_IS_UP() needs to bypass all loopback
  interfaces because loopback interfaces are considered
  up logically as long as the system is running.
  
  This patch fixes the above issues by setting and updating
  the if_link_state variable when the tap interface is
  opened or closed respectively. Similary approach is
  already done in the if_tun device.
  
  MFC after:3 days

Modified:
  head/sys/net/if_tap.c
  head/sys/net/route.h

Modified: head/sys/net/if_tap.c
==
--- head/sys/net/if_tap.c   Thu Mar 11 17:15:40 2010(r205023)
+++ head/sys/net/if_tap.c   Thu Mar 11 17:56:46 2010(r205024)
@@ -502,6 +502,7 @@ tapopen(struct cdev *dev, int flag, int 
ifp-if_drv_flags = ~IFF_DRV_OACTIVE;
if (tapuponopen)
ifp-if_flags |= IFF_UP;
+   if_link_state_change(ifp, LINK_STATE_UP);
splx(s);
 
TAPDEBUG(%s is open. minor = %#x\n, ifp-if_xname, dev2unit(dev));
@@ -547,6 +548,7 @@ tapclose(struct cdev *dev, int foo, int 
} else
mtx_unlock(tp-tap_mtx);
 
+   if_link_state_change(ifp, LINK_STATE_DOWN);
funsetown(tp-tap_sigio);
selwakeuppri(tp-tap_rsel, PZERO+1);
KNOTE_UNLOCKED(tp-tap_rsel.si_note, 0);

Modified: head/sys/net/route.h
==
--- head/sys/net/route.hThu Mar 11 17:15:40 2010(r205023)
+++ head/sys/net/route.hThu Mar 11 17:56:46 2010(r205024)
@@ -319,7 +319,9 @@ struct rt_addrinfo {
 
 #ifdef _KERNEL
 
-#define RT_LINK_IS_UP(ifp) ((ifp)-if_link_state == LINK_STATE_UP)
+#define RT_LINK_IS_UP(ifp) (((ifp)-if_flags  \
+ (IFF_LOOPBACK | IFF_POINTOPOINT)) \
+|| (ifp)-if_link_state == LINK_STATE_UP)
 
 #defineRT_LOCK_INIT(_rt) \
mtx_init((_rt)-rt_mtx, rtentry, NULL, MTX_DEF | MTX_DUPOK)
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r205024 - head/sys/net

[Qing Li]

 A couple of questions:

 (1) It used to be the case that quite a few interface drivers and types
 didn't have a notion of link up -- especially older ethernet devices.  Do
 those all have the same problem?  It was probably a design oversight that
 devices don't declare an explicit capability for can report link state.


  What you raised is definitely a possibility and these fixes take the
  similar approach. I am going to try and go through each of these
  drivers in /sys/dev/ and converting them, very soon.


 (2) While loopback interfaces don't really have a link state, they can be
 administratively down -- should/do you check that as well as link state?
 And more generally, even if link is up, administratively down should be
 obeyed?


  For loopback interfaces, althgouth administrative these can be taken down,
  I personally cannot think one practical usage case where ECMP across
  loopback interfaces would be interesting or usefaul. So I can think of
  very little reason to be concerned in the loopback case.


 Finally, it would be neat if there were a way to have information beyond
 link state influence the choice to balance to a particular route/interface.
  For example, imagine if I have a router with ECMP, and on the other side on
 a single ethernet segment, I have two DSL modems.  The ethernet link will
 remain up, but I may (via out-of-band mechanisms, such as SNMP or an active
 probe) be able to tell that one of the DSL lines is down.  Is there a way to
 push that information into the kernel currently without deleting the routes,
 and instead say yeah, but for ECMP purposes this is 'down'?


  The above really falls into policy based routing. And policy based
  routing infrastrucutre is something I have already been working on
but cannot yet
  push back into -current. In fact Julian and I had a conversation about
  this topic during the FIBs implementation time in late 2008.

  This infrastructure enhancement is definitely coming but I cannot yet prvoide
  a timeline for merge back.

  It's mostly a time issue.

  Let me know if I have answered these questions to your satisfaction.

  -- Qing
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r205024 - head/sys/net

[Qing Li]

I guess it's a good time to clean things up. The if_link_state code has been
around for quite some time, either it be fully utilized or not be there at all.
The inconsistency is the root cause.

I will try going through these tonight and hopefully the fix all take a
common approach.

-- Qing


On Thu, Mar 11, 2010 at 3:35 PM, Juli Mallett jmall...@freebsd.org wrote:
 On Thu, Mar 11, 2010 at 15:30, Qing Li qin...@freebsd.org wrote:

 A couple of questions:

 (1) It used to be the case that quite a few interface drivers and types
 didn't have a notion of link up -- especially older ethernet devices.  Do
 those all have the same problem?  It was probably a design oversight that
 devices don't declare an explicit capability for can report link state.


  What you raised is definitely a possibility and these fixes take the
  similar approach. I am going to try and go through each of these
  drivers in /sys/dev/ and converting them, very soon.

 Go through drivers in the embedded port directories, too.  The Octeon
 port's Ethernet driver was broken by this, and it looks like the
 Atheros if_arge is probably broken, too.  I would even suggest going
 back to the old behavior briefly while the port maintainers are given
 an opportunity to update their drivers.  Actually, it looks like only
 MIPS has Ethernet drivers outside of dev/ at a quick glance, but I'd
 be surprised if there weren't other broken examples.

 Juli.

___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r205024 - head/sys/net

[Qing Li]

 If you can think of a way to add some invariants (warn the first time
 a driver receives a packet without having ever set the link state,
 make sure the media status callback sets the valid flag in the
 request, etc) that would probably be very helpful for people who are
 writing network drivers.  If I hadn't been following the threads about
 your changes, I would have had to spend much longer fixing the Octeon
 port's Ethernet driver, wondering why suddenly it broke and
 instrumenting the routing code.  A printf would go a long way.


  You definitely have a very good point here.  I was a bit surprised
  during debugging that the link state is not consistently initialized
  and by far not enforced across all of the drivers. Admittedly I checked
  the most commonly deployed devices and they are in good state.

  I certainly appreciate your patience on this one and will try to get
  it resolved quickly.

  -- Qing
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r205024 - head/sys/net

[Qing Li]

That's a good idea. I will take your approach.

-- Qing


On Thu, Mar 11, 2010 at 11:15 PM, Julian Elischer jul...@elischer.org wrote:
 Juli Mallett wrote:

 On Thu, Mar 11, 2010 at 15:39, Qing Li qin...@freebsd.org wrote:

 I guess it's a good time to clean things up. The if_link_state code has
 been
 around for quite some time, either it be fully utilized or not be there
 at all.
 The inconsistency is the root cause.

 Sure.  There is an increasing amount of stuff that network drivers are
 expected to do, but they work without doing them.  It's easy to think
 you have a functioning network driver and that you can get by without
 adding support for media changes and link status reporting, etc.

 I will try going through these tonight and hopefully the fix all take a
 common approach.

 probably should add a flag that means we have media state
 and if it is not set, assume it is always on.


 If you can think of a way to add some invariants (warn the first time
 a driver receives a packet without having ever set the link state,
 make sure the media status callback sets the valid flag in the
 request, etc) that would probably be very helpful for people who are
 writing network drivers.  If I hadn't been following the threads about
 your changes, I would have had to spend much longer fixing the Octeon
 port's Ethernet driver, wondering why suddenly it broke and
 instrumenting the routing code.  A printf would go a long way.

 Juli.


___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r204902 - in head/sys: net netinet

[Qing Li]

 I looked at it, and at the diff of his original commit.  The changes were
 large enough that I don't want to assume his patch takes care of all the
 issues given that patch hasn't been committed verbatim.


The change itself is not a huge change but if you disagree, then
please be specific.
The current mechanism and code is broken according to the original
design intention.

I have a habit of not committing things or quick finger until everyone
had a chance
to test the patch, although I have done unit testings myself. When you say
... made the kernel toxic and ...I don't want to assume ..., well, again, be
specific instead about what you mean and give me details.

The other reason I decided to postpone is because I wanted to spend
more time with
the PPP driver. Every other PPP interface types (if_tun, if_ng etc.)
updates the if_link_state
variable except PPP.


 We also have reports of the patch not working in freebsd-curr...@.


I have seen that report but it lacks detail. I have asked for more
information and
will investigate further as soon as it becomes available.

-- Qing
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org