Re: monop(6): drop the conditional shrt macro definitions
On Sat, Jun 10, 2017 at 2:45 PM, Miod Vallat wrote: >> Hi tech@, >> >> Drop the conditional shrt macro definitions and use short everywhere. > > Why not use uint8_t then? Isn't the point of the code to *not* use an unsigned type? Philip Guenther
Re: monop(6): drop the conditional shrt macro definitions
> Hi tech@, > > Drop the conditional shrt macro definitions and use short everywhere. Why not use uint8_t then?
monop(6): drop the conditional shrt macro definitions
Hi tech@,
Drop the conditional shrt macro definitions and use short everywhere.
Comments? OK?
Index: games/monop/houses.c
===
RCS file: /cvs/src/games/monop/houses.c,v
retrieving revision 1.10
diff -u -p -r1.10 houses.c
--- games/monop/houses.c8 Jan 2016 18:20:33 - 1.10
+++ games/monop/houses.c10 Jun 2017 21:22:54 -
@@ -110,7 +110,7 @@ buy_h(MON *mnp)
int i;
MON *mp;
int price;
- shrtinput[3],temp[3];
+ short input[3],temp[3];
int tot, tot2;
PROP*pp;
int nhous, nhot;
@@ -245,7 +245,7 @@ sell_h(MON *mnp)
int i;
MON *mp;
int price;
- shrtinput[3],temp[3];
+ short input[3],temp[3];
int tot;
PROP*pp;
Index: games/monop/monop.h
===
RCS file: /cvs/src/games/monop/monop.h,v
retrieving revision 1.8
diff -u -p -r1.8 monop.h
--- games/monop/monop.h 8 Jan 2016 18:19:47 - 1.8
+++ games/monop/monop.h 10 Jun 2017 21:22:54 -
@@ -32,11 +32,6 @@
* @(#)monop.h 8.1 (Berkeley) 5/31/93
*/
-#ifdef __CHAR_UNSIGNED__
-#defineshrtshort
-#else
-#defineshrtchar
-#endif
#defineboolint8_t
#defineTRUE(1)
@@ -72,8 +67,8 @@
struct sqr_st {/* structure for square
*/
char*name; /* place name */
- shrtowner; /* owner number */
- shrttype; /* place type */
+ short owner; /* owner number */
+ short type; /* place type */
struct prp_st *desc; /* description struct */
int cost; /* cost */
};
@@ -82,10 +77,10 @@ typedef struct sqr_st SQUARE;
struct mon_st {/* monopoly description structure
*/
char*name; /* monop. name (color) */
- shrtowner; /* owner of monopoly*/
- shrtnum_in; /* # in monopoly*/
- shrtnum_own;/* # owned (-1: not poss. monop)*/
- shrth_cost; /* price of houses */
+ short owner; /* owner of monopoly*/
+ short num_in; /* # in monopoly*/
+ short num_own;/* # owned (-1: not poss. monop)*/
+ short h_cost; /* price of houses */
char*not_m; /* name if not monopoly */
char*mon_n; /* name if a monopoly */
charsqnums[3]; /* Square numbers (used to init)*/
@@ -101,8 +96,8 @@ typedef struct mon_stMON;
struct prp_st {/* property description structure
*/
boolmorg; /* set if mortgaged */
boolmonop; /* set if monopoly */
- shrtsquare; /* square description */
- shrthouses; /* number of houses */
+ short square; /* square description */
+ short houses; /* number of houses */
MON *mon_desc; /* name of color*/
int rent[6];/* rents*/
};
@@ -116,11 +111,11 @@ typedef struct own_st OWN;
struct plr_st {/* player description structure
*/
char*name; /* owner name */
- shrtnum_gojf; /* # of get-out-of-jail-free's */
- shrtnum_rr; /* # of railroads owned */
- shrtnum_util; /* # of water works/elec. co. */
- shrtloc;/* location on board*/
- shrtin_jail;/* count of turns in jail */
+ short num_gojf; /* # of get-out-of-jail-free's */
+ short num_rr; /* # of railroads owned */
+ short num_util; /* # of water works/elec. co. */
+ short loc;/* location on board*/
+ short in_jail;/* count of turns in jail */
int money; /* amount of money */
OWN *own_list; /* start of property list */
};
@@ -183,7 +178,7 @@ voidprinthold(int);
/* prop.c */
v
Re: htpasswd: use crypt_newhash instead of bcrypt API
Michal Mazurek wrote:
> When talking about this with mulander@ it came out that the docs could
> use a touch.
>
> The commit message for the diff that didn't update the docs was:
>
> permit "bcrypt" as an alias for "blowfish". this is, after all, what
> 99% of the world calls it.
> allow just "bcrypt" without params to mean auto-tune ("bcrypt,a").
> default remains 8 rounds (for now)
>
> Comments? OK?
>
> Index: lib/libc/crypt/crypt_checkpass.3
> ===
> RCS file: /cvs/src/lib/libc/crypt/crypt_checkpass.3,v
> retrieving revision 1.9
> diff -u -p -r1.9 crypt_checkpass.3
> --- lib/libc/crypt/crypt_checkpass.3 23 Jul 2015 22:20:02 - 1.9
> +++ lib/libc/crypt/crypt_checkpass.3 6 Jun 2017 19:06:59 -
> @@ -58,17 +58,29 @@ The provided
> .Fa password
> is randomly salted and hashed and stored in
> .Fa hash .
> +.Fa hash
> +must already be allocated, and
> +.Fa hashsize
> +must contain its size, which cannot be less than 61 bytes.
that's an implementation detail. if we're advising a limit, i think we
should say 128 or so.
i think the rest is fine.
Re: sppp(4)/pppoe(4) dynamic address hack
On 10/06/17(Sat) 17:45, Stefan Sperling wrote:
> On Sat, Jun 10, 2017 at 04:14:16PM +0200, Martin Pieuchot wrote:
> > I'm not fan of your approach because it makes pppoe(4) special, does
> > umb(4) will need 'dynaddr' and 'dyntest' too?
>
> I consider this a quirk in sppp(4), not pppoe or p2p interfaces in general.
> sppp(4) is using these DYN flags which we're toggling. The new ifconfig
> commands are documented in ifconfig(8)'s 'SPPP' section for that reason.
> umb(4) does not use sppp(4).
>
> In my mind, making some IP addresses act as magic toggles for internal
> sppp(4) state flags is a worse hack than mine. The old hack even has
> an XXX comment saying it should be removed, added by its author.
I agree your hack is better but it's still a hack, limited to sppp(4).
> > So IMHO if what you want is fix the EEXIST, I'd do that.
>
> I see where you are coming from. Fixing the routing table as you suggest
> would make the old hack work again and also bring other benefits.
There's no need to fix the routing table, we could something like:
- if (hisaddr == 1) {
+ if (hisaddr < 10) {
Now I think you have a good point that using a flag is better than a
magic address. But I think the ifconfig(8) interface should be simpler.
What about reusing the 'autoconf' toggle? Could we say:
# ifconfig pppoe0 inet autoconf
Instead of introducing 'dynaddr' and 'dyndest'? Would it be a problem
if somebody wants to run dhclient(8) on top of pppoe(4)?
Re: [PATCH] re: disable PCIe ASPM and ECPM (CLKREQ)
On Sat, Jun 10, 2017 at 11:06:43PM +0800, Kevin Lo wrote:
> On Wed, May 17, 2017 at 03:58:16PM +0800, Kevin Lo wrote:
> >
> > >From FreeBSD (r227593, r307982):
> > More and more RealTek controllers started to implement EEE feature.
> > Vendor driver seems to load a kind of firmware for EEE with
> > additional PHY fixups. It is known that the EEE feature may need
> > ASPM support. Unfortunately there is no documentation for EEE of
> > the controller so enabling ASPM may cause more problems.
> >
> > The Realtek vendor driver [1] also disables ASPM and clock request.
> > While here, add a define for the ECPM (Enable Clock Power Management) bit.
> >
> > Tested with:
> > re0 at pci3 dev 0 function 0 "Realtek 8168" rev 0x0c: RTL8168G/8111G
> > (0x4c00), msi, address 44:8a:5b:39:0a:25
> > rgephy0 at re0 phy 7: RTL8251 PHY, rev. 0
> >
> > re0 at pci2 dev 0 function 0 "Realtek 8168" rev 0x03: RTL8168D/8111D
> > (0x2800), msi, address bc:ae:c5:d6:ac:a3
> > rgephy0 at re0 phy 7: RTL8169S/8110S/8211 PHY, rev. 2
> >
> > [1]
> > http://www.realtek.com.tw/downloads/downloadsView.aspx?Langid=1&PNid=13&PFid=5&Level=5&Conn=4&DownTypeID=3&GetDown=false#2
>
> Updated diff which removes rl_expcap from the softc; spotted by stsp@
Also works on:
re0 at pci1 dev 0 function 0 "Realtek 8101E" rev 0x05: RTL8105E (0x4080), msi,
address 04:7d:7b:21:ad:9d
rlphy0 at re0 phy 7: RTL8201E 10/100 PHY, rev. 2
1:0:0: Realtek 8101E
0x: Vendor ID: 10ec Product ID: 8136
0x0004: Command: 0007 Status: 0010
0x0008: Class: 02 Subclass: 00 Interface: 00 Revision: 05
0x000c: BIST: 00 Header Type: 00 Latency Timer: 00 Cache Line Size: 10
0x0010: BAR io addr: 0x3000/0x0100
0x0014: BAR empty ()
0x0018: BAR mem prefetchable 64bit addr: 0x90004000/0x1000
0x0020: BAR mem prefetchable 64bit addr: 0x9000/0x4000
0x0028: Cardbus CIS:
0x002c: Subsystem Vendor ID: 1025 Product ID: 058f
0x0030: Expansion ROM Base Address:
0x0038:
0x003c: Interrupt Pin: 01 Line: 0b Min Gnt: 00 Max Lat: 00
0x0040: Capability 0x01: Power Management
State: D0 PME# enabled
0x0050: Capability 0x05: Message Signalled Interrupts (MSI)
0x0070: Capability 0x10: PCI Express
Link Speed: 2.5 / 2.5 GT/s Link Width: x1 / x1
0x0100: Enhanced Capability 0x01: Advanced Error Reporting
0x0140: Enhanced Capability 0x02: Virtual Channel Capability
0x0160: Enhanced Capability 0x03: Device Serial Number
0x00b0: Capability 0x11: Extended Message Signalled Interrupts (MSI-X)
0x00d0: Capability 0x03: Vital Product Data (VPD)
OK by me.
> Index: sys/dev/pci/if_re_pci.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_re_pci.c,v
> retrieving revision 1.50
> diff -u -p -u -p -r1.50 if_re_pci.c
> --- sys/dev/pci/if_re_pci.c 28 Dec 2015 05:49:15 - 1.50
> +++ sys/dev/pci/if_re_pci.c 10 Jun 2017 15:03:39 -
> @@ -128,6 +128,8 @@ re_pci_attach(struct device *parent, str
> pci_chipset_tag_t pc = pa->pa_pc;
> pci_intr_handle_t ih;
> const char *intrstr = NULL;
> + pcireg_treg;
> + int offset;
>
> pci_set_powerstate(pa->pa_pc, pa->pa_tag, PCI_PMCSR_STATE_D0);
>
> @@ -172,8 +174,14 @@ re_pci_attach(struct device *parent, str
>* PCI Express check.
>*/
> if (pci_get_capability(pc, pa->pa_tag, PCI_CAP_PCIEXPRESS,
> - NULL, NULL))
> + &offset, NULL)) {
> + /* Disable PCIe ASPM and ECPM. */
> + reg = pci_conf_read(pc, pa->pa_tag, offset + PCI_PCIE_LCSR);
> + reg &= ~(PCI_PCIE_LCSR_ASPM_L0S | PCI_PCIE_LCSR_ASPM_L1 |
> + PCI_PCIE_LCSR_ECPM);
> + pci_conf_write(pc, pa->pa_tag, offset + PCI_PCIE_LCSR, reg);
> sc->rl_flags |= RL_FLAG_PCIE;
> + }
>
> if (!(PCI_VENDOR(pa->pa_id) == PCI_VENDOR_REALTEK &&
> PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_REALTEK_RT8139)) {
> Index: sys/dev/pci/pcireg.h
> ===
> RCS file: /cvs/src/sys/dev/pci/pcireg.h,v
> retrieving revision 1.53
> diff -u -p -u -p -r1.53 pcireg.h
> --- sys/dev/pci/pcireg.h 25 Mar 2017 07:33:46 - 1.53
> +++ sys/dev/pci/pcireg.h 10 Jun 2017 15:03:39 -
> @@ -574,6 +574,7 @@ typedef u_int8_t pci_revision_t;
> #define PCI_PCIE_LCSR_ASPM_L0S 0x0001
> #define PCI_PCIE_LCSR_ASPM_L10x0002
> #define PCI_PCIE_LCSR_ES 0x0080
> +#define PCI_PCIE_LCSR_ECPM 0x0100
> #define PCI_PCIE_SLCAP 0x14
> #define PCI_PCIE_SLCAP_ABP 0x0001
> #define PCI_PCIE_SLCAP_PCP 0x0002
>
Re: sppp(4)/pppoe(4) dynamic address hack
On Sat, Jun 10, 2017 at 04:14:16PM +0200, Martin Pieuchot wrote: > If there's a real need for such arguments, because somebody use pppoe(4) > with static IP/route then why not. But that's not what you said. What > you said is that the current hack of 0.0.0.0+0.0.0.1 is broken. Static IP/route setups with pppoe(4) do exist. Just buy business DSL. The problem I want to fix is allowing more than one dynamic connection. Static setups work either way, with or without my fix. In either case, just configure your real addresses upfront to get a fully static setup. > I'm not fan of your approach because it makes pppoe(4) special, does > umb(4) will need 'dynaddr' and 'dyntest' too? I consider this a quirk in sppp(4), not pppoe or p2p interfaces in general. sppp(4) is using these DYN flags which we're toggling. The new ifconfig commands are documented in ifconfig(8)'s 'SPPP' section for that reason. umb(4) does not use sppp(4). In my mind, making some IP addresses act as magic toggles for internal sppp(4) state flags is a worse hack than mine. The old hack even has an XXX comment saying it should be removed, added by its author. > So IMHO if what you want is fix the EEXIST, I'd do that. I see where you are coming from. Fixing the routing table as you suggest would make the old hack work again and also bring other benefits. You were saying your old diff was "the first of 14 steps." That project seems to be substantially more involved than my proposed patch, which took me just a few hours. It does not sound like a project I could start working on now (I am not even done reading xhci.c, let alone actually hacking on it). I have proposed an easy fix which works and which does not preclude future work being done on the routing table. But I won't insist. I don't expect leaving sppp(4) in a broken state will suddenly motivate anyone to do substantial work on the routing table, though. My guess is that sppp(4) will just stay broken for multiple connections.
Re: [PATCH] re: disable PCIe ASPM and ECPM (CLKREQ)
On Wed, May 17, 2017 at 03:58:16PM +0800, Kevin Lo wrote:
>
> >From FreeBSD (r227593, r307982):
> More and more RealTek controllers started to implement EEE feature.
> Vendor driver seems to load a kind of firmware for EEE with
> additional PHY fixups. It is known that the EEE feature may need
> ASPM support. Unfortunately there is no documentation for EEE of
> the controller so enabling ASPM may cause more problems.
>
> The Realtek vendor driver [1] also disables ASPM and clock request.
> While here, add a define for the ECPM (Enable Clock Power Management) bit.
>
> Tested with:
> re0 at pci3 dev 0 function 0 "Realtek 8168" rev 0x0c: RTL8168G/8111G
> (0x4c00), msi, address 44:8a:5b:39:0a:25
> rgephy0 at re0 phy 7: RTL8251 PHY, rev. 0
>
> re0 at pci2 dev 0 function 0 "Realtek 8168" rev 0x03: RTL8168D/8111D
> (0x2800), msi, address bc:ae:c5:d6:ac:a3
> rgephy0 at re0 phy 7: RTL8169S/8110S/8211 PHY, rev. 2
>
> [1]
> http://www.realtek.com.tw/downloads/downloadsView.aspx?Langid=1&PNid=13&PFid=5&Level=5&Conn=4&DownTypeID=3&GetDown=false#2
Updated diff which removes rl_expcap from the softc; spotted by stsp@
Index: sys/dev/pci/if_re_pci.c
===
RCS file: /cvs/src/sys/dev/pci/if_re_pci.c,v
retrieving revision 1.50
diff -u -p -u -p -r1.50 if_re_pci.c
--- sys/dev/pci/if_re_pci.c 28 Dec 2015 05:49:15 - 1.50
+++ sys/dev/pci/if_re_pci.c 10 Jun 2017 15:03:39 -
@@ -128,6 +128,8 @@ re_pci_attach(struct device *parent, str
pci_chipset_tag_t pc = pa->pa_pc;
pci_intr_handle_t ih;
const char *intrstr = NULL;
+ pcireg_treg;
+ int offset;
pci_set_powerstate(pa->pa_pc, pa->pa_tag, PCI_PMCSR_STATE_D0);
@@ -172,8 +174,14 @@ re_pci_attach(struct device *parent, str
* PCI Express check.
*/
if (pci_get_capability(pc, pa->pa_tag, PCI_CAP_PCIEXPRESS,
- NULL, NULL))
+ &offset, NULL)) {
+ /* Disable PCIe ASPM and ECPM. */
+ reg = pci_conf_read(pc, pa->pa_tag, offset + PCI_PCIE_LCSR);
+ reg &= ~(PCI_PCIE_LCSR_ASPM_L0S | PCI_PCIE_LCSR_ASPM_L1 |
+ PCI_PCIE_LCSR_ECPM);
+ pci_conf_write(pc, pa->pa_tag, offset + PCI_PCIE_LCSR, reg);
sc->rl_flags |= RL_FLAG_PCIE;
+ }
if (!(PCI_VENDOR(pa->pa_id) == PCI_VENDOR_REALTEK &&
PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_REALTEK_RT8139)) {
Index: sys/dev/pci/pcireg.h
===
RCS file: /cvs/src/sys/dev/pci/pcireg.h,v
retrieving revision 1.53
diff -u -p -u -p -r1.53 pcireg.h
--- sys/dev/pci/pcireg.h25 Mar 2017 07:33:46 - 1.53
+++ sys/dev/pci/pcireg.h10 Jun 2017 15:03:39 -
@@ -574,6 +574,7 @@ typedef u_int8_t pci_revision_t;
#define PCI_PCIE_LCSR_ASPM_L0S 0x0001
#define PCI_PCIE_LCSR_ASPM_L1 0x0002
#define PCI_PCIE_LCSR_ES 0x0080
+#define PCI_PCIE_LCSR_ECPM 0x0100
#define PCI_PCIE_SLCAP 0x14
#define PCI_PCIE_SLCAP_ABP 0x0001
#define PCI_PCIE_SLCAP_PCP 0x0002
Re: sppp(4)/pppoe(4) dynamic address hack
On 10/06/17(Sat) 11:55, Stefan Sperling wrote: > On Sat, Jun 10, 2017 at 11:47:27AM +0200, Martin Pieuchot wrote: > > On 10/06/17(Sat) 08:35, Stefan Sperling wrote: > > > On Fri, Jun 09, 2017 at 05:37:44PM +0100, Jason McIntyre wrote: > > > > why do you have to specify 0.0.0.0 *and* dynaddr? > > > [...] > > > Regardless, you probably *do* want a dummy address. An address is needed > > > in order to add a dummy default route which will be updated once the pppoe > > > interface comes up (more details forthcoming in my reply to naddy's > > > question). > > > > So why not fix the problem? How many years are we going to continue > > to add workaround for p2p interfaces? > > > > I send a diff last year to start doing that: > > http://marc.info/?l=openbsd-tech&m=146539593821612&w=2 > > > > Since nobody jumped in the boat and I already have ETOOMANYTHING to do > > I dropped the diff. > > > > That's cool. > But I don't think it conflicts with the change I'm trying to make. > Does it? No it does not, it makes it obsolete because it fixes the problem. Currently there are implicit 'dynaddr' and 'dyndest' arguments. They are just broken. If there's a real need for such arguments, because somebody use pppoe(4) with static IP/route then why not. But that's not what you said. What you said is that the current hack of 0.0.0.0+0.0.0.1 is broken. I'm not fan of your approach because it makes pppoe(4) special, does umb(4) will need 'dynaddr' and 'dyntest' too? Now the 0.0.0.1 hack is broken since 5.8, why not just fix it? EEXIST means the routing table already found an entry with this destination. This is due to the fact that a host route with destination 0.0.0.1 and gateway 0.0.0.0 already exists. Theses values don't matter and I don't see anything in if_spppsubr.c that check for the gateway being 0.0.0.1 So IMHO if what you want is fix the EEXIST, I'd do that. If what you want is fix the crappy config of pppoe(4)/umb(4)/any p2p interface, you have my diff. If what you want is to *not have* dynamic configuration on your pppoe(4) then your diff might make sense.
Re: sed(1): missing NUL in pattern space
On Fri, Jun 09, 2017 at 07:58:48AM +, kshe wrote:
> Hi,
>
> There is an ongoing confusion in sed's source about the concept of EOL:
> the code contradicts itself as to whether it should be determined by a
> trailing NUL or by looking up the `len' field of the SPACE structure.
>
> At least two commands (`l' and `s') assume that the pattern space is
> always NUL-terminated. However, at least two other commands (`c' and
> `D') do not comply with that assumption, modifying it by merely updating
> its length attribute. As a result, using `l' or `s' after `c' or `D'
> produces incorrect output.
>
> For instance, in the following excerpt, `l' should print `bb$':
>
> $ printf 'a\nbb\n' | sed '${l;q;};N;D'
> $
> bb
>
> For the same underlying reason, it also disregards the deletion of the
> pattern space that `c' is supposed to entail:
>
> $ echo abc | sed 'c\
> > text
> > l'
> text
> abc$
>
> The substitute command can likewise get confused and add an extra
> character after a replacement:
>
> $ printf 'a\nbb\n' | sed '${s/$/x/;q;};N;D'
> bbxb
>
> Note how this does not happen when the substitution pattern matches more
> than the empty string:
>
> $ printf 'a\nbb\n' | sed '${s/.$/&x/;q;};N;D'
> bbx
>
> After an empty match, the character that `s' adds to the pattern space
> depends on how memmove(3) modified it beforehand. Here's a very
> simplified version of the original sequence of commands from which I
> discovered these bugs, where `i' appear instead of `b':
>
> $ printf 'a\nbb\n' | sed '${s/$/x/;q;};N;s/a/ZZi/;D'
> bbxi
>
> Similarily, when one believes the pattern space to be empty after a `c'
> command, something like `s/.*//' will magically revive one character
> from that emptiness:
>
> $ echo abc | sed 'c\
> > text
> > s/.*//'
> text
> a
>
> As I came across these in an actual script that I was writing, I was
> quite amazed to find out that they went unnoticed for so long, without
> anyone ever trying to see what `l' does after `c' or `D'. Indeed, the
> `l' command is broken since revision 1.1 of process.c, making that bug
> older than I am. It has also been present in the other BSDs, but:
>
> o FreeBSD fixed it by accident in 2004 by adding support for
> multibyte encodings;
>
> o NetBSD fixed it by accident in 2014 by importing FreeBSD's sed
> (merging their changes afterwards, but that didn't reintroduce
> the bug, since they hadn't touched that part of the code).
>
> The more serious bug regarding empty matches in the substitute command
> was introduced in OpenBSD in 2011, when schwarze@ decided to rewrite its
> main loop, intending to "fix multiple issues regarding the replacement
> of zero-length strings", but apparently with the unfortunate side effect
> of introducing a new issue regarding the replacement of zero-length
> strings. Finally, this commit was adopted by FreeBSD in 2016 when they
> synchronised some of their code with OpenBSD's to ease importing other
> fixes from it.
>
> Thus, OpenBSD still has the old `l' bug as well as the more recent `s'
> one, FreeBSD only has the latter, and since 2014 NetBSD is not longer
> affected by any of these, although the fact that `c' and `D' omit to
> NUL-terminate the pattern space could be considered a bug in itself,
> since at least one piece of code present for 20 years in their tree was
> making the contrary assumption. So who knows where else it was made?
> Actually, with all due respect, probably not even the original
> developers: many of their choices suggest that the end of the pattern
> space was not always meant to be encoded as a trailing NUL, but they
> nevertheless designed the function `lputs' to only take a simple string
> parameter, by which no separate length information could ever be
> accessible.
>
> I therefore have the strong feeling that this is not the last bug of
> BSD's sed. The remaining ones may be hidden in dark corners, but I do
> expect them to show up at some point in the future, considering how
> fragile the existing code looks. The overall situation also prevents
> any significant improvement to be successfully conducted, as changing
> more than two lines will most likely lead to subtle regressions; see the
> last few commits at
>
> https://svnweb.freebsd.org/base/head/usr.bin/sed/process.c
>
> and
>
> http://cvsweb.netbsd.org/cgi-bin/cvsweb.cgi/src/usr.bin/sed/process.c
>
> for recent examples of such failed attempts.
>
> Anyway, until I digress even more, here is a patch for that one, forcing
> `c' and `D' to put a NUL where `l' and `s' expect one to be. By the
> way, whether the fault is on one side or the other is actually unclear.
> It does seem redundant to enforce such a convention when one already has
> access to the length of the corresponding string. As a matter of fact,
> the overwhelming majority of the code does
Re: sppp(4)/pppoe(4) dynamic address hack
On Sat, Jun 10, 2017 at 11:47:27AM +0200, Martin Pieuchot wrote: > On 10/06/17(Sat) 08:35, Stefan Sperling wrote: > > On Fri, Jun 09, 2017 at 05:37:44PM +0100, Jason McIntyre wrote: > > > why do you have to specify 0.0.0.0 *and* dynaddr? > > [...] > > Regardless, you probably *do* want a dummy address. An address is needed > > in order to add a dummy default route which will be updated once the pppoe > > interface comes up (more details forthcoming in my reply to naddy's > > question). > > So why not fix the problem? How many years are we going to continue > to add workaround for p2p interfaces? > > I send a diff last year to start doing that: > http://marc.info/?l=openbsd-tech&m=146539593821612&w=2 > > Since nobody jumped in the boat and I already have ETOOMANYTHING to do > I dropped the diff. > That's cool. But I don't think it conflicts with the change I'm trying to make. Does it? My diff changes how userland enables/disables dynamic address configuration, and removes the need to configure a specific magic address. sppp(4) is messing with address and routes in the same way, regardless.
Re: sppp(4)/pppoe(4) dynamic address hack
On 10/06/17(Sat) 08:35, Stefan Sperling wrote: > On Fri, Jun 09, 2017 at 05:37:44PM +0100, Jason McIntyre wrote: > > why do you have to specify 0.0.0.0 *and* dynaddr? > [...] > Regardless, you probably *do* want a dummy address. An address is needed > in order to add a dummy default route which will be updated once the pppoe > interface comes up (more details forthcoming in my reply to naddy's question). So why not fix the problem? How many years are we going to continue to add workaround for p2p interfaces? I send a diff last year to start doing that: http://marc.info/?l=openbsd-tech&m=146539593821612&w=2 Since nobody jumped in the boat and I already have ETOOMANYTHING to do I dropped the diff.
Re: htpasswd: use crypt_newhash instead of bcrypt API
On 21:16:08, 6.06.17, Michal Mazurek wrote:
> When talking about this with mulander@ it came out that the docs could
> use a touch.
>
> The commit message for the diff that didn't update the docs was:
>
> permit "bcrypt" as an alias for "blowfish". this is, after all, what
> 99% of the world calls it.
> allow just "bcrypt" without params to mean auto-tune ("bcrypt,a").
> default remains 8 rounds (for now)
>
> Comments? OK?
Discussion about the defaults aside, any comments on the doc change? OK?
--
Michal Mazurek
Re: sppp(4)/pppoe(4) dynamic address hack
On Sat, Jun 10, 2017 at 10:22:22AM +0200, Stefan Sperling wrote: > On Sat, Jun 10, 2017 at 08:35:13AM +0100, Jason McIntyre wrote: > > thanks. the ifconfig.8 bits look good, but i think you should also > > change the examples given in pppoe(4) to show the updated syntax, to > > prepare folks. > > > > actually i think it would be better to discuss the deprecated stuff > > in pppoe(4), rather than adding it to ifconfig(8). > > Sure. I think it is also worth pointing out that all this is IPv4 only. > Also document sppp(4) behaviour with respect to the default route. > thanks, i'm fine with this. jmc > Index: sbin/ifconfig/ifconfig.8 > === > RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v > retrieving revision 1.282 > diff -u -p -r1.282 ifconfig.8 > --- sbin/ifconfig/ifconfig.8 12 May 2017 15:11:02 - 1.282 > +++ sbin/ifconfig/ifconfig.8 10 Jun 2017 08:17:32 - > @@ -1393,6 +1393,8 @@ Clear a previously set service name. > .Op Cm authkey Ar key > .Op Cm authname Ar name > .Op Cm authproto Ar proto > +.Op Oo Fl Oc Ns Cm dynaddr > +.Op Oo Fl Oc Ns Cm dyndest > .Op Oo Fl Oc Ns Cm peerflag Ar flag > .Op Cm peerkey Ar key > .Op Cm peername Ar name > @@ -1419,6 +1421,19 @@ The protocol name can be either > or > .Ql none . > In the latter case, authentication will be turned off. > +.It Cm dynaddr > +The local IPv4 address will be changed to an address suggested by the peer. > +.It Cm -dynaddr > +Disable dynamic updates of the local IPv4 address. > +This is the default. > +.It Cm dyndest > +The destination IPv4 address will be changed to an address suggested > +by the peer. > +If a default route which uses this interface exists, the gateway will be > +changed to the suggested address as well. > +.It Cm -dyndest > +Disable dynamic updates of the destination IPv4 address. > +This is the default. > .It Cm peerflag Ar flag > Set a specified PPP flag for the remote authenticator. > The flag name can be either > Index: share/man/man4/pppoe.4 > === > RCS file: /cvs/src/share/man/man4/pppoe.4,v > retrieving revision 1.33 > diff -u -p -r1.33 pppoe.4 > --- share/man/man4/pppoe.422 Mar 2017 21:37:24 - 1.33 > +++ share/man/man4/pppoe.410 Jun 2017 08:11:35 - > @@ -99,9 +99,10 @@ A typical file looks like this: > inet 0.0.0.0 255.255.255.255 NONE \e > pppoedev em0 authproto pap \e > authname 'testcaller' authkey 'donttell' up > -dest 0.0.0.1 > +dest 10.1.1.1 > +dynaddr dyndest > inet6 eui64 > -!/sbin/route add default -ifp pppoe0 0.0.0.1 > +!/sbin/route add default -ifp pppoe0 10.1.1.1 > !/sbin/route add -inet6 default -ifp pppoe0 fe80::%pppoe0 > .Ed > .Pp > @@ -111,19 +112,28 @@ The physical interface must also be mark > # echo "up" > /etc/hostname.em0 > .Ed > .Pp > +In the above example, 10.1.1.1 is an otherwise unused IP address which > +serves as a placeholder for dynamic address configuration. > Since this is a PPP interface, the addresses assigned to the interface > may change during PPP negotiation. > There is no fine grained control available for deciding > which addresses are acceptable and which are not. > -For the local side and the remote address there is exactly one choice: > -hard coded address or wildcard. > -If a real address is assigned to one side of the connection, > -PPP negotiation will only agree to exactly this address. > -If one side is wildcarded, > -every address suggested by the peer will be accepted. > +For the local address and the remote address each there is exactly one > choice: > +hard coded address or dynamic address. > .Pp > +By default, PPP negotiation will only agree to exactly the IPv4 addresses > +which are configured on the interface. > +If dynamic address configuration is enabled for the local address (dynaddr) > +and/or remote address (dyndest), any address suggested by the peer will > +be accepted and overrides the addresses configured on the interface and > +the corresponding default route. > +.Pp > +A deprecated way of enabling dynamic addresses is by using wildcard > addresses. > To wildcard the local address set it to 0.0.0.0; to wildcard the remote > -address set it to 0.0.0.1. > +address set it to 0.0.0.1 (multiple > +.Nm > +interfaces configured with this wildcard destination address cannot share > +a routing table). > .Sh KERNEL OPTIONS > .Nm > does not interfere with other PPPoE implementations
Re: sppp(4)/pppoe(4) dynamic address hack
On Sat, Jun 10, 2017 at 08:35:13AM +0100, Jason McIntyre wrote:
> thanks. the ifconfig.8 bits look good, but i think you should also
> change the examples given in pppoe(4) to show the updated syntax, to
> prepare folks.
>
> actually i think it would be better to discuss the deprecated stuff
> in pppoe(4), rather than adding it to ifconfig(8).
Sure. I think it is also worth pointing out that all this is IPv4 only.
Also document sppp(4) behaviour with respect to the default route.
Index: sbin/ifconfig/ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.282
diff -u -p -r1.282 ifconfig.8
--- sbin/ifconfig/ifconfig.812 May 2017 15:11:02 - 1.282
+++ sbin/ifconfig/ifconfig.810 Jun 2017 08:17:32 -
@@ -1393,6 +1393,8 @@ Clear a previously set service name.
.Op Cm authkey Ar key
.Op Cm authname Ar name
.Op Cm authproto Ar proto
+.Op Oo Fl Oc Ns Cm dynaddr
+.Op Oo Fl Oc Ns Cm dyndest
.Op Oo Fl Oc Ns Cm peerflag Ar flag
.Op Cm peerkey Ar key
.Op Cm peername Ar name
@@ -1419,6 +1421,19 @@ The protocol name can be either
or
.Ql none .
In the latter case, authentication will be turned off.
+.It Cm dynaddr
+The local IPv4 address will be changed to an address suggested by the peer.
+.It Cm -dynaddr
+Disable dynamic updates of the local IPv4 address.
+This is the default.
+.It Cm dyndest
+The destination IPv4 address will be changed to an address suggested
+by the peer.
+If a default route which uses this interface exists, the gateway will be
+changed to the suggested address as well.
+.It Cm -dyndest
+Disable dynamic updates of the destination IPv4 address.
+This is the default.
.It Cm peerflag Ar flag
Set a specified PPP flag for the remote authenticator.
The flag name can be either
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.342
diff -u -p -r1.342 ifconfig.c
--- sbin/ifconfig/ifconfig.c5 Jun 2017 05:10:23 - 1.342
+++ sbin/ifconfig/ifconfig.c9 Jun 2017 15:57:40 -
@@ -266,6 +266,9 @@ voidsetseername(const char *, int);
void setseerkey(const char *, int);
void setseerflag(const char *, int);
void unsetseerflag(const char *, int);
+void sipcpinfo(struct sipcpreq *);
+void setspppdynaddr(const char *, int);
+void setspppdyndest(const char *, int);
void sppp_status(void);
void sppp_printproto(const char *, struct sauthreq *);
void setifpriority(const char *, int);
@@ -446,6 +449,10 @@ const struct cmd {
{ "peerkey",NEXTARG,0, setseerkey },
{ "peerflag", NEXTARG,0, setseerflag },
{ "-peerflag", NEXTARG,0, unsetseerflag },
+ { "dynaddr",1, 0, setspppdynaddr },
+ { "-dynaddr", 0, 0, setspppdynaddr },
+ { "dyndest",1, 0, setspppdyndest },
+ { "-dyndest", 0, 0, setspppdyndest },
{ "nwflag", NEXTARG,0, setifnwflag },
{ "-nwflag",NEXTARG,0, unsetifnwflag },
{ "flowsrc",NEXTARG,0, setpflow_sender },
@@ -4878,6 +4885,63 @@ unsetseerflag(const char *val, int d
}
void
+sipcpinfo(struct sipcpreq *req)
+{
+ bzero(req, sizeof(*req));
+
+ ifr.ifr_data = (caddr_t)req;
+ req->cmd = SPPPIOGIPCP;
+ if (ioctl(s, SIOCGSARAMS, &ifr) == -1)
+ err(1, "SIOCGSARAMS(SPPPIOGIPCP)");
+}
+
+void
+setspppdynaddr(const char *val, int d)
+{
+ struct sipcpreq scp;
+
+ sipcpinfo(&scp);
+
+ if (d == 1) {
+ if (scp.flags & SIPCP_MYADDR_DYN)
+ return;
+ scp.flags |= SIPCP_MYADDR_DYN;
+ } else {
+ if ((scp.flags & SIPCP_MYADDR_DYN) == 0)
+ return;
+ scp.flags &= ~SIPCP_MYADDR_DYN;
+ }
+
+ scp.cmd = SPPPIOSIPCP;
+ if (ioctl(s, SIOCSSARAMS, &ifr) == -1)
+ err(1, "SIOCSSARAMS(SPPPIOSIPCP)");
+}
+
+
+void
+setspppdyndest(const char *val, int d)
+{
+ struct sipcpreq scp;
+
+ sipcpinfo(&scp);
+
+ if (d == 1) {
+ if (scp.flags & SIPCP_HISADDR_DYN)
+ return;
+ scp.flags |= SIPCP_HISADDR_DYN;
+ } else {
+ if ((scp.flags & SIPCP_HISADDR_DYN) == 0)
+ return;
+ scp.flags &= ~SIPCP_HISADDR_DYN;
+ }
+
+ scp.cmd = SPPPIOSIPCP;
+ if (ioctl(s, SIOCSSARAMS, &ifr) == -1)
+ err(1, "SIOCSSARAMS(SPPPIOSIPCP)");
+}
+
+
+void
sppp_printproto(const char *name, struct sauthreq *auth)
{
if (auth->proto == 0)
@@ -4905,6 +4969,7 @@ sppp_status(
Re: sppp(4)/pppoe(4) dynamic address hack
On Sat, Jun 10, 2017 at 09:23:45AM +0200, Stefan Sperling wrote: > On Fri, Jun 09, 2017 at 06:33:46PM +0200, Stefan Sperling wrote: > > This diff changes the way dynamic addresses are configured in sppp(4). > > I was asked in private whether we could avoid a flag day which makes > it inconvenient to upgrade remote boxes only reachable over pppoe(4). > > I see no harm in keeping the old magic working for now. > The new mechanism still works, and the goal of allowing more than one pppoe > instance per routing table does not conflict with temporary backwards compat. > So we would release 6.2 with support for both ways, and remove the old way > in 6.3 or later. This gives people some time for the transition. > > This diff also contains doc updates requested by jmc@. > thanks. the ifconfig.8 bits look good, but i think you should also change the examples given in pppoe(4) to show the updated syntax, to prepare folks. actually i think it would be better to discuss the deprecated stuff in pppoe(4), rather than adding it to ifconfig(8). jmc > Index: sbin/ifconfig/ifconfig.8 > === > RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v > retrieving revision 1.282 > diff -u -p -r1.282 ifconfig.8 > --- sbin/ifconfig/ifconfig.8 12 May 2017 15:11:02 - 1.282 > +++ sbin/ifconfig/ifconfig.8 10 Jun 2017 07:08:01 - > @@ -1393,6 +1393,8 @@ Clear a previously set service name. > .Op Cm authkey Ar key > .Op Cm authname Ar name > .Op Cm authproto Ar proto > +.Op Oo Fl Oc Ns Cm dynaddr > +.Op Oo Fl Oc Ns Cm dyndest > .Op Oo Fl Oc Ns Cm peerflag Ar flag > .Op Cm peerkey Ar key > .Op Cm peername Ar name > @@ -1419,6 +1421,21 @@ The protocol name can be either > or > .Ql none . > In the latter case, authentication will be turned off. > +.It Cm dynaddr > +The local address will be changed to an address suggested by the peer. > +A deprecated way of achieving the same effect is setting the local address > +to 0.0.0.0. > +.It Cm -dynaddr > +Disable dynamic updates of the local address. > +This is the default. > +.It Cm dyndest > +The destination address will be changed to an address suggested by the peer. > +A deprecated way of achieving the same effect is setting the destination > +address to 0.0.0.1 (multiple SPPP interfaces configured with this destination > +address cannot share a routing table). > +.It Cm -dyndest > +Disable dynamic updates of the destination address. > +This is the default. > .It Cm peerflag Ar flag > Set a specified PPP flag for the remote authenticator. > The flag name can be either
Re: sppp(4)/pppoe(4) dynamic address hack
On Fri, Jun 09, 2017 at 06:33:46PM +0200, Stefan Sperling wrote:
> This diff changes the way dynamic addresses are configured in sppp(4).
I was asked in private whether we could avoid a flag day which makes
it inconvenient to upgrade remote boxes only reachable over pppoe(4).
I see no harm in keeping the old magic working for now.
The new mechanism still works, and the goal of allowing more than one pppoe
instance per routing table does not conflict with temporary backwards compat.
So we would release 6.2 with support for both ways, and remove the old way
in 6.3 or later. This gives people some time for the transition.
This diff also contains doc updates requested by jmc@.
Index: sbin/ifconfig/ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.282
diff -u -p -r1.282 ifconfig.8
--- sbin/ifconfig/ifconfig.812 May 2017 15:11:02 - 1.282
+++ sbin/ifconfig/ifconfig.810 Jun 2017 07:08:01 -
@@ -1393,6 +1393,8 @@ Clear a previously set service name.
.Op Cm authkey Ar key
.Op Cm authname Ar name
.Op Cm authproto Ar proto
+.Op Oo Fl Oc Ns Cm dynaddr
+.Op Oo Fl Oc Ns Cm dyndest
.Op Oo Fl Oc Ns Cm peerflag Ar flag
.Op Cm peerkey Ar key
.Op Cm peername Ar name
@@ -1419,6 +1421,21 @@ The protocol name can be either
or
.Ql none .
In the latter case, authentication will be turned off.
+.It Cm dynaddr
+The local address will be changed to an address suggested by the peer.
+A deprecated way of achieving the same effect is setting the local address
+to 0.0.0.0.
+.It Cm -dynaddr
+Disable dynamic updates of the local address.
+This is the default.
+.It Cm dyndest
+The destination address will be changed to an address suggested by the peer.
+A deprecated way of achieving the same effect is setting the destination
+address to 0.0.0.1 (multiple SPPP interfaces configured with this destination
+address cannot share a routing table).
+.It Cm -dyndest
+Disable dynamic updates of the destination address.
+This is the default.
.It Cm peerflag Ar flag
Set a specified PPP flag for the remote authenticator.
The flag name can be either
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.342
diff -u -p -r1.342 ifconfig.c
--- sbin/ifconfig/ifconfig.c5 Jun 2017 05:10:23 - 1.342
+++ sbin/ifconfig/ifconfig.c9 Jun 2017 15:57:40 -
@@ -266,6 +266,9 @@ voidsetseername(const char *, int);
void setseerkey(const char *, int);
void setseerflag(const char *, int);
void unsetseerflag(const char *, int);
+void sipcpinfo(struct sipcpreq *);
+void setspppdynaddr(const char *, int);
+void setspppdyndest(const char *, int);
void sppp_status(void);
void sppp_printproto(const char *, struct sauthreq *);
void setifpriority(const char *, int);
@@ -446,6 +449,10 @@ const struct cmd {
{ "peerkey",NEXTARG,0, setseerkey },
{ "peerflag", NEXTARG,0, setseerflag },
{ "-peerflag", NEXTARG,0, unsetseerflag },
+ { "dynaddr",1, 0, setspppdynaddr },
+ { "-dynaddr", 0, 0, setspppdynaddr },
+ { "dyndest",1, 0, setspppdyndest },
+ { "-dyndest", 0, 0, setspppdyndest },
{ "nwflag", NEXTARG,0, setifnwflag },
{ "-nwflag",NEXTARG,0, unsetifnwflag },
{ "flowsrc",NEXTARG,0, setpflow_sender },
@@ -4878,6 +4885,63 @@ unsetseerflag(const char *val, int d
}
void
+sipcpinfo(struct sipcpreq *req)
+{
+ bzero(req, sizeof(*req));
+
+ ifr.ifr_data = (caddr_t)req;
+ req->cmd = SPPPIOGIPCP;
+ if (ioctl(s, SIOCGSARAMS, &ifr) == -1)
+ err(1, "SIOCGSARAMS(SPPPIOGIPCP)");
+}
+
+void
+setspppdynaddr(const char *val, int d)
+{
+ struct sipcpreq scp;
+
+ sipcpinfo(&scp);
+
+ if (d == 1) {
+ if (scp.flags & SIPCP_MYADDR_DYN)
+ return;
+ scp.flags |= SIPCP_MYADDR_DYN;
+ } else {
+ if ((scp.flags & SIPCP_MYADDR_DYN) == 0)
+ return;
+ scp.flags &= ~SIPCP_MYADDR_DYN;
+ }
+
+ scp.cmd = SPPPIOSIPCP;
+ if (ioctl(s, SIOCSSARAMS, &ifr) == -1)
+ err(1, "SIOCSSARAMS(SPPPIOSIPCP)");
+}
+
+
+void
+setspppdyndest(const char *val, int d)
+{
+ struct sipcpreq scp;
+
+ sipcpinfo(&scp);
+
+ if (d == 1) {
+ if (scp.flags & SIPCP_HISADDR_DYN)
+ return;
+ scp.flags |= SIPCP_HISADDR_DYN;
+ } else {
+ if ((scp.flags & SIPCP_HISADDR_DYN) == 0)
+ ret
