Re: Keyboard resume (zzz) diff
On Tue, Apr 12, 2016 at 06:24:10AM +, Miod Vallat wrote: > > wskbd_enable (calls pckbd_enable) does not appear to be called from > > wskbd_activate, perhaps just calling wskbd_enable from there would be > > better? (as a matter of fact, wskbd_activate doesn't really do much at > > all except set sc_dying=1, and I'm not sure where that gets reset.) > > Nooo! wskbd_enable() is not used in the physical device activation > path, but in the logical path, so that keyboard input gets ignored > unless its device is actually opened. If you are running with serial > console and no getty listening to /dev/ttyC*, for example, then > wskbd_enable() should not get invoked at all! > > Now, some of the work done in wskbd_enable() might be worth doing in > wskbd_activate(), but be very careful when changing this. > That is the same conclusion I came to after reading it more. I think the best idea is to proceed with the diff as posted, with the added check to make sure the keyboard is enabled, and to always call config_activate_children on all paths through pckbd_activate? In other words, no changes to wskbd_enable or wskbd_activate. Sound right?
net80211: with more if_start()/if_enqueue()!
As reported by jsg@ the wifi stack should use if_start() just like the
rest of the kernel. One of the patterns can even be converted to
if_enqueue().
Tested with:
iwn0 at pci2 dev 0 function 0 "Intel Centrino Advanced-N 6205" rev 0x34
ok?
Index: net80211/ieee80211_input.c
===
RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v
retrieving revision 1.169
diff -u -p -r1.169 ieee80211_input.c
--- net80211/ieee80211_input.c 22 Mar 2016 11:37:35 - 1.169
+++ net80211/ieee80211_input.c 12 Apr 2016 06:21:50 -
@@ -360,7 +360,7 @@ ieee80211_input(struct ifnet *ifp, struc
/* dequeue buffered unicast frames */
while ((m = mq_dequeue(&ni->ni_savedq)) != NULL) {
mq_enqueue(&ic->ic_pwrsaveq, m);
- (*ifp->if_start)(ifp);
+ if_start(ifp);
}
}
}
@@ -2870,7 +2870,7 @@ ieee80211_recv_pspoll(struct ieee80211co
wh->i_fc[1] |= IEEE80211_FC1_MORE_DATA;
}
mq_enqueue(&ic->ic_pwrsaveq, m);
- (*ifp->if_start)(ifp);
+ if_start(ifp);
}
#endif /* IEEE80211_STA_ONLY */
Index: net80211/ieee80211_node.c
===
RCS file: /cvs/src/sys/net80211/ieee80211_node.c,v
retrieving revision 1.100
diff -u -p -r1.100 ieee80211_node.c
--- net80211/ieee80211_node.c 3 Mar 2016 07:20:45 - 1.100
+++ net80211/ieee80211_node.c 12 Apr 2016 06:22:07 -
@@ -1847,7 +1847,7 @@ ieee80211_notify_dtim(struct ieee80211co
wh->i_fc[1] |= IEEE80211_FC1_MORE_DATA;
}
mq_enqueue(&ic->ic_pwrsaveq, m);
- (*ifp->if_start)(ifp);
+ if_start(ifp);
}
/* XXX assumes everything has been sent */
ic->ic_tim_mcast_pending = 0;
Index: net80211/ieee80211_output.c
===
RCS file: /cvs/src/sys/net80211/ieee80211_output.c,v
retrieving revision 1.110
diff -u -p -r1.110 ieee80211_output.c
--- net80211/ieee80211_output.c 5 Feb 2016 19:11:31 - 1.110
+++ net80211/ieee80211_output.c 12 Apr 2016 06:22:14 -
@@ -241,7 +241,7 @@ ieee80211_mgmt_output(struct ifnet *ifp,
#endif
mq_enqueue(&ic->ic_mgtq, m);
ifp->if_timer = 1;
- (*ifp->if_start)(ifp);
+ if_start(ifp);
return 0;
}
Index: net80211/ieee80211_pae_output.c
===
RCS file: /cvs/src/sys/net80211/ieee80211_pae_output.c,v
retrieving revision 1.26
diff -u -p -r1.26 ieee80211_pae_output.c
--- net80211/ieee80211_pae_output.c 25 Nov 2015 03:10:00 - 1.26
+++ net80211/ieee80211_pae_output.c 12 Apr 2016 06:25:59 -
@@ -66,7 +66,7 @@ ieee80211_send_eapol_key(struct ieee8021
struct ether_header *eh;
struct ieee80211_eapol_key *key;
u_int16_t info;
- int s, len, error;
+ int len;
M_PREPEND(m, sizeof(struct ether_header), M_DONTWAIT);
if (m == NULL)
@@ -118,22 +118,12 @@ ieee80211_send_eapol_key(struct ieee8021
if (info & EAPOL_KEY_KEYMIC)
ieee80211_eapol_key_mic(key, ptk->kck);
- len = m->m_pkthdr.len;
- s = splnet();
#ifndef IEEE80211_STA_ONLY
/* start a 100ms timeout if an answer is expected from supplicant */
if (info & EAPOL_KEY_KEYACK)
timeout_add_msec(&ni->ni_eapol_to, 100);
#endif
- IFQ_ENQUEUE(&ifp->if_snd, m, error);
- if (error == 0) {
- ifp->if_obytes += len;
- if (!ifq_is_oactive(&ifp->if_snd))
- (*ifp->if_start)(ifp);
- }
- splx(s);
-
- return error;
+ return if_enqueue(ifp, m);
}
#ifndef IEEE80211_STA_ONLY
Index: net80211/ieee80211_proto.c
===
RCS file: /cvs/src/sys/net80211/ieee80211_proto.c,v
retrieving revision 1.64
diff -u -p -r1.64 ieee80211_proto.c
--- net80211/ieee80211_proto.c 8 Feb 2016 01:00:47 - 1.64
+++ net80211/ieee80211_proto.c 12 Apr 2016 06:22:35 -
@@ -1043,7 +1043,7 @@ justcleanup:
ieee80211_set_link_state(ic, LINK_STATE_UP);
}
ic->ic_mgt_timer = 0;
- (*ifp->if_start)(ifp);
+ if_start(ifp);
break;
}
break;
Re: Keyboard resume (zzz) diff
> wskbd_enable (calls pckbd_enable) does not appear to be called from > wskbd_activate, perhaps just calling wskbd_enable from there would be > better? (as a matter of fact, wskbd_activate doesn't really do much at > all except set sc_dying=1, and I'm not sure where that gets reset.) Nooo! wskbd_enable() is not used in the physical device activation path, but in the logical path, so that keyboard input gets ignored unless its device is actually opened. If you are running with serial console and no getty listening to /dev/ttyC*, for example, then wskbd_enable() should not get invoked at all! Now, some of the work done in wskbd_enable() might be worth doing in wskbd_activate(), but be very careful when changing this.
Re: Any reason there's no way to persist pledge(2) state across exec?
Sun, 10 Apr 2016 14:23:02 -0700 Brennan Vincent > Got it. Thanks for the explanation. > > On Sun, Apr 10, 2016, at 01:36 PM, Stuart Henderson wrote: > > On 2016/04/10 20:50, Nicholas Marriott wrote: > > > Hi > > > > > > What's the use for this? What program could use it? > > > > > > On Sun, Apr 10, 2016 at 08:48:08AM -0700, Brennan Vincent wrote: > > > > Subject basically says it all. I think some could find it useful to have > > > > `pledge` promises optionally persist even after the process calls > > > > execve. This could, for example, be implemented with an `exec_noreset` > > > > pledge that gives access to the same syscalls as `exec`, but with this > > > > restricted behavior. > > > > > > > > Is there a good technically reason this can't or shouldn't be done, or > > > > has it simply not been implemented yet? > > > > It doesn't seem like something that would be widely usable - a big > > part of how pledge is designed is based around the fact that programs > > typically need a higher level of access during startup (to open files, > > persistent sockets, etc) and can then be ratcheted down to a very small > > set of system calls after init is done. > > > > I don't think there's a technical reason why it couldn't be done, > > but it would add complexity in a security-sensitive area so it's > > unlikely to happen without a number of real-world use cases. In fact, there is persistence you just maybe don't see it from the best view point yet. It is ensured by proper program design (every step of execution possible) and privilege drop (separation, pledge, expand this here) using various (OpenBSD specific, and not only) methodologies. It is a voluntary choice of system and application design, and is present in Unix class of operating systems, also gaining popularity in others...
Re: bufcache KNF
Mon, 11 Apr 2016 15:59:53 +0200 Mike Belopuhov > On 11 April 2016 at 15:51, Mark Kettenis wrote: > > > > And prototypes with names in public headers are still an issue. > > > > Interesting point. What's a public header though? > Are files that end up in /usr/include/dev/pci/ public headers? > If so, why do we install all of them indiscriminately? With the risk of quoting the obvious, public here means reusable. $ mandoc /usr/share/man/man9/style.9 | grep -B1 -A5 -ni prototyp \ > | sed '/83/q' 56- 57: All functions are prototyped somewhere. 58- 59: Function prototypes for private functions (i.e., functions not used 60- elsewhere) go at the top of the first source module. In userland, 61- functions local to one source module should be declared ‘static’. This 62- should not be done in kernel land since it makes it impossible to use the 63- kernel debugger. 64- 65: Functions used from other parts of the kernel are prototyped in the 66- relevant include file. 67- 68- Functions that are used locally in more than one module go into a 69- separate header file, e.g., extern.h. 70- 71: Prototypes should not have variable names associated with the types; 72- i.e., 73- 74- voidfunction(int); 75- not: 76- voidfunction(int a); 77- 78: Prototypes may have an extra space after a tab to enable function names 79- to line up: 80- 81- static char *function(int, const char *); 82- static void usage(void); 83-
Re: pledge.2: sync list of syscalls with kern_pledge.c
Hi Philip, Philip Guenther wrote on Sun, Apr 10, 2016 at 08:16:21PM -0700: > On Sun, Apr 10, 2016 at 9:16 AM, Ingo Schwarze wrote: >> By the way, since sendsyslog(2) is only intended to be called by >> syslog(3) and not directly by application code, i'd prefer to have >> syslog(3) listed instead of sendsyslog(2) - but that's probably a >> seperate matter. > I'm inclined to list both; we document our internal system calls and > users can see them in the source and in kdump output. Listing both is even better. It helps both casual users and people who care about the gory details. Yours, Ingo
Re: add "route" promise to pledge.2
Hi Sebastien, Sebastien Marie wrote on Mon, Apr 11, 2016 at 11:18:34AM +0200: > Comments ? OK schwarze@. You may want to consider the nits below, but my OK doesn't depend on them. By the way, the sysctl(3) manual seems to be lacking information about NET_RT_TABLE, if somebody wants to look into that - but that's not related to this diff. Yours, Ingo > Index: pledge.2 > === > RCS file: /cvs/src/lib/libc/sys/pledge.2,v > retrieving revision 1.28 > diff -u -p -r1.28 pledge.2 > --- pledge.2 10 Apr 2016 18:52:07 - 1.28 > +++ pledge.2 11 Apr 2016 09:05:09 - > @@ -80,7 +80,8 @@ Only the > and > .Dv FIONBIO > operations are allowed by default. > -Use of the "tty" and "ioctl" promises receive more ioctl requests. > +The "audio", "ioctl", "pf", "route" and "tty" promises permit more ioctl > +requests. A minor nit: We usually add the Oxford comma, like this: The "audio", "ioctl", "pf", "route", and "tty" promises... > .Pp > .It Xr chmod 2 > .It Xr fchmod 2 > @@ -495,6 +496,25 @@ process: > .Xr setrlimit 2 , > .Xr getpriority 2 , > .Xr setpriority 2 . > +.It Va "route" > +Allows a subset of read-only > +.Xr ioctl 2 > +operations on network interfaces: > +.Pp > +.Dv SIOCGIFADDR , > +.Dv SIOCGIFFLAGS , > +.Dv SIOCGIFMETRIC , > +.Dv SIOCGIFGMEMB , > +.Dv SIOCGIFRDOMAIN , > +.Dv SIOCGIFDSTADDR_IN6 , > +.Dv SIOCGIFNETMASK_IN6 , > +.Dv SIOCGNBRINFO_IN6 , > +.Dv SIOCGIFINFO_IN6 , > +.Dv SIOCGIFMEDIA . > +.Pp > +And allows a subset of > +.Xr sysctl 3 > +interfaces for routing table observation. The following might read a bit better: It also allows read access to some .Xr sysctl 3 nodes for inspection of the routing table. > .It Va "pf" > Allows a subset of > .Xr ioctl 2
Re: Keyboard resume (zzz) diff
On Mon, Apr 11, 2016 at 11:23:57PM +0200, Martin Pieuchot wrote:
> On 10/04/16(Sun) 23:33, Mike Larkin wrote:
> > Please test this diff on all machines that you can successfully (today)
> > resume with 'zzz'. Please make sure that after resume, the keyboard
> > still works.
> >
> > This diff re-enables the keyboard on resume. Previously, we were re-enabling
> > the keyboard *controller* but apparently on some machines (notably the HP
> > 4530s) the keyboard itself resumes in 'disabled' state and requires a
> > re-enable.
>
> I wonder if this isn't a race. I though that pckbd_enable() was called
> by the wscons(4) layer when resuming. Does your keyboard also stays
> disable when you suspend and resume in console?
It's not my keyboard; it's reported to fix a bunch of HP machines of various
models.
wskbd_enable (calls pckbd_enable) does not appear to be called from
wskbd_activate, perhaps just calling wskbd_enable from there would be
better? (as a matter of fact, wskbd_activate doesn't really do much at
all except set sc_dying=1, and I'm not sure where that gets reset.)
>
> > Keyboards are finicky, and this diff requires widespread testing before it
> > can go in, to ensure no regressions. Please let me know if you have keyboard
> > issues after resume.
>
> BTW any reason for not calling pckbd_enable() here?
>
> > Index: dev/pckbc/pckbd.c
> > ===
> > RCS file: /cvs/src/sys/dev/pckbc/pckbd.c,v
> > retrieving revision 1.42
> > diff -u -p -a -u -r1.42 pckbd.c
> > --- dev/pckbc/pckbd.c 4 May 2015 09:33:46 - 1.42
> > +++ dev/pckbc/pckbd.c 11 Apr 2016 06:24:06 -
> > @@ -123,13 +123,14 @@ static int pckbd_is_console(pckbc_tag_t,
> >
> > int pckbdprobe(struct device *, void *, void *);
> > void pckbdattach(struct device *, struct device *, void *);
> > +int pckbdactivate(struct device *, int);
> >
> > struct cfattach pckbd_ca = {
> > sizeof(struct pckbd_softc),
> > pckbdprobe,
> > pckbdattach,
> > NULL,
> > - NULL
> > + pckbdactivate
> > };
> >
> > intpckbd_enable(void *, int);
> > @@ -181,6 +182,29 @@ static int pckbd_decode(struct pckbd_int
> > static int pckbd_led_encode(int);
> >
> > struct pckbd_internal pckbd_consdata;
> > +
> > +int
> > +pckbdactivate(struct device *self, int act)
> > +{
> > + struct pckbd_softc *sc = (struct pckbd_softc *)self;
> > + int rv = 0;
> > + u_char cmd[1];
> > +
> > + switch(act) {
> > + case DVACT_RESUME:
> > + /*
> > +* Some keyboards are not enabled after a reset,
> > +* so make sure it is enabled now.
> > +*/
> > + cmd[0] = KBC_ENABLE;
> > + (void) pckbc_poll_cmd(sc->id->t_kbctag, sc->id->t_kbcslot,
> > + cmd, 1, 0, NULL, 0);
> > + rv = config_activate_children(self, act);
> > + break;
> > + }
> > +
> > + return (rv);
> > +}
> >
> > int
> > pckbd_set_xtscancode(pckbc_tag_t kbctag, pckbc_slot_t kbcslot,
> >
>
Re: Keyboard resume (zzz) diff
On 10/04/16(Sun) 23:33, Mike Larkin wrote:
> Please test this diff on all machines that you can successfully (today)
> resume with 'zzz'. Please make sure that after resume, the keyboard
> still works.
>
> This diff re-enables the keyboard on resume. Previously, we were re-enabling
> the keyboard *controller* but apparently on some machines (notably the HP
> 4530s) the keyboard itself resumes in 'disabled' state and requires a
> re-enable.
I wonder if this isn't a race. I though that pckbd_enable() was called
by the wscons(4) layer when resuming. Does your keyboard also stays
disable when you suspend and resume in console?
> Keyboards are finicky, and this diff requires widespread testing before it
> can go in, to ensure no regressions. Please let me know if you have keyboard
> issues after resume.
BTW any reason for not calling pckbd_enable() here?
> Index: dev/pckbc/pckbd.c
> ===
> RCS file: /cvs/src/sys/dev/pckbc/pckbd.c,v
> retrieving revision 1.42
> diff -u -p -a -u -r1.42 pckbd.c
> --- dev/pckbc/pckbd.c 4 May 2015 09:33:46 - 1.42
> +++ dev/pckbc/pckbd.c 11 Apr 2016 06:24:06 -
> @@ -123,13 +123,14 @@ static int pckbd_is_console(pckbc_tag_t,
>
> int pckbdprobe(struct device *, void *, void *);
> void pckbdattach(struct device *, struct device *, void *);
> +int pckbdactivate(struct device *, int);
>
> struct cfattach pckbd_ca = {
> sizeof(struct pckbd_softc),
> pckbdprobe,
> pckbdattach,
> NULL,
> - NULL
> + pckbdactivate
> };
>
> int pckbd_enable(void *, int);
> @@ -181,6 +182,29 @@ static int pckbd_decode(struct pckbd_int
> static int pckbd_led_encode(int);
>
> struct pckbd_internal pckbd_consdata;
> +
> +int
> +pckbdactivate(struct device *self, int act)
> +{
> + struct pckbd_softc *sc = (struct pckbd_softc *)self;
> + int rv = 0;
> + u_char cmd[1];
> +
> + switch(act) {
> + case DVACT_RESUME:
> + /*
> + * Some keyboards are not enabled after a reset,
> + * so make sure it is enabled now.
> + */
> + cmd[0] = KBC_ENABLE;
> + (void) pckbc_poll_cmd(sc->id->t_kbctag, sc->id->t_kbcslot,
> + cmd, 1, 0, NULL, 0);
> + rv = config_activate_children(self, act);
> + break;
> + }
> +
> + return (rv);
> +}
>
> int
> pckbd_set_xtscancode(pckbc_tag_t kbctag, pckbc_slot_t kbcslot,
>
EFIboot and HP Stream 13
Hi! I am hitting an issue in efiboot on my HP Stream 13. Patch below. Sorry if this is the wrong list. When trying to boot current with the EFI bootloader it hangs: probing: pc0 mem[444K 88K 511M 1374M 19M] disk: hd0 hd1* hd2* hd3* hd4_ <- hangs. Then it stops responding and I have to power off/on. After inserting printf's in the code and troubleshooting a bit, I think I have found the issue. The block count of the device (u_int blks) is reported as 0 which makes the off % blks code blow up. Patch below tested on the HP stream allows it to boot. I also tested that is doesn't break boot on my other HP Laptop (ZBook 15) BTW, when hitting the error both blks and off where always zero, diff below only checks blks not sure if this is correct. It's probably also wrong to just return here, but hopefully this will provide someone with enough clues to fix it properly. I you want more information, please let me know and I will send it. ? hp_efiboot.patch Index: efidev.c === RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efidev.c,v retrieving revision 1.16 diff -u -p -r1.16 efidev.c --- efidev.c 6 Jan 2016 02:10:03 - 1.16 +++ efidev.c 11 Apr 2016 18:34:05 - @@ -91,6 +91,7 @@ efid_io(int rw, efi_diskinfo_t ed, u_int lba = off / blks; /* leading and trailing unaligned blocks in intrisic block */ + if(blks == 0) return (EFI_UNSUPPORTED); //HP stream 0 blocks workaround i_lblks = ((off % blks) == 0)? 0 : blks - (off % blks); i_tblks = (off + nsect) % blks;
Re: bufcache KNF
On Mon, Apr 11, 2016 at 11:49 AM, Miod Vallat wrote: > The point is that putting argument names in > public headers increases the risk of breaking third-party software > thanks to the preprocessor. The safe way for the implementation (us!) to do that is to use identifiers that start with an underbar and a lowercase letter, ala: int ptrace(int _request, pid_t _pid, caddr_t _addr, int _data); as that namespace is reserved for implementations; applications may not define macros that match those. Philip Guenther
Re: bufcache KNF
Miod Vallat wrote: > > >> fwiw i like names in prototypes, so i know what's going on. i know > >> style says that, but i think the advice is obsolete. > > > > The compiler doesn't check that the argument names in the prototype > > match those in the definition. The below program compiles without > > warning. > > This is not the point. The point is that putting argument names in > public headers increases the risk of breaking third-party software > thanks to the preprocessor. I wasn't suggesting that that's the only reason. I was just pointing it out in case someone suggested the guideline "only specify argument names in file-local prototypes".
Re: bufcache KNF
>> fwiw i like names in prototypes, so i know what's going on. i know >> style says that, but i think the advice is obsolete. > > The compiler doesn't check that the argument names in the prototype > match those in the definition. The below program compiles without > warning. This is not the point. The point is that putting argument names in public headers increases the risk of breaking third-party software thanks to the preprocessor.
pstat diff fixed segmentation fault
Running pstat with either [-M core] or [-N system] along with the -T option
results in a segmentation fault. Make sure kvm access is initialized prior
to running kvm_read() when -T is specified.
This issue appears to have been introduced when setgid kmem support was
removed in revision 1.99.
Rob
Index: pstat.c
===
RCS file: /cvs/src/usr.sbin/pstat/pstat.c,v
retrieving revision 1.101
diff -u -p -r1.101 pstat.c
--- pstat.c 11 Dec 2015 11:53:52 - 1.101
+++ pstat.c 11 Apr 2016 18:22:35 -
@@ -193,7 +193,7 @@ main(int argc, char *argv[])
need_nlist = vnodeflag || dformat;
if (nlistf != NULL || memf != NULL) {
- if (fileflag)
+ if (fileflag || totalflag)
need_nlist = 1;
}
Re: Keyboard resume (zzz) diff
On 04/10/16 23:33, Mike Larkin wrote:
Please test this diff on all machines that you can successfully (today)
resume with 'zzz'. Please make sure that after resume, the keyboard
still works.
This diff re-enables the keyboard on resume. Previously, we were re-enabling
the keyboard *controller* but apparently on some machines (notably the HP
4530s) the keyboard itself resumes in 'disabled' state and requires a
re-enable.
Keyboards are finicky, and this diff requires widespread testing before it
can go in, to ensure no regressions. Please let me know if you have keyboard
issues after resume.
Thanks.
-ml
Index: dev/pckbc/pckbd.c
===
RCS file: /cvs/src/sys/dev/pckbc/pckbd.c,v
retrieving revision 1.42
diff -u -p -a -u -r1.42 pckbd.c
--- dev/pckbc/pckbd.c 4 May 2015 09:33:46 - 1.42
+++ dev/pckbc/pckbd.c 11 Apr 2016 06:24:06 -
@@ -123,13 +123,14 @@ static int pckbd_is_console(pckbc_tag_t,
int pckbdprobe(struct device *, void *, void *);
void pckbdattach(struct device *, struct device *, void *);
+int pckbdactivate(struct device *, int);
struct cfattach pckbd_ca = {
sizeof(struct pckbd_softc),
pckbdprobe,
pckbdattach,
NULL,
- NULL
+ pckbdactivate
};
int pckbd_enable(void *, int);
@@ -181,6 +182,29 @@ static int pckbd_decode(struct pckbd_int
static intpckbd_led_encode(int);
struct pckbd_internal pckbd_consdata;
+
+int
+pckbdactivate(struct device *self, int act)
+{
+ struct pckbd_softc *sc = (struct pckbd_softc *)self;
+ int rv = 0;
+ u_char cmd[1];
+
+ switch(act) {
+ case DVACT_RESUME:
+ /*
+* Some keyboards are not enabled after a reset,
+* so make sure it is enabled now.
+*/
+ cmd[0] = KBC_ENABLE;
+ (void) pckbc_poll_cmd(sc->id->t_kbctag, sc->id->t_kbcslot,
+ cmd, 1, 0, NULL, 0);
+ rv = config_activate_children(self, act);
+ break;
+ }
+
+ return (rv);
+}
int
pckbd_set_xtscancode(pckbc_tag_t kbctag, pckbc_slot_t kbcslot,
I have an HP 9480m that suffers from the "keyboard broken after resume"
problem. This patch seems to fix it! The attached USB keyboard still
functions normally.
Thanks!
Re: Keyboard resume (zzz) diff
On Mon, Apr 11, 2016 at 04:02:34PM +0200, Mark Kettenis wrote:
> > Date: Sun, 10 Apr 2016 23:33:59 -0700
> > From: Mike Larkin
> >
> > Please test this diff on all machines that you can successfully (today)
> > resume with 'zzz'. Please make sure that after resume, the keyboard
> > still works.
> >
> > This diff re-enables the keyboard on resume. Previously, we were re-enabling
> > the keyboard *controller* but apparently on some machines (notably the HP
> > 4530s) the keyboard itself resumes in 'disabled' state and requires a
> > re-enable.
> >
> > Keyboards are finicky, and this diff requires widespread testing before it
> > can go in, to ensure no regressions. Please let me know if you have keyboard
> > issues after resume.
>
> FWIW, I think this is fairly low-risk. You should probably only do
> this if the keyboard is actually enabled. Which means, only if
> sc_enabled is set.
I was trying to find out when and under what conditions that gets set
earlier and ran into a maze. I'll give it another shot.
>
> I also wonder if we also need the pckbd_set_xtscancode() call. That
> shouldn't keep you from committing this as a first step though.
I took the bits out of the reset path, I'll look again to see if I missed
setxtscancode and add that.
>
> Also, you should always call config_activate_children(), not just only
> in the DVACT_RESUME case.
Thanks for spotting that ; the original version of the diff I was cooking
had this, but I was removing some debug printfs and looks like I got over
zealous.
-ml
>
>
> > Index: dev/pckbc/pckbd.c
> > ===
> > RCS file: /cvs/src/sys/dev/pckbc/pckbd.c,v
> > retrieving revision 1.42
> > diff -u -p -a -u -r1.42 pckbd.c
> > --- dev/pckbc/pckbd.c 4 May 2015 09:33:46 - 1.42
> > +++ dev/pckbc/pckbd.c 11 Apr 2016 06:24:06 -
> > @@ -123,13 +123,14 @@ static int pckbd_is_console(pckbc_tag_t,
> >
> > int pckbdprobe(struct device *, void *, void *);
> > void pckbdattach(struct device *, struct device *, void *);
> > +int pckbdactivate(struct device *, int);
> >
> > struct cfattach pckbd_ca = {
> > sizeof(struct pckbd_softc),
> > pckbdprobe,
> > pckbdattach,
> > NULL,
> > - NULL
> > + pckbdactivate
> > };
> >
> > intpckbd_enable(void *, int);
> > @@ -181,6 +182,29 @@ static int pckbd_decode(struct pckbd_int
> > static int pckbd_led_encode(int);
> >
> > struct pckbd_internal pckbd_consdata;
> > +
> > +int
> > +pckbdactivate(struct device *self, int act)
> > +{
> > + struct pckbd_softc *sc = (struct pckbd_softc *)self;
> > + int rv = 0;
> > + u_char cmd[1];
> > +
> > + switch(act) {
> > + case DVACT_RESUME:
> > + /*
> > +* Some keyboards are not enabled after a reset,
> > +* so make sure it is enabled now.
> > +*/
> > + cmd[0] = KBC_ENABLE;
> > + (void) pckbc_poll_cmd(sc->id->t_kbctag, sc->id->t_kbcslot,
> > + cmd, 1, 0, NULL, 0);
> > + rv = config_activate_children(self, act);
> > + break;
> > + }
> > +
> > + return (rv);
> > +}
> >
> > int
> > pckbd_set_xtscancode(pckbc_tag_t kbctag, pckbc_slot_t kbcslot,
> >
> >
Re: bufcache KNF
Ted Unangst wrote:
> Martin Pieuchot wrote:
> > ok?
> >
> > -int chillbufs(struct
> > -bufcache *cache, struct bufqueue *queue, int64_t *queuepages);
> > +int chillbufs(struct bufcache *, struct bufqueue *, int64_t *);
>
> fwiw i like names in prototypes, so i know what's going on. i know
> style says that, but i think the advice is obsolete.
The compiler doesn't check that the argument names in the prototype
match those in the definition. The below program compiles without
warning.
int f(int a);
int
f(int b)
{
return b+3;
}
int
main()
{
return f(2);
}
Re: bufcache KNF
guys. i have stuff outstanding in here. find something else to bikeshed please On Monday, 11 April 2016, Ted Unangst wrote: > Mark Kettenis wrote: > > And prototypes with names in public headers are still an issue. > > I think you misspelled standard. :) > >
Re: bufcache KNF
Mark Kettenis wrote: > And prototypes with names in public headers are still an issue. I think you misspelled standard. :)
Re: More KERNEL_LOCK in ip_{in,out}put()
> On 8 Apr 2016, at 12:58 AM, Martin Pieuchot wrote:
>
> I'd like to add a lock/unlock the remaining parts of the IPv4 forwarding
> path that still require some work:
>
> 1/ IP source options
> 2/ per-ifp lookups related to multicast traffic
> 3/ IPsec lookups
>
> This should not matter for the moment since the whole path already run
> under the big lock. However this document which part of the code needs
> to be fixed and will allow us to test the routing lookup with PF
> disabled, then the big fat PF_LOCK.
>
> Comments? Oks?
ok
>
> Index: netinet/in.c
> ===
> RCS file: /cvs/src/sys/netinet/in.c,v
> retrieving revision 1.126
> diff -u -p -r1.126 in.c
> --- netinet/in.c 21 Jan 2016 11:23:48 - 1.126
> +++ netinet/in.c 7 Apr 2016 14:48:01 -
> @@ -890,8 +890,10 @@ in_hasmulti(struct in_addr *ap, struct i
> struct in_multi *inm;
> int joined;
>
> + KERNEL_LOCK();
> IN_LOOKUP_MULTI(*ap, ifp, inm);
> joined = (inm != NULL);
> + KERNEL_UNLOCK();
>
> return (joined);
> }
> Index: netinet/ip_input.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.269
> diff -u -p -r1.269 ip_input.c
> --- netinet/ip_input.c29 Mar 2016 10:34:42 - 1.269
> +++ netinet/ip_input.c7 Apr 2016 14:48:01 -
> @@ -227,7 +227,7 @@ ipv4_input(struct mbuf *m)
> {
> struct ifnet *ifp;
> struct ip *ip;
> - int hlen, len;
> + int rv, hlen, len;
> in_addr_t pfrdr = 0;
>
> ifp = if_get(m->m_pkthdr.ph_ifidx);
> @@ -355,8 +355,6 @@ ipv4_input(struct mbuf *m)
>
> #ifdef MROUTING
> if (ipmforwarding && ip_mrouter) {
> - int rv;
> -
> if (m->m_flags & M_EXT) {
> if ((m = m_pullup(m, hlen)) == NULL) {
> ipstat.ips_toosmall++;
> @@ -430,7 +428,10 @@ ipv4_input(struct mbuf *m)
> }
> #ifdef IPSEC
> if (ipsec_in_use) {
> - if (ip_input_ipsec_fwd_check(m, hlen) != 0) {
> + KERNEL_LOCK();
> + rv = ip_input_ipsec_fwd_check(m, hlen);
> + KERNEL_UNLOCK();
> + if (rv != 0) {
> ipstat.ips_cantforward++;
> goto bad;
> }
> @@ -1025,6 +1026,7 @@ ip_dooptions(struct mbuf *m, struct ifne
> cp = (u_char *)(ip + 1);
> cnt = (ip->ip_hl << 2) - sizeof (struct ip);
>
> + KERNEL_LOCK();
> for (; cnt > 0; cnt -= optlen, cp += optlen) {
> opt = cp[IPOPT_OPTVAL];
> if (opt == IPOPT_EOL)
> @@ -1225,12 +1227,14 @@ ip_dooptions(struct mbuf *m, struct ifne
> ipt.ipt_ptr += sizeof(u_int32_t);
> }
> }
> + KERNEL_UNLOCK();
> if (forward && ipforwarding) {
> ip_forward(m, ifp, 1);
> return (1);
> }
> return (0);
> bad:
> + KERNEL_UNLOCK();
> icmp_error(m, type, code, 0, 0);
> ipstat.ips_badoptions++;
> return (1);
> Index: netinet/ip_output.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_output.c,v
> retrieving revision 1.318
> diff -u -p -r1.318 ip_output.c
> --- netinet/ip_output.c 11 Feb 2016 12:56:08 - 1.318
> +++ netinet/ip_output.c 7 Apr 2016 14:48:01 -
> @@ -100,10 +100,9 @@ ip_output(struct mbuf *m0, struct mbuf *
> struct ifnet *ifp = NULL;
> struct mbuf *m = m0;
> int hlen = sizeof (struct ip);
> - int len, error = 0;
> + int rv, len, error = 0;
> struct route iproute;
> struct sockaddr_in *dst;
> - struct in_ifaddr *ia;
> struct tdb *tdb = NULL;
> u_long mtu;
>
> @@ -183,9 +182,17 @@ reroute:
> if ((IN_MULTICAST(ip->ip_dst.s_addr) ||
> (ip->ip_dst.s_addr == INADDR_BROADCAST)) &&
> imo != NULL && (ifp = if_get(imo->imo_ifidx)) != NULL) {
> + struct in_ifaddr *ia;
> +
> mtu = ifp->if_mtu;
> + KERNEL_LOCK();
> IFP_TO_IA(ifp, ia);
> + if (ip->ip_src.s_addr == INADDR_ANY && ia)
> + ip->ip_src = ia->ia_addr.sin_addr;
> + KERNEL_UNLOCK();
> } else {
> + struct in_ifaddr *ia;
> +
> if (ro->ro_rt == NULL)
> ro->ro_rt = rtalloc_mpath(&ro->ro_dst,
> &ip->ip_src.s_addr, ro->ro_tableid);
> @@ -206,17 +213,19 @@ reroute:
>
> if (ro->ro_rt->rt_flags & RTF_GATEWAY)
> dst = satosin(ro->ro_rt->rt_gateway);
> - }
>
> - /* Set the source IP address */
> - if (ip->ip_src.s_addr == INADDR_ANY && ia)
> - ip->ip_src = ia->ia_addr.sin_addr;
> + /* Set the source IP address */
> + if (ip
Re: bufcache KNF
On 11 April 2016 at 15:51, Mark Kettenis wrote: > > And prototypes with names in public headers are still an issue. > Interesting point. What's a public header though? Are files that end up in /usr/include/dev/pci/ public headers? If so, why do we install all of them indiscriminately?
Re: Keyboard resume (zzz) diff
> Date: Sun, 10 Apr 2016 23:33:59 -0700
> From: Mike Larkin
>
> Please test this diff on all machines that you can successfully (today)
> resume with 'zzz'. Please make sure that after resume, the keyboard
> still works.
>
> This diff re-enables the keyboard on resume. Previously, we were re-enabling
> the keyboard *controller* but apparently on some machines (notably the HP
> 4530s) the keyboard itself resumes in 'disabled' state and requires a
> re-enable.
>
> Keyboards are finicky, and this diff requires widespread testing before it
> can go in, to ensure no regressions. Please let me know if you have keyboard
> issues after resume.
FWIW, I think this is fairly low-risk. You should probably only do
this if the keyboard is actually enabled. Which means, only if
sc_enabled is set.
I also wonder if we also need the pckbd_set_xtscancode() call. That
shouldn't keep you from committing this as a first step though.
Also, you should always call config_activate_children(), not just only
in the DVACT_RESUME case.
> Index: dev/pckbc/pckbd.c
> ===
> RCS file: /cvs/src/sys/dev/pckbc/pckbd.c,v
> retrieving revision 1.42
> diff -u -p -a -u -r1.42 pckbd.c
> --- dev/pckbc/pckbd.c 4 May 2015 09:33:46 - 1.42
> +++ dev/pckbc/pckbd.c 11 Apr 2016 06:24:06 -
> @@ -123,13 +123,14 @@ static int pckbd_is_console(pckbc_tag_t,
>
> int pckbdprobe(struct device *, void *, void *);
> void pckbdattach(struct device *, struct device *, void *);
> +int pckbdactivate(struct device *, int);
>
> struct cfattach pckbd_ca = {
> sizeof(struct pckbd_softc),
> pckbdprobe,
> pckbdattach,
> NULL,
> - NULL
> + pckbdactivate
> };
>
> int pckbd_enable(void *, int);
> @@ -181,6 +182,29 @@ static int pckbd_decode(struct pckbd_int
> static int pckbd_led_encode(int);
>
> struct pckbd_internal pckbd_consdata;
> +
> +int
> +pckbdactivate(struct device *self, int act)
> +{
> + struct pckbd_softc *sc = (struct pckbd_softc *)self;
> + int rv = 0;
> + u_char cmd[1];
> +
> + switch(act) {
> + case DVACT_RESUME:
> + /*
> + * Some keyboards are not enabled after a reset,
> + * so make sure it is enabled now.
> + */
> + cmd[0] = KBC_ENABLE;
> + (void) pckbc_poll_cmd(sc->id->t_kbctag, sc->id->t_kbcslot,
> + cmd, 1, 0, NULL, 0);
> + rv = config_activate_children(self, act);
> + break;
> + }
> +
> + return (rv);
> +}
>
> int
> pckbd_set_xtscancode(pckbc_tag_t kbctag, pckbc_slot_t kbcslot,
>
>
Re: bufcache KNF
> From: "Ted Unangst" > Date: Mon, 11 Apr 2016 09:44:26 -0400 > > Martin Pieuchot wrote: > > ok? > > > > -int chillbufs(struct > > -bufcache *cache, struct bufqueue *queue, int64_t *queuepages); > > +int chillbufs(struct bufcache *, struct bufqueue *, int64_t *); > > fwiw i like names in prototypes, so i know what's going on. i know style says > that, but i think the advice is obsolete. At the same time I don't like prototypes that span multiple lines; you can't win. And prototypes with names in public headers are still an issue.
Re: bufcache KNF
Martin Pieuchot wrote: > ok? > > -int chillbufs(struct > -bufcache *cache, struct bufqueue *queue, int64_t *queuepages); > +int chillbufs(struct bufcache *, struct bufqueue *, int64_t *); fwiw i like names in prototypes, so i know what's going on. i know style says that, but i think the advice is obsolete.
Fewer ip{_6,}forwart_rt
Instead of rtfree(9)ing the cached route after using it, if it is a
multipath one, free it before.
Ok?
Index: netinet/ip_input.c
===
RCS file: /cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.269
diff -u -p -r1.269 ip_input.c
--- netinet/ip_input.c 29 Mar 2016 10:34:42 - 1.269
+++ netinet/ip_input.c 11 Apr 2016 13:21:44 -
@@ -1422,8 +1422,9 @@ ip_forward(struct mbuf *m, struct ifnet
rtableid = m->m_pkthdr.ph_rtableid;
+ rt = ipforward_rt.ro_rt;
sin = satosin(&ipforward_rt.ro_dst);
- if ((rt = ipforward_rt.ro_rt) == NULL ||
+ if (rt == NULL || ISSET(rt->rt_flags, RTF_MPATH) ||
ip->ip_dst.s_addr != sin->sin_addr.s_addr ||
rtableid != ipforward_rt.ro_tableid) {
if (ipforward_rt.ro_rt) {
@@ -1506,7 +1507,7 @@ ip_forward(struct mbuf *m, struct ifnet
goto freecopy;
}
if (!fake)
- goto freert;
+ return;
switch (error) {
@@ -1528,9 +1529,7 @@ ip_forward(struct mbuf *m, struct ifnet
code = ICMP_UNREACH_NEEDFRAG;
#ifdef IPSEC
- if (ipforward_rt.ro_rt) {
- struct rtentry *rt = ipforward_rt.ro_rt;
-
+ if (rt != NULL) {
if (rt->rt_rmx.rmx_mtu)
destmtu = rt->rt_rmx.rmx_mtu;
else {
@@ -1569,15 +1568,6 @@ ip_forward(struct mbuf *m, struct ifnet
freecopy:
if (fake)
m_tag_delete_chain(&mfake);
- freert:
-#ifndef SMALL_KERNEL
- if (ipmultipath && ipforward_rt.ro_rt &&
- (ipforward_rt.ro_rt->rt_flags & RTF_MPATH)) {
- rtfree(ipforward_rt.ro_rt);
- ipforward_rt.ro_rt = NULL;
- }
-#endif
- return;
}
int
Index: netinet6/ip6_forward.c
===
RCS file: /cvs/src/sys/netinet6/ip6_forward.c,v
retrieving revision 1.87
diff -u -p -r1.87 ip6_forward.c
--- netinet6/ip6_forward.c 29 Mar 2016 11:57:51 - 1.87
+++ netinet6/ip6_forward.c 11 Apr 2016 13:21:45 -
@@ -225,6 +225,7 @@ reroute:
* ip6_forward_rt.ro_dst.sin6_addr is equal to ip6->ip6_dst
*/
if (!rtisvalid(ip6_forward_rt.ro_rt) ||
+ ISSET(ip6_forward_rt.ro_rt->rt_flags, RTF_MPATH) ||
ip6_forward_rt.ro_tableid != rtableid) {
if (ip6_forward_rt.ro_rt) {
rtfree(ip6_forward_rt.ro_rt);
@@ -248,6 +249,7 @@ reroute:
return;
}
} else if (!rtisvalid(ip6_forward_rt.ro_rt) ||
+ ISSET(ip6_forward_rt.ro_rt->rt_flags, RTF_MPATH) ||
!IN6_ARE_ADDR_EQUAL(&ip6->ip6_dst, &dst->sin6_addr) ||
ip6_forward_rt.ro_tableid != rtableid) {
if (ip6_forward_rt.ro_rt) {
@@ -303,7 +305,7 @@ reroute:
icmp6_error(mcopy, ICMP6_DST_UNREACH,
ICMP6_DST_UNREACH_BEYONDSCOPE, 0);
m_freem(m);
- goto freert;
+ goto out;
}
#ifdef IPSEC
@@ -346,7 +348,7 @@ reroute:
/* Callee frees mbuf */
error = ipsp_process_packet(m, tdb, AF_INET6, 0);
m_freem(mcopy);
- goto freert;
+ goto out;
}
#endif /* IPSEC */
@@ -387,7 +389,7 @@ reroute:
icmp6_error(mcopy, ICMP6_DST_UNREACH,
ICMP6_DST_UNREACH_ADDR, 0);
m_freem(m);
- goto freert;
+ goto out;
}
type = ND_REDIRECT;
}
@@ -434,7 +436,7 @@ reroute:
icmp6_error(mcopy, ICMP6_PACKET_TOO_BIG, 0,
ifp->if_mtu);
m_freem(m);
- goto freert;
+ goto out;
}
error = nd6_output(ifp, m, dst, rt);
@@ -454,12 +456,12 @@ reroute:
senderr:
#endif
if (mcopy == NULL)
- goto freert;
+ goto out;
switch (error) {
case 0:
if (type == ND_REDIRECT) {
icmp6_redirect_output(mcopy, rt);
- goto freert;
+ goto out;
}
goto freecopy;
@@ -481,17 +483,10 @@ senderr:
break;
}
icmp6_error(mcopy, type, code, 0);
- goto freert;
+ goto out;
- freecopy:
+freecopy:
m_freem(mcopy);
- freert:
-#ifndef SMALL_KERNEL
- if (ip6_multipath && ip6_forward_rt.ro_rt &&
- (ip6_forward_rt.ro_rt->rt_flags & RTF_MPATH)) {
- rtfree(ip6_forward_rt.ro_rt);
- ip6_forward_rt.ro_rt = NULL;
- }
-#endif
+out:
Kill in_rtaddr()
``ipforward_rt'' is going away but rather than sending a big diff,
here's the first step.
This diff replaces in_rtaddr() by rtalloc(9). Note that since the code
here is interested in rt_ifa it has to call rtisvalid(9) to prevent a
NULL-dereference in case of a stale ``ifa''.
For the same reason rtfree(9) has to be called *after* the pointer has
been dereferenced. Because the reference on ``rt'' acts as a proxy for
``ifa''.
ok?
Index: netinet/ip_input.c
===
RCS file: /cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.269
diff -u -p -r1.269 ip_input.c
--- netinet/ip_input.c 29 Mar 2016 10:34:42 - 1.269
+++ netinet/ip_input.c 11 Apr 2016 13:10:51 -
@@ -1013,6 +1013,8 @@ int
ip_dooptions(struct mbuf *m, struct ifnet *ifp)
{
struct ip *ip = mtod(m, struct ip *);
+ unsigned int rtableid = m->m_pkthdr.ph_rtableid;
+ struct rtentry *rt;
struct sockaddr_in ipaddr;
u_char *cp;
struct ip_timestamp ipt;
@@ -1108,19 +1110,31 @@ ip_dooptions(struct mbuf *m, struct ifne
m->m_pkthdr.ph_rtableid))) == NULL)
ia = ifatoia(ifa_ifwithnet(sintosa(&ipaddr),
m->m_pkthdr.ph_rtableid));
- } else
+ if (ia == NULL) {
+ type = ICMP_UNREACH;
+ code = ICMP_UNREACH_SRCFAIL;
+ goto bad;
+ }
+ memcpy(cp + off, &ia->ia_addr.sin_addr,
+ sizeof(struct in_addr));
+ cp[IPOPT_OFFSET] += sizeof(struct in_addr);
+ } else {
/* keep packet in the virtual instance */
- ia = ip_rtaddr(ipaddr.sin_addr,
- m->m_pkthdr.ph_rtableid);
- if (ia == NULL) {
- type = ICMP_UNREACH;
- code = ICMP_UNREACH_SRCFAIL;
- goto bad;
+ rt = rtalloc(sintosa(&ipaddr), RT_RESOLVE,
+ rtableid);
+ if (!rtisvalid(rt)) {
+ type = ICMP_UNREACH;
+ code = ICMP_UNREACH_SRCFAIL;
+ rtfree(rt);
+ goto bad;
+ }
+ ia = ifatoia(rt->rt_ifa);
+ memcpy(cp + off, &ia->ia_addr.sin_addr,
+ sizeof(struct in_addr));
+ rtfree(rt);
+ cp[IPOPT_OFFSET] += sizeof(struct in_addr);
}
ip->ip_dst = ipaddr.sin_addr;
- memcpy(cp + off, &ia->ia_addr.sin_addr,
- sizeof(struct in_addr));
- cp[IPOPT_OFFSET] += sizeof(struct in_addr);
/*
* Let ip_intr's mcast routing check handle mcast pkts
*/
@@ -1152,16 +1166,17 @@ ip_dooptions(struct mbuf *m, struct ifne
* use the incoming interface (should be same).
* Again keep the packet inside the virtual instance.
*/
- if ((ia = ifatoia(ifa_ifwithaddr(sintosa(&ipaddr),
- m->m_pkthdr.ph_rtableid))) == NULL &&
- (ia = ip_rtaddr(ipaddr.sin_addr,
- m->m_pkthdr.ph_rtableid)) == NULL) {
+ rt = rtalloc(sintosa(&ipaddr), RT_RESOLVE, rtableid);
+ if (!rtisvalid(rt)) {
type = ICMP_UNREACH;
code = ICMP_UNREACH_HOST;
+ rtfree(rt);
goto bad;
}
+ ia = ifatoia(rt->rt_ifa);
memcpy(cp + off, &ia->ia_addr.sin_addr,
sizeof(struct in_addr));
+ rtfree(rt);
cp[IPOPT_OFFSET] += sizeof(struct in_addr);
break;
@@ -1234,34 +1249,6 @@ bad:
icmp_error(m, type, code, 0, 0);
ipstat.ips_badoptions++;
return (1);
-}
-
-/*
- * Given address of next destination (final or next hop),
- * return internet address info of interface to be used to get there.
- */
-struct in_ifaddr *
-ip_rtaddr(struct in_addr dst, u_int rtableid)
-{
- struct sockaddr_in *sin;
-
- sin = satosin(&ipforward_rt.ro_dst);
-
- if (ipforwa
Re: rtableid in ip6_input()
Martin Pieuchot writes: > This variable is also used for route lookups, so it must always be > assigned. > > ok? ok jca@ > Index: netinet6//ip6_input.c > === > RCS file: /cvs/src/sys/netinet6/ip6_input.c,v > retrieving revision 1.156 > diff -u -p -r1.156 ip6_input.c > --- netinet6//ip6_input.c 29 Mar 2016 11:57:51 - 1.156 > +++ netinet6//ip6_input.c 11 Apr 2016 11:14:54 - > @@ -416,9 +416,7 @@ ip6_input(struct mbuf *m) > goto hbhcheck; > } > > -#if NPF > 0 > rtableid = m->m_pkthdr.ph_rtableid; > -#endif > > /* >* Unicast check > -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: rtableid in ip6_input()
On 11 April 2016 at 13:16, Martin Pieuchot wrote: > This variable is also used for route lookups, so it must always be > assigned. > > ok? > OK. Initially there was some pf glue that got removed.
rtableid in ip6_input()
This variable is also used for route lookups, so it must always be assigned. ok? Index: netinet6//ip6_input.c === RCS file: /cvs/src/sys/netinet6/ip6_input.c,v retrieving revision 1.156 diff -u -p -r1.156 ip6_input.c --- netinet6//ip6_input.c 29 Mar 2016 11:57:51 - 1.156 +++ netinet6//ip6_input.c 11 Apr 2016 11:14:54 - @@ -416,9 +416,7 @@ ip6_input(struct mbuf *m) goto hbhcheck; } -#if NPF > 0 rtableid = m->m_pkthdr.ph_rtableid; -#endif /* * Unicast check
Re: add "route" promise to pledge.2
On Thu, Apr 07, 2016 at 04:01:52PM -0400, Rob Pierce wrote: > I wasn't sure of where to put it in the list. > > How is this? > > Rob > Hi, Sorry for the late reply. We have discuted a bit the proper way to document "route" promise. Your diff is a good starting point for "route" promise. The first sentence was reformulated by ingo@, and I have tried to enhance it a bit with description of the behaviour (read-only operations) and mentions sysctls interface for routing table observation. I hope someone more familiar than me with these ioctls and sysctls interfaces for routing will confirm the exactness of the description (or provide a more precise description). I added in Cc people using "route" promise (in dhclient, iked, bgpd, dhcpcd, route6d, rtadvd), which should be competent for that. Comments ? -- Sebastien Marie Index: pledge.2 === RCS file: /cvs/src/lib/libc/sys/pledge.2,v retrieving revision 1.28 diff -u -p -r1.28 pledge.2 --- pledge.210 Apr 2016 18:52:07 - 1.28 +++ pledge.211 Apr 2016 09:05:09 - @@ -80,7 +80,8 @@ Only the and .Dv FIONBIO operations are allowed by default. -Use of the "tty" and "ioctl" promises receive more ioctl requests. +The "audio", "ioctl", "pf", "route" and "tty" promises permit more ioctl +requests. .Pp .It Xr chmod 2 .It Xr fchmod 2 @@ -495,6 +496,25 @@ process: .Xr setrlimit 2 , .Xr getpriority 2 , .Xr setpriority 2 . +.It Va "route" +Allows a subset of read-only +.Xr ioctl 2 +operations on network interfaces: +.Pp +.Dv SIOCGIFADDR , +.Dv SIOCGIFFLAGS , +.Dv SIOCGIFMETRIC , +.Dv SIOCGIFGMEMB , +.Dv SIOCGIFRDOMAIN , +.Dv SIOCGIFDSTADDR_IN6 , +.Dv SIOCGIFNETMASK_IN6 , +.Dv SIOCGNBRINFO_IN6 , +.Dv SIOCGIFINFO_IN6 , +.Dv SIOCGIFMEDIA . +.Pp +And allows a subset of +.Xr sysctl 3 +interfaces for routing table observation. .It Va "pf" Allows a subset of .Xr ioctl 2
