Fix mfi to set drive state (offline, rebuild..) properly

2014-11-28 Thread YASUOKA Masahiko
I'm using NEC Express5800 with its optional RAID contoller, but I
could not set the drive state into rebuild or offline by bioctl(8).

Tsubai found mfi(4) needs to be fixed to operate the firmware
interface properly.

I'd like to commit below diff from him.

test? ok?

Fix mfi ioctl to set drive state properly.

diff from Tsubai Masanari

Index: sys/dev/ic/mfi.c
===
RCS file: /disk/cvs/openbsd/src/sys/dev/ic/mfi.c,v
retrieving revision 1.157
diff -u -p -r1.157 mfi.c
--- sys/dev/ic/mfi.c14 Sep 2014 14:17:24 -  1.157
+++ sys/dev/ic/mfi.c29 Nov 2014 02:01:18 -
@@ -1900,6 +1900,7 @@ int
 mfi_ioctl_setstate(struct mfi_softc *sc, struct bioc_setstate *bs)
 {
struct mfi_pd_list  *pd;
+   struct mfi_pd_details   *info;
int i, found, rv = EINVAL;
uint8_t mbox[MFI_MBOX_SIZE];
 
@@ -1907,6 +1908,7 @@ mfi_ioctl_setstate(struct mfi_softc *sc,
bs->bs_status);
 
pd = malloc(sizeof(*pd), M_DEVBUF, M_WAITOK);
+   info = malloc(sizeof *info, M_DEVBUF, M_WAITOK);
 
if (mfi_mgmt(sc, MR_DCMD_PD_GET_LIST, MFI_DATA_IN,
sizeof(*pd), pd, NULL))
@@ -1925,23 +1927,30 @@ mfi_ioctl_setstate(struct mfi_softc *sc,
memset(mbox, 0, sizeof mbox);
 
*((uint16_t *)&mbox) = pd->mpl_address[i].mpa_pd_id;
+   if (mfi_mgmt(sc, MR_DCMD_PD_GET_INFO, MFI_DATA_IN,
+   sizeof *info, info, mbox))
+   goto done;
+
+   *((uint16_t *)&mbox[0]) = pd->mpl_address[i].mpa_pd_id;
+   *((uint16_t *)&mbox[2]) = info->mpd_pd.mfp_seq;
 
switch (bs->bs_status) {
case BIOC_SSONLINE:
-   mbox[2] = MFI_PD_ONLINE;
+   mbox[4] = MFI_PD_ONLINE;
break;
 
case BIOC_SSOFFLINE:
-   mbox[2] = MFI_PD_OFFLINE;
+   mbox[4] = MFI_PD_OFFLINE;
break;
 
case BIOC_SSHOTSPARE:
-   mbox[2] = MFI_PD_HOTSPARE;
+   mbox[4] = MFI_PD_HOTSPARE;
break;
-/*
+
case BIOC_SSREBUILD:
+   mbox[4] = MFI_PD_REBUILD;
break;
-*/
+
default:
DNPRINTF(MFI_D_IOCTL, "%s: mfi_ioctl_setstate invalid "
"opcode %x\n", DEVNAME(sc), bs->bs_status);
@@ -1955,6 +1964,7 @@ mfi_ioctl_setstate(struct mfi_softc *sc,
rv = 0;
 done:
free(pd, M_DEVBUF, 0);
+   free(info, M_DEVBUF, 0);
return (rv);
 }
 



Disable NOINET6 for carp parent interfaces automatically

2014-11-28 Thread Florian Riehm
Hi,

in OpenBSD 5.6 NOINET6 gets disabled on a carp interface after configuring
an ipv6 address. Additionally you have to disable NOINET6 at the physical
interface or ip6_output() will fail because carp_send_ad() could not determine
an ipv6 source address via ifaof_ifpforaddr().

The attached patch enables IPv6 also at the carp parent interface if enabling
it at a carp interface.

Are there other interfaces which have the same problem?

Regards

Florian

Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.304
diff -u -p -r1.304 if.c
--- net/if.c23 Nov 2014 07:39:02 -  1.304
+++ net/if.c28 Nov 2014 19:39:58 -
@@ -1323,6 +1323,18 @@ ifioctl(struct socket *so, u_long cmd, c
if (ifp->if_xflags & IFXF_NOINET6 &&
!(ifr->ifr_flags & IFXF_NOINET6)) {
ifp->if_xflags &= ~IFXF_NOINET6;
+#if NCARP > 0
+   /* also enable ipv6 on carp parent interface */
+   if (ifp->if_type == IFT_CARP && ifp->if_carpdev &&
+   ifp->if_carpdev->if_xflags & IFXF_NOINET6) {
+   ifp->if_carpdev->if_xflags &= ~IFXF_NOINET6;
+   if (ifp->if_carpdev->if_flags & IFF_UP) {
+   s = splnet();
+   in6_if_up(ifp->if_carpdev);
+   splx(s);
+   }
+   }
+#endif
if (ifp->if_flags & IFF_UP) {
/* configure link-local address */
s = splnet();



Re: [Patch]rcs: use rcsnum_cmp

2014-11-28 Thread Fritjof Bornebusch
On Fri, Nov 28, 2014 at 04:14:50PM +0100, Otto Moerbeek wrote:
> On Sun, Nov 23, 2014 at 05:19:16PM +0100, Fritjof Bornebusch wrote:
> 
> > Hi tech,
> > 
> > like the XXX comment says, rcsnum_cmp() can be used instead of a *for* loop.
> > The following shows the original behavior:
> > 
> > $ co -r1.2 foo.txt,v
> > foo.txt,v  -->  foo.txt
> > revision 1.2
> > done
> > 
> > $ co -r1.1 foo.txt,v 
> > foo.txt,v  -->  foo.txt
> > revision 1.1
> > done
> > 
> > $ co foo.txt,v
> > foo.txt,v  -->  foo.txt
> > revision 1.2
> > done
> > 
> > The following shows the changed behavior:
> > 
> > $ co -r1.2 foo.txt,v 
> > foo.txt,v  -->  foo.txt
> > revision 1.2
> > done
> > 
> > $ co -r1.1 foo.txt,v
> > foo.txt,v  -->  foo.txt
> > revision 1.1
> > done
> > 
> > $ co foo.txt,v
> > foo.txt,v  -->  foo.txt
> > revision 1.2
> > done
> > 
> > Could some verify that I didn't miss a test case.
> 
> Hmm, your examples show no diffference. I think the intention is to
> not change the behaviour, right? 
>

Yes.
But as Tobias mentioned the behaviour will change anyway using my diff.
I'm checking out the official GnuRCS version regarding this right now.
 
>   -Otto
> 

fritjof

> > 
> > fritjof
> > 
> > Index: co.c
> > ===
> > RCS file: /cvs/src/usr.bin/rcs/co.c,v
> > retrieving revision 1.119
> > diff -u -p -r1.119 co.c
> > --- co.c10 Oct 2014 08:15:25 -  1.119
> > +++ co.c23 Nov 2014 15:40:30 -
> > @@ -265,18 +265,14 @@ checkout_rev(RCSFILE *file, RCSNUM *frev
> > (void)fprintf(stderr,
> > "no revisions present; generating empty revision 0.0\n");
> >  
> > -   /* XXX rcsnum_cmp()
> > +   /*
> >  * Check out the latest revision if  is greater than HEAD
> >  */
> > if (file->rf_ndelta != 0) {
> > -   for (i = 0; i < file->rf_head->rn_len; i++) {
> > -   if (file->rf_head->rn_id[i] < frev->rn_id[i]) {
> > -   frev = file->rf_head;
> > -   break;
> > -   }
> > -   }
> > +   if (rcsnum_cmp(file->rf_head, frev, 0) == 1)
> > +   frev = file->rf_head;
> > }
> > -
> > +   
> > lcount = 0;
> > TAILQ_FOREACH(lkp, &(file->rf_locks), rl_list) {
> > if (!strcmp(lkp->rl_name, lockname))
> > 
> > 
> > Index: rcs.c
> > ===
> > RCS file: /cvs/src/usr.bin/rcs/rcs.c,v
> > retrieving revision 1.81
> > diff -u -p -r1.81 rcs.c
> > --- rcs.c   10 Oct 2014 08:15:25 -  1.81
> > +++ rcs.c   23 Nov 2014 15:56:58 -
> > @@ -905,16 +905,15 @@ rcs_getrev(RCSFILE *rfp, RCSNUM *frev)
> > else
> > rev = frev;
> >  
> > -   /* XXX rcsnum_cmp() */
> > -   for (i = 0; i < rfp->rf_head->rn_len; i++) {
> > -   if (rfp->rf_head->rn_id[i] < rev->rn_id[i]) {
> > -   rcs_errno = RCS_ERR_NOENT;
> > -   return (NULL);
> > -   }
> > +   if(rcsnum_cmp(rfp->rf_head, rev, 0) == 1) {
> > +   rcs_errno = RCS_ERR_NOENT;
> > +   return (NULL);
> > }
> > -
> > -   /* No matter what, we'll need everything parsed up until the description
> > -   so go for it. */
> > +   
> > +   /*
> > +* No matter what, we'll need everything parsed up until the description
> > +* so go for it.
> > +*/
> > if (rcsparse_deltas(rfp, NULL))
> > return (NULL);
> >  
> 
> 


pgpZD2UEfcOPl.pgp
Description: PGP signature


httpd: explicit errors on unknown method or missing http version

2014-11-28 Thread Bertrand Janin
Hi,

If a client sends a bogus request with an unknown method or no http version
string, httpd currently grabs the last error with strerror(), this patch causes
it to call server_abort_http() directly with a more explicit error message:



Index: server_http.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
retrieving revision 1.54
diff -u -p -r1.54 server_http.c
--- server_http.c   25 Oct 2014 03:23:49 -  1.54
+++ server_http.c   28 Nov 2014 15:37:51 -
@@ -216,8 +216,10 @@ server_read_http(struct bufferevent *bev
 */
if (clt->clt_line == 1) {
if ((desc->http_method = server_httpmethod_byname(key))
-   == HTTP_METHOD_NONE)
-   goto fail;
+   == HTTP_METHOD_NONE) {
+   server_abort_http(clt, 501, "unknown method");
+   return
+   }
 
/*
 * Decode request path and query
@@ -230,7 +232,8 @@ server_read_http(struct bufferevent *bev
desc->http_version = strchr(desc->http_path, ' ');
if (desc->http_version == NULL) {
free(line);
-   goto fail;
+   server_abort_http(clt, 500, "no http version");
+   return;
}
*desc->http_version++ = '\0';
desc->http_query = strchr(desc->http_path, '?');



Re: [Patch]rcs: use rcsnum_cmp

2014-11-28 Thread Otto Moerbeek
On Sun, Nov 23, 2014 at 05:19:16PM +0100, Fritjof Bornebusch wrote:

> Hi tech,
> 
> like the XXX comment says, rcsnum_cmp() can be used instead of a *for* loop.
> The following shows the original behavior:
> 
> $ co -r1.2 foo.txt,v
> foo.txt,v  -->  foo.txt
> revision 1.2
> done
> 
> $ co -r1.1 foo.txt,v 
> foo.txt,v  -->  foo.txt
> revision 1.1
> done
> 
> $ co foo.txt,v
> foo.txt,v  -->  foo.txt
> revision 1.2
> done
> 
> The following shows the changed behavior:
> 
> $ co -r1.2 foo.txt,v 
> foo.txt,v  -->  foo.txt
> revision 1.2
> done
> 
> $ co -r1.1 foo.txt,v
> foo.txt,v  -->  foo.txt
> revision 1.1
> done
> 
> $ co foo.txt,v
> foo.txt,v  -->  foo.txt
> revision 1.2
> done
> 
> Could some verify that I didn't miss a test case.

Hmm, your examples show no diffference. I think the intention is to
not change the behaviour, right? 

-Otto

> 
> fritjof
> 
> Index: co.c
> ===
> RCS file: /cvs/src/usr.bin/rcs/co.c,v
> retrieving revision 1.119
> diff -u -p -r1.119 co.c
> --- co.c  10 Oct 2014 08:15:25 -  1.119
> +++ co.c  23 Nov 2014 15:40:30 -
> @@ -265,18 +265,14 @@ checkout_rev(RCSFILE *file, RCSNUM *frev
>   (void)fprintf(stderr,
>   "no revisions present; generating empty revision 0.0\n");
>  
> - /* XXX rcsnum_cmp()
> + /*
>* Check out the latest revision if  is greater than HEAD
>*/
>   if (file->rf_ndelta != 0) {
> - for (i = 0; i < file->rf_head->rn_len; i++) {
> - if (file->rf_head->rn_id[i] < frev->rn_id[i]) {
> - frev = file->rf_head;
> - break;
> - }
> - }
> + if (rcsnum_cmp(file->rf_head, frev, 0) == 1)
> + frev = file->rf_head;
>   }
> -
> + 
>   lcount = 0;
>   TAILQ_FOREACH(lkp, &(file->rf_locks), rl_list) {
>   if (!strcmp(lkp->rl_name, lockname))
> 
> 
> Index: rcs.c
> ===
> RCS file: /cvs/src/usr.bin/rcs/rcs.c,v
> retrieving revision 1.81
> diff -u -p -r1.81 rcs.c
> --- rcs.c 10 Oct 2014 08:15:25 -  1.81
> +++ rcs.c 23 Nov 2014 15:56:58 -
> @@ -905,16 +905,15 @@ rcs_getrev(RCSFILE *rfp, RCSNUM *frev)
>   else
>   rev = frev;
>  
> - /* XXX rcsnum_cmp() */
> - for (i = 0; i < rfp->rf_head->rn_len; i++) {
> - if (rfp->rf_head->rn_id[i] < rev->rn_id[i]) {
> - rcs_errno = RCS_ERR_NOENT;
> - return (NULL);
> - }
> + if(rcsnum_cmp(rfp->rf_head, rev, 0) == 1) {
> + rcs_errno = RCS_ERR_NOENT;
> + return (NULL);
>   }
> -
> - /* No matter what, we'll need everything parsed up until the description
> -   so go for it. */
> + 
> + /*
> +  * No matter what, we'll need everything parsed up until the description
> +  * so go for it.
> +  */
>   if (rcsparse_deltas(rfp, NULL))
>   return (NULL);
>  




Re: 64-bit PCI bridge support: testers needed

2014-11-28 Thread sven falempin
There is no direct relation with the commit, but i will anyway report
the test result on my problematic hardware.
The behavior is mostly the same, but now  is
spammed by the kernel.

As you can see the system work for more than 40 hours.

I really wonder what could be the cause of this, after rebooting the
machine reproduce the bug quickly,
maybe there is some kind of electric problem.

Aug 22 18:00:07 unicorn /bsd: rl0: watchdog timeout
# uptime
 7:58PM  up 1 day, 23:17, 1 user, load averages: 5.23, 5.50, 5.30
# date
Fri Aug 22 19:58:38 EDT 2014
# rl0: watchdog timeout
rl0: watchdog timeout


On Wed, Nov 26, 2014 at 10:10 AM, sven falempin  wrote:
> HEllo,
>
> So i reported a bug with a pci bridge a while ago. On an Apu with a
> pci to pci bridge over pci express.
> Dmesg below
>
> I use a recent snapshot
> OpenBSD 5.6-current (GENERIC.MP) #610: Tue Nov 25 06:00:07 MST 2014
>
> and assume the commit was in
>
> The situation improved, as i can have the card running with bsd.mp for
> more than a second. But on the second boot.
> During the first boot i add the  kernel message when
> asking for a lease on vr3. At this moment my not reliable serial usb
> decide to do the classic freez. This time i was not able to get access
> to the machine.
>
> I am currently sending icmp trhough rl and vr and waiting for a problem.
>
> bug mail objet was : 
>
> dmesg:
>
>
> OpenBSD 5.6-current (GENERIC.MP) #610: Tue Nov 25 06:00:07 MST 2014
> dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> real mem = 2098520064 (2001MB)
> avail mem = 2038870016 (1944MB)
> mpath0 at root
> scsibus0 at mpath0: 256 targets
> mainbus0 at root
> bios0 at mainbus0: SMBIOS rev. 2.7 @ 0x7e16d820 (7 entries)
> bios0: vendor coreboot version "4.0" date 09/08/2014
> bios0: PC Engines APU
> acpi0 at bios0: rev 0
> acpi0: sleep states S0 S1 S3 S4 S5
> acpi0: tables DSDT FACP SPCR HPET APIC HEST SSDT SSDT SSDT
> acpi0: wakeup devices AGPB(S4) HDMI(S4) PBR4(S4) PBR5(S4) PBR6(S4)
> PBR7(S4) PE20(S4) PE21(S4) PE22(S4) PE23(S4) PIBR(S4) UOH1(S3)
> UOH2(S3) UOH3(S3) UOH4(S3) UOH5(S3) [...]
> acpitimer0 at acpi0: 3579545 Hz, 32 bits
> acpihpet0 at acpi0: 14318180 Hz
> acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
> cpu0 at mainbus0: apid 0 (boot processor)
> cpu0: AMD G-T40E Processor, 1000.14 MHz
> cpu0: 
> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,MWAIT,SSSE3,CX16,POPCNT,NXE,MMXX,FFXSR,PAGE1GB,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,IBS,SKINIT,ITSC
> cpu0: 32KB 64b/line 2-way I-cache, 32KB 64b/line 8-way D-cache, 512KB
> 64b/line 16-way L2 cache
> cpu0: 8 4MB entries fully associative
> cpu0: DTLB 40 4KB entries fully associative, 8 4MB entries fully associative
> cpu0: smt 0, core 0, package 0
> mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
> cpu0: apic clock running at 200MHz
> cpu1 at mainbus0: apid 1 (application processor)
> cpu1: AMD G-T40E Processor, 1000.00 MHz
> cpu1: 
> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,MWAIT,SSSE3,CX16,POPCNT,NXE,MMXX,FFXSR,PAGE1GB,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,IBS,SKINIT,ITSC
> cpu1: 32KB 64b/line 2-way I-cache, 32KB 64b/line 8-way D-cache, 512KB
> 64b/line 16-way L2 cache
> cpu1: 8 4MB entries fully associative
> cpu1: DTLB 40 4KB entries fully associative, 8 4MB entries fully associative
> cpu1: smt 0, core 1, package 0
> ioapic0 at mainbus0: apid 2 pa 0xfec0, version 21, 24 pins
> acpiprt0 at acpi0: bus -1 (AGPB)
> acpiprt1 at acpi0: bus -1 (HDMI)
> acpiprt2 at acpi0: bus 1 (PBR4)
> acpiprt3 at acpi0: bus 2 (PBR5)
> acpiprt4 at acpi0: bus 3 (PBR6)
> acpiprt5 at acpi0: bus -1 (PBR7)
> acpiprt6 at acpi0: bus 5 (PE20)
> acpiprt7 at acpi0: bus -1 (PE21)
> acpiprt8 at acpi0: bus -1 (PE22)
> acpiprt9 at acpi0: bus -1 (PE23)
> acpiprt10 at acpi0: bus 0 (PCI0)
> acpiprt11 at acpi0: bus 4 (PIBR)
> acpicpu0 at acpi0: C2, PSS
> acpicpu1 at acpi0: C2, PSS
> acpibtn0 at acpi0: PWRB
> cpu0: 1000 MHz: speeds: 1000 800 MHz
> pci0 at mainbus0 bus 0
> pchb0 at pci0 dev 0 function 0 "AMD AMD64 14h Host" rev 0x00
> ppb0 at pci0 dev 4 function 0 "AMD AMD64 14h PCIE" rev 0x00: msi
> pci1 at ppb0 bus 1
> re0 at pci1 dev 0 function 0 "Realtek 8168" rev 0x06: RTL8168E/8111E
> (0x2c00), msi, address 00:0d:b9:33:85:d8
> rgephy0 at re0 phy 7: RTL8169S/8110S PHY, rev. 4
> ppb1 at pci0 dev 5 function 0 "AMD AMD64 14h PCIE" rev 0x00: msi
> pci2 at ppb1 bus 2
> re1 at pci2 dev 0 function 0 "Realtek 8168" rev 0x06: RTL8168E/8111E
> (0x2c00), msi, address 00:0d:b9:33:85:d9
> rgephy1 at re1 phy 7: RTL8169S/8110S PHY, rev. 4
> ppb2 at pci0 dev 6 function 0 "AMD AMD64 14h PCIE" rev 0x00: msi
> pci3 at ppb2 bus 3
> re2 at pci3 dev 0 function 0 "Realtek 8168" rev 0x06: RTL8168E/8111E
> (0x2c00), msi, address 00:0d:b9:33:85:da
> rgephy2 at re2 phy 7: RTL8169S/8110S PHY, rev. 4
> ahci0 at pci0 dev 17 function 0 "ATI SBx00 SATA" rev 0x40: a

Re: Behavior of changing routes on OpenBSD 5.6

2014-11-28 Thread Florian Riehm
Hi Martin,

the tests were successful.
Your fix works.

Thanks for your efforts!

Florian


On 11/27/14 09:55, Florian Riehm wrote:
> Hi Martin,
> 
> thank you for clarification and thank you for your patch.
> 
> Your patch looks reasonably to me. I forgot RTAX_IFP and RTAX_IFA in my patch.
> After a first trial, the fix works for me.
> Today I will start nightly regression tests with it.
> You will get the result tomorrow.
> 
> Regards
> 
> Florian
> 
> On 11/26/14 12:58, Martin Pieuchot wrote:
>> Hello Florian,
>>
>> On 26/11/14(Wed) 06:56, Florian Riehm wrote:
>>> since OpenBSD 5.6 route change messages can change the interface of a route
>>> (rt_ifa) even if a message doesn't seem to require it because of a changed
>>> gateway or stuff like that.
>>> I would like to ask if it's a regression or if the new behavior is intended.
>>
>> Since the behavior is different and it's not documented, it is a 
>> regression, thanks for reporting it.  I've just committed some new
>> tests in order to prevent this from happening again.
>>
>>> Example: (only for testing - it doesn't represent my network topology)
>>> ifconfig em0 inet6 fd88::1/64
>>> ifconfig em1 inet6 fd99::1/64
>>> route add -inet6 fd88::666 fd99::1
>>> route get fd88::666
>>> interface: em1 (as expected)
>>> route change fd88::666 -mtu 1500
>>> route get fd88::666
>>> interface: em0 (broken - trying to ping the target results in "No route 
>>> to
>>> host")
>>>
>>> In the example I can workaround the problem with adding a gateway while 
>>> changing
>>> the mtu:
>>> route change fd88::666 fd99::1 -mtu 1500
>>>
>>> A comment in route_output (rtsock.c) says
>>> /*
>>> * new gateway could require new ifaddr, ifp;
>>> * flags may also be different; ifp may be specified
>>> * by ll sockaddr when protocol address is ambiguous
>>> */
>>> but their is no check for a 'new gateway'.
>>
>> You're right.  What's happening here is that we always call rt_geifa() in
>> the first place.  This is a nasty function that tries to find an ifaddr
>> to attach a route.  But if the gateway is not new or you didn't specify
>> any of the "-ifp" or "-ifa" argument we should not look for a different
>> ifaddr.
>>
>> Here's a slightly different diff, could you tell me if it fixes the
>> regression in your case?
>>
>> Index: net/rtsock.c
>> ===
>> RCS file: /home/ncvs/src/sys/net/rtsock.c,v
>> retrieving revision 1.152
>> diff -u -p -r1.152 rtsock.c
>> --- net/rtsock.c 12 Aug 2014 13:52:08 -  1.152
>> +++ net/rtsock.c 26 Nov 2014 11:55:25 -
>> @@ -740,13 +740,6 @@ report:
>>  break;
>>  
>>  case RTM_CHANGE:
>> -/*
>> - * new gateway could require new ifaddr, ifp;
>> - * flags may also be different; ifp may be specified
>> - * by ll sockaddr when protocol address is ambiguous
>> - */
>> -if ((error = rt_getifa(&info, tableid)) != 0)
>> -goto flush;
>>  newgate = 0;
>>  if (info.rti_info[RTAX_GATEWAY] != NULL)
>>  if (rt->rt_gateway == NULL ||
>> @@ -761,7 +754,17 @@ report:
>>  error = EDQUOT;
>>  goto flush;
>>  }
>> -ifa = info.rti_ifa;
>> +/*
>> + * new gateway could require new ifaddr, ifp;
>> + * flags may also be different; ifp may be specified
>> + * by ll sockaddr when protocol address is ambiguous
>> + */
>> +if (newgate || info.rti_info[RTAX_IFP] != NULL ||
>> +info.rti_info[RTAX_IFA] != NULL) {
>> +if ((error = rt_getifa(&info, tableid)) != 0)
>> +goto flush;
>> +ifa = info.rti_ifa;
>> +}
>>  if (ifa) {
>>  if (rt->rt_ifa != ifa) {
>>  if (rt->rt_ifa->ifa_rtrequest)
>>
> 



Re: tcpdump: Ethernet header is not dumped with -xX if IP header is unaligned

2014-11-28 Thread Mike Belopuhov
Still looking for OK's.

On Wed, Nov 26, 2014 at 17:18 +0100, Mike Belopuhov wrote:
> > 
> > better diff.  the problem is that dissectors use packetp and
> > snapend pointers themselves therefore they should be pointing
> > to the newly allocated structure.  we can restore them once
> > we're done with the inner content and go back to the caller
> > to see if we need to hexdump the contents.
> > 
> > i'll see if i can cook and test the ipv6 version.
> > 
> > OK?
> > 
> 
> now with an ip6 version and i've made sure that this fixes
> dumping unaligned ipv6 packets as well.  in the meantime
> jsg@ has lured me into looking at the afl crash in the same
> code and it looks like the check from ip6_print is useful
> here: if we haven't got enough data for a header, don't
> bother with anything else and just bail.
> 
> ok?
> 
> diff --git usr.sbin/tcpdump/print-ip.c usr.sbin/tcpdump/print-ip.c
> index 3f4194c..e9d2185 100644
> --- usr.sbin/tcpdump/print-ip.c
> +++ usr.sbin/tcpdump/print-ip.c
> @@ -351,22 +351,27 @@ in_cksum(const u_short *addr, register int len, int 
> csum)
>   * print an IP datagram.
>   */
>  void
>  ip_print(register const u_char *bp, register u_int length)
>  {
> + static u_char *abuf = NULL;
>   register const struct ip *ip;
>   register u_int hlen, len, off;
>   register const u_char *cp;
> + const u_char *pktp = packetp;
> + const u_char *send = snapend;
>  
>   ip = (const struct ip *)bp;
> + if ((u_char *)(ip + 1) > snapend) {
> + printf("[|ip]");
> + return;
> + }
> +
>   /*
>* If the IP header is not aligned, copy into abuf.
> -  * This will never happen with BPF.  It does happen with raw packet
> -  * dumps from -r.
>*/
>   if ((intptr_t)ip & (sizeof(long)-1)) {
> - static u_char *abuf = NULL;
>   static int didwarn = 0;
>   int clen = snapend - bp;
>  
>   if (clen > snaplen)
>   clen = snaplen;
> @@ -387,11 +392,11 @@ ip_print(register const u_char *bp, register u_int 
> length)
>   }
>  
>   TCHECK(*ip);
>   if (ip->ip_v != IPVERSION) {
>   (void)printf("bad-ip-version %u", ip->ip_v);
> - return;
> + goto out;
>   }
>  
>   len = ntohs(ip->ip_len);
>   if (length < len) {
>   (void)printf("truncated-ip - %d bytes missing!",
> @@ -400,11 +405,11 @@ ip_print(register const u_char *bp, register u_int 
> length)
>   }
>  
>   hlen = ip->ip_hl * 4;
>   if (hlen < sizeof(struct ip) || hlen > len) {
>   (void)printf("bad-hlen %d", hlen);
> - return;
> + goto out;
>   }
>  
>   len -= hlen;
>  
>   /*
> @@ -465,11 +470,11 @@ ip_print(register const u_char *bp, register u_int 
> length)
>ipaddr_string(&ip->ip_src),
>ipaddr_string(&ip->ip_dst));
>   ip_print(cp, len);
>   if (! vflag) {
>   printf(" (encap)");
> - return;
> + goto out;
>   }
>   break;
>  
>  #ifdef INET6
>  #ifndef IPPROTO_IPV6
> @@ -482,11 +487,11 @@ ip_print(register const u_char *bp, register u_int 
> length)
>ipaddr_string(&ip->ip_src),
>ipaddr_string(&ip->ip_dst));
>   ip6_print(cp, len);
>   if (! vflag) {
>   printf(" (encap)");
> - return;
> + goto out;
>   }
>   break;
>  #endif /*INET6*/
>  
>  #ifndef IPPROTO_GRE
> @@ -499,11 +504,11 @@ ip_print(register const u_char *bp, register u_int 
> length)
>ipaddr_string(&ip->ip_dst));
>   /* do it */
>   gre_print(cp, len);
>   if (! vflag) {
>   printf(" (gre encap)");
> - return;
> + goto out;
>   }
>   break;
>  
>  #ifndef IPPROTO_ESP
>  #define IPPROTO_ESP 50
> @@ -528,11 +533,11 @@ ip_print(register const u_char *bp, register u_int 
> length)
>ipaddr_string(&ip->ip_src),
>ipaddr_string(&ip->ip_dst));
>   mobile_print(cp, len);
>   if (! vflag) {
>   printf(" (mobile encap)");
> - return;
> + goto out;
>   }
>   break;
>  
>  #ifndef IPPROTO_ETHERIP
>  #define IPPROTO_ETHERIP  97
> @@ -653,10 +658,13 @@ ip_prin

Re: operations on nd_prefix list must take rdomain into account

2014-11-28 Thread Mike Belopuhov
Still looking for OK's.

On Wed, Nov 26, 2014 at 18:24 +0100, Mike Belopuhov wrote:
> More rdomain checks are needed to be able to use the same subnet
> in a back to back connection between IPv6 rdomains as pointed out
> by mpi@.
> 
> OK?
> 
> diff --git sys/netinet6/nd6.c sys/netinet6/nd6.c
> index 9616187..d704cd6 100644
> --- sys/netinet6/nd6.c
> +++ sys/netinet6/nd6.c
> @@ -1264,10 +1264,13 @@ nd6_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp)
>   s = splsoftnet();
>   /* First purge the addresses referenced by a prefix. */
>   LIST_FOREACH_SAFE(pr, &nd_prefix, ndpr_entry, npr) {
>   struct in6_ifaddr *ia6, *ia6_next;
>  
> + if (pr->ndpr_ifp->if_rdomain != ifp->if_rdomain)
> + continue;
> +
>   if (IN6_IS_ADDR_LINKLOCAL(&pr->ndpr_prefix.sin6_addr))
>   continue; /* XXX */
>  
>   /* do we really have to remove addresses as well? */
>   TAILQ_FOREACH_SAFE(ia6, &in6_ifaddr, ia_list, ia6_next) 
> {
> @@ -1282,10 +1285,13 @@ nd6_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp)
>* Purging the addresses might remove the prefix as well.
>* So run the loop again to access only prefixes that have
>* not been freed already.
>*/
>   LIST_FOREACH_SAFE(pr, &nd_prefix, ndpr_entry, npr) {
> + if (pr->ndpr_ifp->if_rdomain != ifp->if_rdomain)
> + continue;
> +
>   if (IN6_IS_ADDR_LINKLOCAL(&pr->ndpr_prefix.sin6_addr))
>   continue; /* XXX */
>  
>   prelist_remove(pr);
>   }
> diff --git sys/netinet6/nd6_rtr.c sys/netinet6/nd6_rtr.c
> index bfc9c9f..e46b3b4 100644
> --- sys/netinet6/nd6_rtr.c
> +++ sys/netinet6/nd6_rtr.c
> @@ -1690,10 +1690,13 @@ nd6_prefix_onlink(struct nd_prefix *pr)
>* interface, and the prefix has already installed the interface route.
>* Although such a configuration is expected to be rare, we explicitly
>* allow it.
>*/
>   LIST_FOREACH(opr, &nd_prefix, ndpr_entry) {
> + if (opr->ndpr_ifp->if_rdomain != ifp->if_rdomain)
> + continue;
> +
>   if (opr == pr)
>   continue;
>  
>   if ((opr->ndpr_stateflags & NDPRF_ONLINK) == 0)
>   continue;
> @@ -1826,10 +1829,13 @@ nd6_prefix_offlink(struct nd_prefix *pr)
>* the interface route (see comments in nd6_prefix_onlink).
>* If there's one, try to make the prefix on-link on the
>* interface.
>*/
>   LIST_FOREACH(opr, &nd_prefix, ndpr_entry) {
> + if (opr->ndpr_ifp->if_rdomain != ifp->if_rdomain)
> + continue;
> +
>   if (opr == pr)
>   continue;
>  
>   if ((opr->ndpr_stateflags & NDPRF_ONLINK) != 0)
>   continue;



Re: Make every interface with a watchdog register it's own timeout

2014-11-28 Thread Mike Belopuhov
I've got one ok for this.  Can I have another one please?

On Mon, Nov 24, 2014 at 13:54 +0100, Mike Belopuhov wrote:
> On Sun, Nov 23, 2014 at 12:06 +0100, Martin Pieuchot wrote:
> > On 23/11/14(Sun) 02:10, Mike Belopuhov wrote:
> > > Hi,
> > > 
> > > This removes the system wide if_slowtimo timeout and lets every
> > > interface with a valid if_watchdog method register it's own.
> > > The rational is to get rid of the ifnet loop in the softclock
> > > context to avoid further complications with concurrent access
> > > to the ifnet list.  This might also save some cpu cycles in the
> > > runtime if number of interfaces is large while only a fraction
> > > all of them have a watchdog routine.
> > > 
> > > This has originally come up on NetBSD mailing lists with some
> > > rather complicated locking strategy.  Masao Uebayashi has later
> > > proposed a similar solution.
> > > 
> > > The diff was looked at and corrected by mpi@.  I've tested the
> > > trunk and a system build as well.  OK?
> > 
> > I'm basically ok, some remarks below.
> > 
> > 
> > What about putting the allocation inside if_attach_common() with the
> > hooks that are allocated for the same reason?
> 
> Sure.
> 
> > Maybe we should explain why we allocate them?
> > 
> 
> We all know why, don't think it will help anything.
> 
> New version.  OK?
> 
> 
> diff --git sys/net/if.c sys/net/if.c
> index 0632eec..679c8c8 100644
> --- sys/net/if.c
> +++ sys/net/if.c
> @@ -133,11 +133,10 @@ voidif_attachdomain1(struct ifnet *);
>  void if_attach_common(struct ifnet *);
>  
>  void if_detach_queues(struct ifnet *, struct ifqueue *);
>  void if_detached_start(struct ifnet *);
>  int  if_detached_ioctl(struct ifnet *, u_long, caddr_t);
> -void if_detached_watchdog(struct ifnet *);
>  
>  int  if_getgroup(caddr_t, struct ifnet *);
>  int  if_getgroupmembers(caddr_t);
>  int  if_getgroupattribs(caddr_t);
>  int  if_setgroupattribs(caddr_t);
> @@ -169,16 +168,12 @@ int net_livelocked(void);
>   * parameters.
>   */
>  void
>  ifinit()
>  {
> - static struct timeout if_slowtim;
> -
> - timeout_set(&if_slowtim, if_slowtimo, &if_slowtim);
>   timeout_set(&net_tick_to, net_tick, &net_tick_to);
>  
> - if_slowtimo(&if_slowtim);
>   net_tick(&net_tick_to);
>  }
>  
>  static unsigned int if_index = 0;
>  static unsigned int if_indexlim = 0;
> @@ -270,10 +265,13 @@ if_attachsetup(struct ifnet *ifp)
>   if_attachdomain1(ifp);
>  #if NPF > 0
>   pfi_attach_ifnet(ifp);
>  #endif
>  
> + timeout_set(ifp->if_slowtimo, if_slowtimo, ifp);
> + if_slowtimo(ifp);
> +
>   /* Announce the interface. */
>   rt_ifannouncemsg(ifp, IFAN_ARRIVAL);
>  }
>  
>  /*
> @@ -426,10 +424,13 @@ if_attach_common(struct ifnet *ifp)
>   M_TEMP, M_WAITOK);
>   TAILQ_INIT(ifp->if_linkstatehooks);
>   ifp->if_detachhooks = malloc(sizeof(*ifp->if_detachhooks),
>   M_TEMP, M_WAITOK);
>   TAILQ_INIT(ifp->if_detachhooks);
> +
> + ifp->if_slowtimo = malloc(sizeof(*ifp->if_slowtimo), M_TEMP,
> + M_WAITOK|M_ZERO);
>  }
>  
>  void
>  if_start(struct ifnet *ifp)
>  {
> @@ -481,19 +482,22 @@ if_detach(struct ifnet *ifp)
>   struct domain *dp;
>  
>   ifp->if_flags &= ~IFF_OACTIVE;
>   ifp->if_start = if_detached_start;
>   ifp->if_ioctl = if_detached_ioctl;
> - ifp->if_watchdog = if_detached_watchdog;
> + ifp->if_watchdog = NULL;
>  
>   /*
>* Call detach hooks from head to tail.  To make sure detach
>* hooks are executed in the reverse order they were added, all
>* the hooks have to be added to the head!
>*/
>   dohooks(ifp->if_detachhooks, HOOK_REMOVE | HOOK_FREE);
>  
> + /* Remove the watchdog timeout */
> + timeout_del(ifp->if_slowtimo);
> +
>  #if NBRIDGE > 0
>   /* Remove the interface from any bridge it is part of.  */
>   if (ifp->if_bridgeport)
>   bridge_ifdetach(ifp);
>  #endif
> @@ -577,10 +581,12 @@ do { \
>  
>   free(ifp->if_addrhooks, M_TEMP, 0);
>   free(ifp->if_linkstatehooks, M_TEMP, 0);
>   free(ifp->if_detachhooks, M_TEMP, 0);
>  
> + free(ifp->if_slowtimo, M_TEMP, sizeof(*ifp->if_slowtimo));
> +
>   for (dp = domains; dp; dp = dp->dom_next) {
>   if (dp->dom_ifdetach && ifp->if_afdata[dp->dom_family])
>   (*dp->dom_ifdetach)(ifp,
>   ifp->if_afdata[dp->dom_family]);
>   }
> @@ -1160,29 +1166,26 @@ if_link_state_change_task(void *arg, void *unused)
>   }
>   splx(s);
>  }
>  
>  /*
> - * Handle interface watchdog timer routines.  Called
> - * from softclock, we decrement timers (if set) and
> + * Handle interface watchdog timer routine.  Called
> + * from softclock, we decrement timer (if set) and
>   * call the appropriate interface routine on expiration.
>   */
>  void
>  if_slowtimo(void *arg)
>  {
> - struct timeout *to = (struct timeout *)arg;
> - struct ifnet *ifp;
> + struct ifnet *ifp =