Re: in.c autoadding prefix route
Hi Max,
On Thu, Nov 11, 2004 at 09:24:05PM +0100, Max Laier wrote:
> I know I have sent this a couple of times before, but never got anywhere.
> This
> time I am set to commit!
>
Hey, you did it! ;)
> The attached patch (http://people.freebsd.org/~mlaier/in.c.patch) derived
> from
> WIDE via OpenBSD in.c, rev 1.21 improves the handling of automatic prefix
> routes.
>
> Right now you can't have two legs into the same network. If you want to, you
> must give on of the interfaces a host address only (netmask /32). This way it
> is not possible to hand over the route if one of the interfaces is
> "removed" (however this is done in the special case).
>
> The patch allows to add more than on IPv4 address with the same prefix. In
> the
> case that there is a route already, we leave it alone and add the new address
> without the IFA_ROUTE flag. When we remove an address later on, that has a
> route associated, we try to find an alternative address to use for the route
> and hand it over.
>
I cannot give your patch a thorough review at the moment, but I like the
algorithm, and I don't see how it can hurt anything.
> --- ../dist/sys/netinet/in.c Sat Nov 6 21:01:08 2004
> +++ sys/netinet/in.c Mon Nov 8 02:05:17 2004
> @@ -654,14 +684,7 @@
> register struct ifnet *ifp;
> register struct in_ifaddr *ia;
> {
> -
> - if ((ia->ia_flags & IFA_ROUTE) == 0)
> - return;
> - if (ifp->if_flags & (IFF_LOOPBACK|IFF_POINTOPOINT))
> - rtinit(&(ia->ia_ifa), (int)RTM_DELETE, RTF_HOST);
> - else
> - rtinit(&(ia->ia_ifa), (int)RTM_DELETE, 0);
> - ia->ia_flags &= ~IFA_ROUTE;
> + in_scrubprefix(ia);
> }
>
Looks like "ifp" argument is no longer needed for in_ifscrub(),
perhaps it should be killed then.
Also, there are a lot of style bugs (besides those that others
have already mentioned), the most annoying is comments -- they
should be written as per style(9) (make them look like the real
sentences).
Cheers,
--
Ruslan Ermilov
[EMAIL PROTECTED]
FreeBSD committer
pgpUfSbTHMK84.pgp
Description: PGP signature
Re: in.c autoadding prefix route
On Fri, Nov 12, 2004 at 01:16:31AM +0100, Max Laier wrote:
> On Thursday 11 November 2004 22:55, Andrea Campi wrote:
> > On Thu, Nov 11, 2004 at 09:24:05PM +0100, Max Laier wrote:
> > > The attached patch (http://people.freebsd.org/~mlaier/in.c.patch) derived
> > > from WIDE via OpenBSD in.c, rev 1.21 improves the handling of automatic
> > > prefix routes.
> >
> > Sounds like a very useful change indeed.
> >
> > One comment though:
> > > @@ -743,26 +766,7 @@
> > > return (0);
> > >flags |= RTF_HOST;
> > > }
> > > -
> > > - /*-
> > > - * Don't add host routes for interface addresses of
> > > - * 0.0.0.0 --> 0.255.255.255 netmask 255.0.0.0. This makes it
> > > - * possible to assign several such address pairs with consistent
> > > - * results (no host route) and is required by BOOTP.
> > > - *
> > > - * XXX: This is ugly ! There should be a way for the caller to
> > > - * say that they don't want a host route.
> > > - */
> > > - if (ia->ia_addr.sin_addr.s_addr != INADDR_ANY ||
> > > - ia->ia_netmask != IN_CLASSA_NET ||
> > > - ia->ia_dstaddr.sin_addr.s_addr != htonl(IN_CLASSA_HOST)) {
> > > - if ((error = rtinit(&ia->ia_ifa, (int)RTM_ADD, flags)) != 0) {
> > > - ia->ia_addr = oldaddr;
> > > - return (error);
> > > - }
> > > - ia->ia_flags |= IFA_ROUTE;
> > > - }
> > > -
> > > + error = in_addprefix(ia, flags);
> > > /*
> > >* If the interface supports multicast, join the "all hosts"
> > >* multicast group on that interface.
> >
> > Are you sure you want to go on if you got an error? Regardless, you should
> > probably have an empty line after in_addrprefix.
>
> Not sure how to deal with the error case. All errors we should get are memory
> related (as we check carefully that the rtinit will success). This means that
> the following in_addmulti will sleep if we hit it (in_addmulti mallocs with
> M_WAITOK) ... so I guess you are right and it's the least intrusive if we do
> return.
>
The old code did:
if ((error = rtinit()) != 0)
return (error);
Why the new code should behave differently, I fail to see? in_addprefix()
is just a wrapper around rtinit(), after all.
Cheers,
--
Ruslan Ermilov
[EMAIL PROTECTED]
FreeBSD committer
pgpPi1PZW2D2E.pgp
Description: PGP signature
Re: ng_ksocket as divert socket is broken
Brian,
I have finally resolved this problem.
In case when ng_ksocket acts as divert socket, we get diverted packets
returned from 'pseudo-userland' with their tags on them. In ip_divert.c
rev 1.100 you do unconditional m_tag_prepend() of a new tag with
cookie equal 0, and this sends packet back to ipfw rule 0. The packed
is looped forever.
Attached patch solves this problem (also fixing incorrect KASSERT,
which blocked this functionality on invarianted kernels). I've also
added M_ZERO to malloc args, removing explicit zeroing of fields.
If there are no objections, I'll commit it ASAP.
On Tue, Nov 09, 2004 at 11:57:03PM +0300, Gleb Smirnoff wrote:
T> Brian,
T>
T> doing a serie of tests I have found that this commit
T> has introduced regression described below:
T>
T> http://lists.freebsd.org/pipermail/cvs-src/2004-October/032888.html
T>
T> Now I'm working on this, but I'd be glad if you join.
T>
T> On Tue, Nov 09, 2004 at 11:29:03AM +0300, Gleb Smirnoff wrote:
T> T> I've recently noticed a regression between RELENG_5 and CURRENT.
T> T> In CURRENT ng_ksocket is unable to work as divert socket. Since
T> T> you have touched divert code recently I'm asking you. Today I'm
T> T> going to dig deeply there, but probably you can give some ideas
T> T> without investigation.
T> T>
T> T> A test for this functionality looks like this:
T> T>
T> T> /usr/sbin/ngctl -f- <<-SEQ
T> T> mkpeer echo dummy dummy
T> T> name .:dummy echo_div
T> T> mkpeer echo_div: ksocket echo inet/raw/divert
T> T> name echo_div:echo div_sock
T> T> rmhook dummy
T> T> msg div_sock: bind inet/0.0.0.0:
T> T> SEQ
T> T>
T> T> ipfw add 1000 divert all from any to any via fxp0
T> T>
T> T> And packets should flow thru fxp0 in both directions. Do not
T> T> try lo0, there are some problems in there.
T> T>
T> T> You also need this patch (going to commit it soon), if you are
T> T> running INVARIANTS:
T> T>
T> T> --- ip_divert.c 25 Oct 2004 20:02:34 - 1.106
T> T> +++ ip_divert.c 9 Nov 2004 08:27:24 -
T> T> @@ -277,7 +277,7 @@
T> T> struct divert_tag *dt;
T> T> int error = 0;
T> T>
T> T> - KASSERT(m->m_pkthdr.rcvif == NULL, ("rcvif not null"));
T> T> + m->m_pkthdr.rcvif = NULL;
T> T>
T> T> if (control)
T> T> m_freem(control); /* XXX */
T> T>
--
Totus tuus, Glebius.
GLEBIUS-RIPN GLEB-RIPE
Index: ip_divert.c
===
RCS file: /home/ncvs/src/sys/netinet/ip_divert.c,v
retrieving revision 1.107
diff -u -r1.107 ip_divert.c
--- ip_divert.c 8 Nov 2004 14:44:53 - 1.107
+++ ip_divert.c 12 Nov 2004 10:28:11 -
@@ -277,21 +277,22 @@
struct divert_tag *dt;
int error = 0;
- KASSERT(m->m_pkthdr.rcvif == NULL, ("rcvif not null"));
+ m->m_pkthdr.rcvif = NULL;
if (control)
m_freem(control); /* XXX */
- mtag = m_tag_get(PACKET_TAG_DIVERT,
- sizeof(struct divert_tag), M_NOWAIT);
- if (mtag == NULL) {
- error = ENOBUFS;
- goto cantsend;
- }
- dt = (struct divert_tag *)(mtag+1);
- dt->info = 0;
- dt->cookie = 0;
- m_tag_prepend(m, mtag);
+ if ((mtag = m_tag_find(m, PACKET_TAG_DIVERT, NULL)) == NULL) {
+ mtag = m_tag_get(PACKET_TAG_DIVERT, sizeof(struct divert_tag),
+ M_NOWAIT | M_ZERO);
+ if (mtag == NULL) {
+ error = ENOBUFS;
+ goto cantsend;
+ }
+ dt = (struct divert_tag *)(mtag+1);
+ m_tag_prepend(m, mtag);
+ } else
+ dt = (struct divert_tag *)(mtag+1);
/* Loopback avoidance and state recovery */
if (sin) {
___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
OpenBGPd?
Hi, Is anyone working on a port of OpenBGPd, or current version of Quagga (0.97.3)? -- this space was intentionally left blank| Łukasz Bromirski you can insert your favourite quote here | lukasz:bromirski,net _ List sprawdzony skanerem poczty mks_vir ( http://www.mks.com.pl ) ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: in.c autoadding prefix route
On Fri, Nov 12, 2004 at 10:30:14AM +0200, Ruslan Ermilov wrote:
> Hi Max,
>
> On Thu, Nov 11, 2004 at 09:24:05PM +0100, Max Laier wrote:
> > I know I have sent this a couple of times before, but never got anywhere.
> > This
> > time I am set to commit!
> >
> Hey, you did it! ;)
>
> > The attached patch (http://people.freebsd.org/~mlaier/in.c.patch) derived
> > from
> > WIDE via OpenBSD in.c, rev 1.21 improves the handling of automatic prefix
> > routes.
> >
> > Right now you can't have two legs into the same network. If you want to,
> > you
> > must give on of the interfaces a host address only (netmask /32). This way
> > it
> > is not possible to hand over the route if one of the interfaces is
> > "removed" (however this is done in the special case).
> >
> > The patch allows to add more than on IPv4 address with the same prefix. In
> > the
> > case that there is a route already, we leave it alone and add the new
> > address
> > without the IFA_ROUTE flag. When we remove an address later on, that has a
> > route associated, we try to find an alternative address to use for the
> > route
> > and hand it over.
> >
> I cannot give your patch a thorough review at the moment, but I like the
> algorithm, and I don't see how it can hurt anything.
>
> > --- ../dist/sys/netinet/in.cSat Nov 6 21:01:08 2004
> > +++ sys/netinet/in.cMon Nov 8 02:05:17 2004
> > @@ -654,14 +684,7 @@
> > register struct ifnet *ifp;
> > register struct in_ifaddr *ia;
> > {
> > -
> > - if ((ia->ia_flags & IFA_ROUTE) == 0)
> > - return;
> > - if (ifp->if_flags & (IFF_LOOPBACK|IFF_POINTOPOINT))
> > - rtinit(&(ia->ia_ifa), (int)RTM_DELETE, RTF_HOST);
> > - else
> > - rtinit(&(ia->ia_ifa), (int)RTM_DELETE, 0);
> > - ia->ia_flags &= ~IFA_ROUTE;
> > + in_scrubprefix(ia);
> > }
> >
> Looks like "ifp" argument is no longer needed for in_ifscrub(),
> perhaps it should be killed then.
As in_ifscrub() isn't static in in.c I think it must be considered kernel
API and hence I will wait with this cleanup 'till after the MFC. I otherwise
agree that it should be done.
> Also, there are a lot of style bugs (besides those that others
> have already mentioned), the most annoying is comments -- they
> should be written as per style(9) (make them look like the real
> sentences).
I updated the patch at: http://people.freebsd.org/~mlaier/in.c.patch with
new comments. Can you be more specific about the other style(9) violations,
as I don't seem to find them. K&R was choosen as the rest of in.c is K&R
still and I think style(9) requires that the overall style of a file is
maintained.
Thanks.
--
/"\ Best regards, | [EMAIL PROTECTED]
\ / Max Laier | ICQ #67774661
X http://pf4freebsd.love2party.net/ | [EMAIL PROTECTED]
/ \ ASCII Ribbon Campaign | Against HTML Mail and News
___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: OpenBGPd?
On Fri, Nov 12, 2004 at 03:51:37PM +0100, ?ukasz Bromirski wrote: > Is anyone working on a port of OpenBGPd, or current version of Quagga > (0.97.3)? I'm not, but I intend to commit a port of XORP immediately after the next point release. BMS ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Trendnet TU-ET100C
Has anyone seen, or is anyone working on a driver for the TrendNet TU-ET100C ... which is a USB to Ethernet product. Alternatively, is anyone working on a USB to Ethernet driver that's not yet in the tree? Alternatively again, do USB drivers recognise themselves via product numbers and is there a chance that there's a known chipset in there? How might I tell? Dave. -- |David Gilbert, Independent Contractor. | Two things can only be | |Mail: [EMAIL PROTECTED]| equal if and only if they | |http://daveg.ca | are precisely opposite. | =GLO ___ [EMAIL PROTECTED] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "[EMAIL PROTECTED]"
