radeondrm(4) display port output number assignation
Hi tech@, On an ATI Radeon HD 7870 display card with 6 mini-display-ports, I have plugged screens on the first 3 DP (identified DP1 - DP2 - DP3 on the bracket). In Xorg.0.log, the connected output are listed as : [14.513] (II) RADEON(0): Output DisplayPort-0 connected [14.513] (II) RADEON(0): Output DisplayPort-1 disconnected [14.513] (II) RADEON(0): Output DisplayPort-2 connected [14.513] (II) RADEON(0): Output DisplayPort-3 connected [14.513] (II) RADEON(0): Output DisplayPort-4 disconnected [14.513] (II) RADEON(0): Output DisplayPort-5 disconnected It seems that : DP1 = DisplayPort-2 DP2 = DisplayPort-3 DP3 = DisplayPort-0 DP4 = DisplayPort-1 DP5 = DisplayPort-4 DP6 = DisplayPort-5 I have followed http://www.openbsd.org/faq/current.html#20130812 # dmesg OpenBSD 5.4-current (GENERIC.MP) #0: Sun Aug 25 05:23:13 CEST 2013 ale...@test.test.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 212992 (2031MB) avail mem = 2065186816 (1969MB) mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.4 @ 0xf06a0 (74 entries) bios0: vendor American Megatrends Inc. version 1201 date 10/01/2008 bios0: ASUSTeK Computer INC. P5W64 WS Pro acpi0 at bios0: rev 0 acpi0: sleep states S0 S1 S3 S4 S5 acpi0: tables DSDT FACP APIC OEMB HPET MCFG acpi0: wakeup devices P0P1(S4) P0P2(S4) P0P3(S4) P0P5(S4) P0P6(S4) P0P7(S4) P0P8(S4) P0P9(S4) PS2K(S4) PS2M(S4) UAR1(S4) USB2(S4) USB3(S4) USB4(S4) MC97(S4) USB1(S4) [...] acpitimer0 at acpi0: 3579545 Hz, 24 bits acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Core(TM)2 Duo CPU E8400 @ 3.00GHz, 3001.58 MHz cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,NXE,LONG,LAHF,PERF cpu0: 6MB 64b/line 16-way L2 cache cpu0: smt 0, core 0, package 0 cpu0: apic clock running at 333MHz cpu1 at mainbus0: apid 1 (application processor) cpu1: Intel(R) Core(TM)2 Duo CPU E8400 @ 3.00GHz, 3001.17 MHz cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,NXE,LONG,LAHF,PERF cpu1: 6MB 64b/line 16-way L2 cache cpu1: smt 0, core 1, package 0 ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 24 pins acpihpet0 at acpi0: 14318179 Hz acpimcfg0 at acpi0 addr 0xf000, bus 0-63 acpiprt0 at acpi0: bus 0 (PCI0) acpiprt1 at acpi0: bus 5 (P0P1) acpiprt2 at acpi0: bus -1 (P0P2) acpiprt3 at acpi0: bus -1 (P2PA) acpiprt4 at acpi0: bus 1 (P0P3) acpiprt5 at acpi0: bus 3 (P0P8) acpiprt6 at acpi0: bus 2 (P0P9) acpicpu0 at acpi0: PSS acpicpu1 at acpi0: PSS aibs0 at acpi0: RTMP RVLT RFAN acpibtn0 at acpi0: SLPB acpibtn1 at acpi0: PWRB cpu0: Enhanced SpeedStep 3001 MHz: speeds: 2997, 1998 MHz pci0 at mainbus0 bus 0 pchb0 at pci0 dev 0 function 0 Intel 82975X Host rev 0xc0 ppb0 at pci0 dev 1 function 0 Intel 82975X PCIE rev 0xc0: msi pci1 at ppb0 bus 5 radeondrm0 at pci1 dev 0 function 0 ATI Radeon HD 7870 rev 0x00: apic 2 int 16 drm0 at radeondrm0 azalia0 at pci1 dev 0 function 1 vendor ATI, unknown product 0xaab0 rev 0x00: msi azalia0: no supported codecs azalia1 at pci0 dev 27 function 0 Intel 82801GB HD Audio rev 0x01: msi azalia1: codecs: Analog Devices AD1988B audio0 at azalia1 ppb1 at pci0 dev 28 function 0 Intel 82801GB PCIE rev 0x01 pci2 at ppb1 bus 4 ppb2 at pci0 dev 28 function 4 Intel 82801G PCIE rev 0x01 pci3 at ppb2 bus 3 mskc0 at pci3 dev 0 function 0 Marvell Yukon 88E8052 rev 0x21, Yukon-2 EC rev. A3 (0x2): apic 2 int 16 msk0 at mskc0 port A: address 00:18:f3:6c:44:da eephy0 at msk0 phy 0: 88E Gigabit PHY, rev. 2 ppb3 at pci0 dev 28 function 5 Intel 82801G PCIE rev 0x01 pci4 at ppb3 bus 2 Marvell 88SE6145 SATA rev 0xa1 at pci4 dev 0 function 0 not configured uhci0 at pci0 dev 29 function 0 Intel 82801GB USB rev 0x01: apic 2 int 20 uhci1 at pci0 dev 29 function 1 Intel 82801GB USB rev 0x01: apic 2 int 17 uhci2 at pci0 dev 29 function 2 Intel 82801GB USB rev 0x01: apic 2 int 18 uhci3 at pci0 dev 29 function 3 Intel 82801GB USB rev 0x01: apic 2 int 19 ehci0 at pci0 dev 29 function 7 Intel 82801GB USB rev 0x01: apic 2 int 20 usb0 at ehci0: USB revision 2.0 uhub0 at usb0 Intel EHCI root hub rev 2.00/1.00 addr 1 ppb4 at pci0 dev 30 function 0 Intel 82801BA Hub-to-PCI rev 0xe1 pci5 at ppb4 bus 1 TI TSB43AB22 FireWire rev 0x00 at pci5 dev 3 function 0 not configured skc0 at pci5 dev 5 function 0 Marvell Yukon 88E8001/8003/8010 rev 0x14, Yukon Lite (0x9): apic 2 int 21 sk0 at skc0 port A: address 00:18:f3:6c:44:db eephy1 at sk0 phy 0: 88E1011 Gigabit PHY, rev. 5 pcib0 at pci0 dev 31 function 0 Intel 82801GB LPC rev 0x01 pciide0 at pci0 dev 31 function 1 Intel 82801GB IDE rev 0x01: DMA, channel 0 configured to compatibility, channel 1 configured to compatibility pciide0: channel 0 disabled (no drives) pciide0: channel 1 disabled (no
Re: openbsd ioctl fix (in6.c)
On 22/08/13(Thu) 23:31, Claudio Jeker wrote: On Wed, Aug 21, 2013 at 09:59:56AM -0700, Loganaden Velvindron wrote: I'm not sure if applies to OpenBSD as well, but NetBSD also disallowed SIOCSIFDSTADDR for ioctl. [...] Index: sys/netinet6/in6.c === RCS file: /cvs/src/sys/netinet6/in6.c,v retrieving revision 1.117 diff -u -p -r1.117 in6.c --- sys/netinet6/in6.c 13 Aug 2013 05:52:25 - 1.117 +++ sys/netinet6/in6.c 21 Aug 2013 15:50:00 - @@ -429,8 +429,9 @@ in6_control(struct socket *so, u_long cm sa6 = ifr-ifr_addr; break; case SIOCSIFADDR: + case SIOCSIFDSTADDR: /* -* Do not pass this ioctl to driver handler since it is not +* Do not pass those ioctl to driver handler since they are not * properly setup. Instead just error out. */ return (EOPNOTSUPP); Are any of our driver ioctl handlers handling SIOCSIFDSTADDR? I don't think so but I think this could be just extra safety and should therefor be commited. Only tun(4) seems to have some code for it, but I don't think it is correct. And if it is, we should probably use SIOCGIFDSTADDR_IN6 anyway. Maybe a small cleanup can be done? So I'm also in favor of this supplementary safety check, and I would even add SIOCSIFBRDADDR to this list. M.
Re: openbsd ioctl fix (in6.c)
On Tue, Aug 27, 2013 at 10:37:30AM +0200, Martin Pieuchot wrote: On 22/08/13(Thu) 23:31, Claudio Jeker wrote: On Wed, Aug 21, 2013 at 09:59:56AM -0700, Loganaden Velvindron wrote: I'm not sure if applies to OpenBSD as well, but NetBSD also disallowed SIOCSIFDSTADDR for ioctl. [...] Index: sys/netinet6/in6.c === RCS file: /cvs/src/sys/netinet6/in6.c,v retrieving revision 1.117 diff -u -p -r1.117 in6.c --- sys/netinet6/in6.c13 Aug 2013 05:52:25 - 1.117 +++ sys/netinet6/in6.c21 Aug 2013 15:50:00 - @@ -429,8 +429,9 @@ in6_control(struct socket *so, u_long cm sa6 = ifr-ifr_addr; break; case SIOCSIFADDR: + case SIOCSIFDSTADDR: /* - * Do not pass this ioctl to driver handler since it is not + * Do not pass those ioctl to driver handler since they are not * properly setup. Instead just error out. */ return (EOPNOTSUPP); Are any of our driver ioctl handlers handling SIOCSIFDSTADDR? I don't think so but I think this could be just extra safety and should therefor be commited. Only tun(4) seems to have some code for it, but I don't think it is correct. And if it is, we should probably use SIOCGIFDSTADDR_IN6 anyway. Maybe a small cleanup can be done? I haven't looked at tun(4) code yet, but I can give it a try :-) So I'm also in favor of this supplementary safety check, and I would even add SIOCSIFBRDADDR to this list. I'm updating the diff. M.
Re: defer routing table updates on link state changes
On 26/08/13(Mon) 13:36, Mike Belopuhov wrote:
hi,
in order to make our life a bit easier and prevent rogue
accesses to the routing table from the hardware interrupt
context violating all kinds of spl assumptions we would
like if_link_state_change that is called by network device
drivers in their interrupt service routines to defer its
work to the process context or thereabouts.
i did some testing here, but wouldn't mind if someone
tries this diff in gre/vlan/ospf/anything-weird setups.
making sure that hot-plugging/unplugging usb interfaces
doesn't produce any undesirable effects would be superb
as well.
I just tested vlan and usb interfaces, it works just fine.
please note that a token (an interface index) is passed
to the workq in order to make sure that if the interface
would be gone by the time syswq goes around to run the
task it would just fall through.
I think that's the right approach but the current code generating
interfaces indexes is too clever from my point of view, it tries
to reuse the last index if possible. This could lead to some
funny races if we detach and reattach an interface before the
task get scheduled.
So I'm ok with your diff if we avoid to reuse the last index too
soon. Diff below does that.
Index: net/if.c
===
RCS file: /home/ncvs/src/sys/net/if.c,v
retrieving revision 1.262
diff -u -p -r1.262 if.c
--- net/if.c20 Aug 2013 09:14:22 - 1.262
+++ net/if.c27 Aug 2013 10:22:44 -
@@ -204,31 +204,37 @@ if_attachsetup(struct ifnet *ifp)
{
int wrapped = 0;
- if (ifindex2ifnet == 0)
+ /*
+* Always increment the index to avoid races.
+*/
+ if_index++;
+
+ /*
+* If we hit USHRT_MAX, we skip back to 1 since there are a
+* number of places where the value of ifp-if_index or
+* if_index itself is compared to or stored in an unsigned
+* short. By jumping back, we won't botch those assignments
+* or comparisons.
+*/
+ if (if_index == USHRT_MAX) {
if_index = 1;
- else {
- while (if_index if_indexlim
- ifindex2ifnet[if_index] != NULL) {
- if_index++;
+ wrapped++;
+ }
+
+ while (if_index if_indexlim ifindex2ifnet[if_index] != NULL) {
+ if_index++;
+
+ if (if_index == USHRT_MAX) {
/*
-* If we hit USHRT_MAX, we skip back to 1 since
-* there are a number of places where the value
-* of ifp-if_index or if_index itself is compared
-* to or stored in an unsigned short. By
-* jumping back, we won't botch those assignments
-* or comparisons.
+* If we have to jump back to 1 twice without
+* finding an empty slot then there are too many
+* interfaces.
*/
- if (if_index == USHRT_MAX) {
- if_index = 1;
- /*
-* However, if we have to jump back to 1
-* *twice* without finding an empty
-* slot in ifindex2ifnet[], then there
-* there are too many (65535) interfaces.
-*/
- if (wrapped++)
- panic(too many interfaces);
- }
+ if (wrapped)
+ panic(too many interfaces);
+
+ if_index = 1;
+ wrapped++;
}
}
ifp-if_index = if_index;
Re: defer routing table updates on link state changes
On 27 August 2013 13:39, Martin Pieuchot mpieuc...@nolizard.org wrote: I think that's the right approach but the current code generating interfaces indexes is too clever from my point of view, it tries to reuse the last index if possible. This could lead to some funny races if we detach and reattach an interface before the task get scheduled. So I'm ok with your diff if we avoid to reuse the last index too soon. Diff below does that. i'm ok with this change.
Split rtinit()
So I started to play with the routine table and I'm slowly trying to
unify the various code paths to add and delete route entries. The
diff below is a first step, it splits rtinit() into rt_add() and
rt_delete() there should be no functional change.
ok?
Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.262
diff -u -p -r1.262 if.c
--- net/if.c20 Aug 2013 09:14:22 - 1.262
+++ net/if.c27 Aug 2013 13:33:03 -
@@ -353,7 +353,7 @@ if_free_sadl(struct ifnet *ifp)
return;
s = splnet();
- rtinit(ifa, RTM_DELETE, 0);
+ rt_del(ifa, 0);
#if 0
ifa_del(ifp, ifa);
ifp-if_lladdr = NULL;
Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.144
diff -u -p -r1.144 route.c
--- net/route.c 28 Mar 2013 23:10:05 - 1.144
+++ net/route.c 27 Aug 2013 13:33:03 -
@@ -1069,11 +1069,10 @@ rt_maskedcopy(struct sockaddr *src, stru
* for an interface.
*/
int
-rtinit(struct ifaddr *ifa, int cmd, int flags)
+rt_add(struct ifaddr *ifa, int flags)
{
struct rtentry *rt;
- struct sockaddr *dst, *deldst;
- struct mbuf *m = NULL;
+ struct sockaddr *dst;
struct rtentry *nrt = NULL;
int error;
struct rt_addrinfo info;
@@ -1081,35 +1080,11 @@ rtinit(struct ifaddr *ifa, int cmd, int
u_short rtableid = ifa-ifa_ifp-if_rdomain;
dst = flags RTF_HOST ? ifa-ifa_dstaddr : ifa-ifa_addr;
- if (cmd == RTM_DELETE) {
- if ((flags RTF_HOST) == 0 ifa-ifa_netmask) {
- m = m_get(M_DONTWAIT, MT_SONAME);
- if (m == NULL)
- return (ENOBUFS);
- deldst = mtod(m, struct sockaddr *);
- rt_maskedcopy(dst, deldst, ifa-ifa_netmask);
- dst = deldst;
- }
- if ((rt = rtalloc1(dst, 0, rtableid)) != NULL) {
- rt-rt_refcnt--;
- /* try to find the right route */
- while (rt rt-rt_ifa != ifa)
- rt = (struct rtentry *)
- ((struct radix_node *)rt)-rn_dupedkey;
- if (!rt) {
- if (m != NULL)
- (void) m_free(m);
- return (flags RTF_HOST ? EHOSTUNREACH
- : ENETUNREACH);
- }
- }
- }
bzero(info, sizeof(info));
info.rti_ifa = ifa;
info.rti_flags = flags | ifa-ifa_flags;
info.rti_info[RTAX_DST] = dst;
- if (cmd == RTM_ADD)
- info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
+ info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
info.rti_info[RTAX_LABEL] =
rtlabel_id2sa(ifa-ifa_ifp-if_rtlabelid, sa_rl);
@@ -1120,22 +1095,11 @@ rtinit(struct ifaddr *ifa, int cmd, int
* change it to meet bsdi4 behavior.
*/
info.rti_info[RTAX_NETMASK] = ifa-ifa_netmask;
- error = rtrequest1(cmd, info, RTP_CONNECTED, nrt, rtableid);
- if (cmd == RTM_DELETE) {
- if (error == 0 (rt = nrt) != NULL) {
- rt_newaddrmsg(cmd, ifa, error, nrt);
- if (rt-rt_refcnt = 0) {
- rt-rt_refcnt++;
- rtfree(rt);
- }
- }
- if (m != NULL)
- (void) m_free(m);
- }
- if (cmd == RTM_ADD error == 0 (rt = nrt) != NULL) {
+ error = rtrequest1(RTM_ADD, info, RTP_CONNECTED, nrt, rtableid);
+ if (error == 0 (rt = nrt) != NULL) {
rt-rt_refcnt--;
if (rt-rt_ifa != ifa) {
- printf(rtinit: wrong ifa (%p) was (%p)\n,
+ printf(%s: wrong ifa (%p) was (%p)\n, __func__,
ifa, rt-rt_ifa);
if (rt-rt_ifa-ifa_rtrequest)
rt-rt_ifa-ifa_rtrequest(RTM_DELETE, rt, NULL);
@@ -1146,8 +1110,68 @@ rtinit(struct ifaddr *ifa, int cmd, int
if (ifa-ifa_rtrequest)
ifa-ifa_rtrequest(RTM_ADD, rt, NULL);
}
- rt_newaddrmsg(cmd, ifa, error, nrt);
+ rt_newaddrmsg(RTM_ADD, ifa, error, nrt);
+ }
+
+ return (error);
+}
+
+int
+rt_del(struct ifaddr *ifa, int flags)
+{
+ struct rtentry *rt;
+ struct sockaddr *dst, *deldst;
+ struct mbuf *m = NULL;
+ struct rtentry *nrt = NULL;
+
Remove unused argument from *rtrequest()
In order to define a proper API for our routine table, I'd like to turn
the struct rt_addrinfo into a private type (ie: only used in route.c
and rtsock.c).
This type is used by a lost of code in our network stack to add or delete
a route but also in the various *rtrequest() functions. However in
these functions the argument is never used! So the diff below kills it.
ok?
Index: net/if.c
===
RCS file: /home/ncvs/src/sys/net/if.c,v
retrieving revision 1.262
diff -u -p -r1.262 if.c
--- net/if.c20 Aug 2013 09:14:22 - 1.262
+++ net/if.c27 Aug 2013 13:50:50 -
@@ -985,7 +985,7 @@ ifaof_ifpforaddr(struct sockaddr *addr,
* This should be moved to /sys/net/link.c eventually.
*/
void
-link_rtrequest(int cmd, struct rtentry *rt, struct rt_addrinfo *info)
+link_rtrequest(int cmd, struct rtentry *rt)
{
struct ifaddr *ifa;
struct sockaddr *dst;
@@ -999,7 +999,7 @@ link_rtrequest(int cmd, struct rtentry *
ifafree(rt-rt_ifa);
rt-rt_ifa = ifa;
if (ifa-ifa_rtrequest ifa-ifa_rtrequest != link_rtrequest)
- ifa-ifa_rtrequest(cmd, rt, info);
+ ifa-ifa_rtrequest(cmd, rt);
}
}
Index: net/if.h
===
RCS file: /home/ncvs/src/sys/net/if.h,v
retrieving revision 1.144
diff -u -p -r1.144 if.h
--- net/if.h20 Jun 2013 12:03:40 - 1.144
+++ net/if.h27 Aug 2013 13:50:50 -
@@ -494,7 +494,7 @@ struct ifaddr {
struct ifnet *ifa_ifp; /* back-pointer to interface */
TAILQ_ENTRY(ifaddr) ifa_list; /* list of addresses for interface */
/* check or clean routes (+ or -)'d */
- void(*ifa_rtrequest)(int, struct rtentry *, struct rt_addrinfo *);
+ void(*ifa_rtrequest)(int, struct rtentry *);
u_int ifa_flags; /* mostly rt_flags for cloning */
u_int ifa_refcnt; /* count of references */
int ifa_metric; /* cost of going out this interface */
@@ -842,7 +842,7 @@ struct ifaddr *ifa_ifwithroute(int, stru
struct sockaddr *, u_int);
struct ifaddr *ifaof_ifpforaddr(struct sockaddr *, struct ifnet *);
void ifafree(struct ifaddr *);
-void link_rtrequest(int, struct rtentry *, struct rt_addrinfo *);
+void link_rtrequest(int, struct rtentry *);
void if_clone_attach(struct if_clone *);
void if_clone_detach(struct if_clone *);
@@ -858,7 +858,7 @@ int loioctl(struct ifnet *, u_long, cadd
void loopattach(int);
intlooutput(struct ifnet *,
struct mbuf *, struct sockaddr *, struct rtentry *);
-void lortrequest(int, struct rtentry *, struct rt_addrinfo *);
+void lortrequest(int, struct rtentry *);
void ifa_add(struct ifnet *, struct ifaddr *);
void ifa_del(struct ifnet *, struct ifaddr *);
void ifa_update_broadaddr(struct ifnet *, struct ifaddr *,
Index: net/if_loop.c
===
RCS file: /home/ncvs/src/sys/net/if_loop.c,v
retrieving revision 1.49
diff -u -p -r1.49 if_loop.c
--- net/if_loop.c 28 Mar 2013 16:55:27 - 1.49
+++ net/if_loop.c 27 Aug 2013 13:50:50 -
@@ -373,7 +373,7 @@ lo_altqstart(struct ifnet *ifp)
/* ARGSUSED */
void
-lortrequest(int cmd, struct rtentry *rt, struct rt_addrinfo *info)
+lortrequest(int cmd, struct rtentry *rt)
{
if (rt)
rt-rt_rmx.rmx_mtu = LOMTU;
Index: net/route.c
===
RCS file: /home/ncvs/src/sys/net/route.c,v
retrieving revision 1.144
diff -u -p -r1.144 route.c
--- net/route.c 28 Mar 2013 23:10:05 - 1.144
+++ net/route.c 27 Aug 2013 13:50:50 -
@@ -781,7 +781,7 @@ rtrequest1(int req, struct rt_addrinfo *
rt-rt_flags = ~RTF_UP;
if ((ifa = rt-rt_ifa) ifa-ifa_rtrequest)
- ifa-ifa_rtrequest(RTM_DELETE, rt, info);
+ ifa-ifa_rtrequest(RTM_DELETE, rt);
rttrash++;
if (ret_nrt)
@@ -925,14 +925,13 @@ rtrequest1(int req, struct rt_addrinfo *
was (%p)\n, ifa, (*ret_nrt)-rt_ifa);
if ((*ret_nrt)-rt_ifa-ifa_rtrequest)
(*ret_nrt)-rt_ifa-ifa_rtrequest(
- RTM_DELETE, *ret_nrt, NULL);
+ RTM_DELETE, *ret_nrt);
ifafree((*ret_nrt)-rt_ifa);
(*ret_nrt)-rt_ifa = ifa;
(*ret_nrt)-rt_ifp = ifa-ifa_ifp;
ifa-ifa_refcnt++;
if (ifa-ifa_rtrequest)
-
Re: Remove unused argument from *rtrequest()
On Tue, Aug 27, 2013 at 03:58:51PM +0200, Martin Pieuchot wrote:
In order to define a proper API for our routine table, I'd like to turn
the struct rt_addrinfo into a private type (ie: only used in route.c
and rtsock.c).
This type is used by a lost of code in our network stack to add or delete
a route but also in the various *rtrequest() functions. However in
these functions the argument is never used! So the diff below kills it.
ok?
Less is more. ok krw@.
Ken
Index: net/if.c
===
RCS file: /home/ncvs/src/sys/net/if.c,v
retrieving revision 1.262
diff -u -p -r1.262 if.c
--- net/if.c 20 Aug 2013 09:14:22 - 1.262
+++ net/if.c 27 Aug 2013 13:50:50 -
@@ -985,7 +985,7 @@ ifaof_ifpforaddr(struct sockaddr *addr,
* This should be moved to /sys/net/link.c eventually.
*/
void
-link_rtrequest(int cmd, struct rtentry *rt, struct rt_addrinfo *info)
+link_rtrequest(int cmd, struct rtentry *rt)
{
struct ifaddr *ifa;
struct sockaddr *dst;
@@ -999,7 +999,7 @@ link_rtrequest(int cmd, struct rtentry *
ifafree(rt-rt_ifa);
rt-rt_ifa = ifa;
if (ifa-ifa_rtrequest ifa-ifa_rtrequest != link_rtrequest)
- ifa-ifa_rtrequest(cmd, rt, info);
+ ifa-ifa_rtrequest(cmd, rt);
}
}
Index: net/if.h
===
RCS file: /home/ncvs/src/sys/net/if.h,v
retrieving revision 1.144
diff -u -p -r1.144 if.h
--- net/if.h 20 Jun 2013 12:03:40 - 1.144
+++ net/if.h 27 Aug 2013 13:50:50 -
@@ -494,7 +494,7 @@ struct ifaddr {
struct ifnet *ifa_ifp; /* back-pointer to interface */
TAILQ_ENTRY(ifaddr) ifa_list; /* list of addresses for interface */
/* check or clean routes (+ or -)'d */
- void(*ifa_rtrequest)(int, struct rtentry *, struct rt_addrinfo *);
+ void(*ifa_rtrequest)(int, struct rtentry *);
u_int ifa_flags; /* mostly rt_flags for cloning */
u_int ifa_refcnt; /* count of references */
int ifa_metric; /* cost of going out this interface */
@@ -842,7 +842,7 @@ structifaddr *ifa_ifwithroute(int, stru
struct sockaddr *, u_int);
struct ifaddr *ifaof_ifpforaddr(struct sockaddr *, struct ifnet *);
void ifafree(struct ifaddr *);
-void link_rtrequest(int, struct rtentry *, struct rt_addrinfo *);
+void link_rtrequest(int, struct rtentry *);
void if_clone_attach(struct if_clone *);
void if_clone_detach(struct if_clone *);
@@ -858,7 +858,7 @@ int loioctl(struct ifnet *, u_long, cadd
void loopattach(int);
int looutput(struct ifnet *,
struct mbuf *, struct sockaddr *, struct rtentry *);
-void lortrequest(int, struct rtentry *, struct rt_addrinfo *);
+void lortrequest(int, struct rtentry *);
void ifa_add(struct ifnet *, struct ifaddr *);
void ifa_del(struct ifnet *, struct ifaddr *);
void ifa_update_broadaddr(struct ifnet *, struct ifaddr *,
Index: net/if_loop.c
===
RCS file: /home/ncvs/src/sys/net/if_loop.c,v
retrieving revision 1.49
diff -u -p -r1.49 if_loop.c
--- net/if_loop.c 28 Mar 2013 16:55:27 - 1.49
+++ net/if_loop.c 27 Aug 2013 13:50:50 -
@@ -373,7 +373,7 @@ lo_altqstart(struct ifnet *ifp)
/* ARGSUSED */
void
-lortrequest(int cmd, struct rtentry *rt, struct rt_addrinfo *info)
+lortrequest(int cmd, struct rtentry *rt)
{
if (rt)
rt-rt_rmx.rmx_mtu = LOMTU;
Index: net/route.c
===
RCS file: /home/ncvs/src/sys/net/route.c,v
retrieving revision 1.144
diff -u -p -r1.144 route.c
--- net/route.c 28 Mar 2013 23:10:05 - 1.144
+++ net/route.c 27 Aug 2013 13:50:50 -
@@ -781,7 +781,7 @@ rtrequest1(int req, struct rt_addrinfo *
rt-rt_flags = ~RTF_UP;
if ((ifa = rt-rt_ifa) ifa-ifa_rtrequest)
- ifa-ifa_rtrequest(RTM_DELETE, rt, info);
+ ifa-ifa_rtrequest(RTM_DELETE, rt);
rttrash++;
if (ret_nrt)
@@ -925,14 +925,13 @@ rtrequest1(int req, struct rt_addrinfo *
was (%p)\n, ifa, (*ret_nrt)-rt_ifa);
if ((*ret_nrt)-rt_ifa-ifa_rtrequest)
(*ret_nrt)-rt_ifa-ifa_rtrequest(
- RTM_DELETE, *ret_nrt, NULL);
+ RTM_DELETE, *ret_nrt);
ifafree((*ret_nrt)-rt_ifa);
(*ret_nrt)-rt_ifa = ifa;
(*ret_nrt)-rt_ifp = ifa-ifa_ifp;
Re: defer routing table updates on link state changes
On Tue, Aug 27, 2013 at 01:54:34PM +0200, Mike Belopuhov wrote: On 27 August 2013 13:39, Martin Pieuchot mpieuc...@nolizard.org wrote: I think that's the right approach but the current code generating interfaces indexes is too clever from my point of view, it tries to reuse the last index if possible. This could lead to some funny races if we detach and reattach an interface before the task get scheduled. So I'm ok with your diff if we avoid to reuse the last index too soon. Diff below does that. i'm ok with this change. I like that tweak too. Makes me feel warm and safe. Ken
Re: Split rtinit()
On Tue, Aug 27, 2013 at 03:38:49PM +0200, Martin Pieuchot wrote:
So I started to play with the routine table and I'm slowly trying to
unify the various code paths to add and delete route entries. The
diff below is a first step, it splits rtinit() into rt_add() and
rt_delete() there should be no functional change.
ok?
That makes s much more sense. :-).
ok krw@ (note: not a routing table guru!)
Ken
Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.262
diff -u -p -r1.262 if.c
--- net/if.c 20 Aug 2013 09:14:22 - 1.262
+++ net/if.c 27 Aug 2013 13:33:03 -
@@ -353,7 +353,7 @@ if_free_sadl(struct ifnet *ifp)
return;
s = splnet();
- rtinit(ifa, RTM_DELETE, 0);
+ rt_del(ifa, 0);
#if 0
ifa_del(ifp, ifa);
ifp-if_lladdr = NULL;
Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.144
diff -u -p -r1.144 route.c
--- net/route.c 28 Mar 2013 23:10:05 - 1.144
+++ net/route.c 27 Aug 2013 13:33:03 -
@@ -1069,11 +1069,10 @@ rt_maskedcopy(struct sockaddr *src, stru
* for an interface.
*/
int
-rtinit(struct ifaddr *ifa, int cmd, int flags)
+rt_add(struct ifaddr *ifa, int flags)
{
struct rtentry *rt;
- struct sockaddr *dst, *deldst;
- struct mbuf *m = NULL;
+ struct sockaddr *dst;
struct rtentry *nrt = NULL;
int error;
struct rt_addrinfo info;
@@ -1081,35 +1080,11 @@ rtinit(struct ifaddr *ifa, int cmd, int
u_short rtableid = ifa-ifa_ifp-if_rdomain;
dst = flags RTF_HOST ? ifa-ifa_dstaddr : ifa-ifa_addr;
- if (cmd == RTM_DELETE) {
- if ((flags RTF_HOST) == 0 ifa-ifa_netmask) {
- m = m_get(M_DONTWAIT, MT_SONAME);
- if (m == NULL)
- return (ENOBUFS);
- deldst = mtod(m, struct sockaddr *);
- rt_maskedcopy(dst, deldst, ifa-ifa_netmask);
- dst = deldst;
- }
- if ((rt = rtalloc1(dst, 0, rtableid)) != NULL) {
- rt-rt_refcnt--;
- /* try to find the right route */
- while (rt rt-rt_ifa != ifa)
- rt = (struct rtentry *)
- ((struct radix_node *)rt)-rn_dupedkey;
- if (!rt) {
- if (m != NULL)
- (void) m_free(m);
- return (flags RTF_HOST ? EHOSTUNREACH
- : ENETUNREACH);
- }
- }
- }
bzero(info, sizeof(info));
info.rti_ifa = ifa;
info.rti_flags = flags | ifa-ifa_flags;
info.rti_info[RTAX_DST] = dst;
- if (cmd == RTM_ADD)
- info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
+ info.rti_info[RTAX_GATEWAY] = ifa-ifa_addr;
info.rti_info[RTAX_LABEL] =
rtlabel_id2sa(ifa-ifa_ifp-if_rtlabelid, sa_rl);
@@ -1120,22 +1095,11 @@ rtinit(struct ifaddr *ifa, int cmd, int
* change it to meet bsdi4 behavior.
*/
info.rti_info[RTAX_NETMASK] = ifa-ifa_netmask;
- error = rtrequest1(cmd, info, RTP_CONNECTED, nrt, rtableid);
- if (cmd == RTM_DELETE) {
- if (error == 0 (rt = nrt) != NULL) {
- rt_newaddrmsg(cmd, ifa, error, nrt);
- if (rt-rt_refcnt = 0) {
- rt-rt_refcnt++;
- rtfree(rt);
- }
- }
- if (m != NULL)
- (void) m_free(m);
- }
- if (cmd == RTM_ADD error == 0 (rt = nrt) != NULL) {
+ error = rtrequest1(RTM_ADD, info, RTP_CONNECTED, nrt, rtableid);
+ if (error == 0 (rt = nrt) != NULL) {
rt-rt_refcnt--;
if (rt-rt_ifa != ifa) {
- printf(rtinit: wrong ifa (%p) was (%p)\n,
+ printf(%s: wrong ifa (%p) was (%p)\n, __func__,
ifa, rt-rt_ifa);
if (rt-rt_ifa-ifa_rtrequest)
rt-rt_ifa-ifa_rtrequest(RTM_DELETE, rt, NULL);
@@ -1146,8 +1110,68 @@ rtinit(struct ifaddr *ifa, int cmd, int
if (ifa-ifa_rtrequest)
ifa-ifa_rtrequest(RTM_ADD, rt, NULL);
}
- rt_newaddrmsg(cmd, ifa, error, nrt);
+ rt_newaddrmsg(RTM_ADD, ifa, error, nrt);
+ }
+
+ return (error);
+}
+
+int
+rt_del(struct ifaddr *ifa, int flags)
+{
+ struct rtentry *rt;
+
Re: Remove unused argument from *rtrequest()
On 27 August 2013 15:58, Martin Pieuchot mpieuc...@nolizard.org wrote: In order to define a proper API for our routine table, I'd like to turn the struct rt_addrinfo into a private type (ie: only used in route.c and rtsock.c). This type is used by a lost of code in our network stack to add or delete a route but also in the various *rtrequest() functions. However in these functions the argument is never used! So the diff below kills it. ok? ok
