Re: ssh nits

2023-03-08 Thread Darren Tucker
On Thu, 9 Mar 2023 at 06:50, joshua stein  wrote:
> On Thu, 09 Mar 2023 at 06:41:50 +1100, Darren Tucker wrote:
> > This seems to be one too many parens?   ie
> > if (negate = (attrib[0] == '!'))
>
> clang warns if there's not the extra set of parens in case it's an
> accidental = instead of ==.

ok dtucker for this one.

"no objection" to the sshpty.c one.



--
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: ssh nits

2023-03-08 Thread Darren Tucker
On Thu, 9 Mar 2023 at 02:09, joshua stein  wrote:
> cppcheck found these, are they worth fixing?
>
> In the non-fail case, done is set to NULL and then free()d.
> free(NULL) is legal but maybe worth removing?

ssh uses this pattern a lot, and I agree with millert that it's not
worth changing.

char *thing = NULL;
[lots of stuff that might set thing]
if (maybe()) {
free(thing);
thing = something;
}
[more stuff]
free(thing)

We actually went through and changed all the "if (thing) free(thing)"
instances to drop the "if" some time ago.

> grp == NULL fatal()s, so remove the ternary operations that will
> never be the conditionals they aspire to be.

That's true in OpenBSD, but not in -portable where the check is a
debug only.  That said, there's already a diff in this code block
that'll stop future syncs from applying cleanly so I don't mind either
way.

> +   if ((r = encode_dest_constraint_hop(b, >from)) != 0 ||
> +   (r = encode_dest_constraint_hop(b, >to)) != 0 ||

I'll wait for djm to comment on this one.

> +   if ((negate = (attrib[0] == '!')))

This seems to be one too many parens?   ie
if (negate = (attrib[0] == '!'))

> -   if ((r = parse_dest_constraint_hop(frombuf, >from) != 0) ||
> -   (r = parse_dest_constraint_hop(tobuf, >to) != 0))
> +   if ((r = parse_dest_constraint_hop(frombuf, >from)) != 0 ||
> +       (r = parse_dest_constraint_hop(tobuf, >to)) != 0)

also djm.

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: ssh-keygen(1): by default generate ed25519 key (instead of rsa)

2022-11-08 Thread Darren Tucker
On Tue, 8 Nov 2022 at 14:23, Joerg Sonnenberger  wrote:
> Am Tue, Nov 08, 2022 at 01:23:52PM +1100 schrieb Darren Tucker:
[...]
> > Not quite: the default value for IdentityFile has RSA before ED25519.
[...]
> I tried that first and it picked up id_ed25519 from the agent, even if
> both keys are accepted by the server.

It prefers keys present in the agent as those don't require entering a
passphrase.  It'll also prefer keys explicitly specified by the user
on the command line since that demonstrates user intent.  And the
behaviour is also modified by IdentitiesOnly.

> I guess that makes the answer a case of "it's complicated".

It is.  And IdentityFile works differently to most other options (it's
cumulative, not first-match) which was probably a mistake, but we're
kind of stuck with it.

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: ssh-keygen(1): by default generate ed25519 key (instead of rsa)

2022-11-07 Thread Darren Tucker
On Tue, 8 Nov 2022 at 11:05, Joerg Sonnenberger  wrote:
> Am Mon, Nov 07, 2022 at 12:53:43PM +0100 schrieb Renaud Allard:
[...]
> > Wouldn't it also be a good idea for ssh client to also try the ed25519 key
> > first if there are multiple keys?
>
> That's already happening.

Not quite: the default value for IdentityFile has RSA before ED25519.
Changing the default order is a potentially disruptive change, though,
as configs that previously worked may hit MaxAuthTries instead.

 IdentityFile
 [...] The default is ~/.ssh/id_rsa,
 ~/.ssh/id_ecdsa, ~/.ssh/id_ecdsa_sk, ~/.ssh/id_ed25519,
 ~/.ssh/id_ed25519_sk and ~/.ssh/id_dsa.

$ SSH_AUTH_SOCK= ssh -F/dev/null localhost
Enter passphrase for key '/home/dtucker/.ssh/id_rsa':
Enter passphrase for key '/home/dtucker/.ssh/id_ecdsa':
Enter passphrase for key '/home/dtucker/.ssh/id_ed25519':

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: SSH_USER_AUTH

2022-09-18 Thread Darren Tucker
On Mon, 19 Sept 2022 at 04:36, Joerg Sonnenberger  wrote:
> does anyone still know the motivation for SSH_USER_AUTH pointing to a
> file with the data instead of containing it directly?

Authentication data is sensitive and a process's environment variables
can be inspected by any other process on the system, whereas files
have ownership and permission bits that control access.

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: ssh-add(1): fix NULL in fprintf

2022-06-17 Thread Darren Tucker
On Fri, 17 Jun 2022 at 04:49, Martin Vahlensieck
 wrote:
> ping, diff attached

Applied, thanks.

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: sshd_config(5): Use correct path for system-wide known_hosts

2022-04-11 Thread Darren Tucker
On Mon, 11 Apr 2022 at 16:12, Martin Vahlensieck
 wrote:
> The path to the system-wide known_hosts file is /etc/ssh/ssh_known_hosts
> and not /etc/ssh/known_hosts.  See auth2-hostbased.c line 221-223.

Applied, thanks.

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



dhcpd: look up ethernet addresses in /etc/ethers

2020-02-18 Thread Darren Tucker
Hi.

I've got a bunch of things that have static reservations by MAC address in
dhcpd.conf.  I also have the MAC addresses in /etc/ethers for tcpdumping.
This duplication and potential inconsistency annoys me.

Is there any reason we can't have dhcpd look up names in /etc/ethers?

Index: usr.sbin/dhcpd/parse.c
===
RCS file: /cvs/src/usr.sbin/dhcpd/parse.c,v
retrieving revision 1.26
diff -u -p -r1.26 parse.c
--- usr.sbin/dhcpd/parse.c  16 Feb 2017 00:24:43 -  1.26
+++ usr.sbin/dhcpd/parse.c  18 Feb 2020 09:36:02 -
@@ -46,6 +46,7 @@
 #include 
 
 #include 
+#include 
 
 #include 
 #include 
@@ -218,8 +219,8 @@ void
 parse_hardware_param(FILE *cfile, struct hardware *hardware)
 {
char *val;
-   int token, hlen;
-   unsigned char *t;
+   int token, hlen = 0;
+   unsigned char *e, *t = NULL;
 
token = next_token(, cfile);
switch (token) {
@@ -235,6 +236,19 @@ parse_hardware_param(FILE *cfile, struct
return;
}
 
+   /* Try looking up in /etc/ethers first. */
+   if (hardware->htype == HTYPE_ETHER) {
+   token = peek_token(, cfile);
+   hlen = sizeof(struct ether_addr);
+   if ((e = malloc(hlen)) == NULL)
+   fatalx("can't allocate space for ethernet address.");
+   if (ether_hostton(val, (struct ether_addr *)e) == 0) {
+   (void)next_token(, cfile); /* consume token */
+   t = e;
+   } else
+   free(e);
+   }
+
/*
 * Parse the hardware address information.   Technically, it
 * would make a lot of sense to restrict the length of the data
@@ -244,8 +258,9 @@ parse_hardware_param(FILE *cfile, struct
 * accept that data in the lease file rather than simply failing
 * on such clients.   Yuck.
 */
-   hlen = 0;
-   t = parse_numeric_aggregate(cfile, NULL, , ':', 16, 8);
+   if (!t)
+   t = parse_numeric_aggregate(cfile, NULL, , ':', 16, 8);
+
if (!t)
return;
if (hlen > sizeof(hardware->haddr)) {

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: backgrounded ssh, strange terminal behaviour

2019-12-10 Thread Darren Tucker
On Wed, 11 Dec 2019 at 01:10, Stuart Henderson  wrote:

> Not new (it happens in at least 6.6) but I just noticed this.
>
> If I run some program via ssh command-line ("ssh localhost sleep 60"
> is good enough), then put it in the background (^Z bg), the terminal
> misses about a third of characters typed.
>
> typed:123456789012345678901234567890
> accepted: 24680246891235780235790
>
> Should this be treated as something more than "don't do that then"?
>

Sounds like a bug.  I can reproduce with controlmaster  as described by
otto but not otherwise.  Can you confirm that you are also using
controlmaster or are you seeing this in a single process?

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.


Re: ssh "kex_exchange_identification: Connection closed by remote host"

2019-11-12 Thread Darren Tucker
On Tue, Nov 12, 2019 at 08:52:58PM +1100, Darren Tucker wrote:
> On Tue, 12 Nov 2019 at 20:47, Darren Tucker  wrote:
> > I got this on the second try although the log is not very helpful.
> > I'd suggest checking your MaxStartups setting in sshd_config and
> > comparing the settings to the numbers of connections you have.
> 
> Confirmed that exceeding MaxStartups matches the observed behaviour.
> It'll produce the following log message but only at LogLevel verbose
> or higher:
> 
> drop connection #1 from [127.0.0.1]:45006 on [127.0.0.1]:2022 past MaxStartups

The SSH protocol does actually allow text prior to the protocol banner
exchange (RFC4253 section 4.2) so doing something like this is actually
protocol compliant, although our client only shows it at LogLevel
debug1.

$ ssh -v -p 2022 localhost
[...]
debug1: Connecting to localhost [127.0.0.1] port 2022.
debug1: fd 3 clearing O_NONBLOCK
debug1: Connection established.
[...]
debug1: Local version string SSH-2.0-OpenSSH_8.1
debug1: kex_exchange_identification: banner line 0: exceeded MaxStartups
kex_exchange_identification: Connection closed by remote host

Index: sshd.c
===
RCS file: /cvs/src/usr.bin/ssh/sshd.c,v
retrieving revision 1.539
diff -u -p -r1.539 sshd.c
--- sshd.c  31 Oct 2019 21:23:19 -  1.539
+++ sshd.c  12 Nov 2019 10:29:15 -
@@ -1098,6 +1098,7 @@ server_accept_loop(int *sock_in, int *so
if (drop_connection(startups) == 1) {
char *laddr = get_local_ipaddr(*newsock);
char *raddr = get_peer_ipaddr(*newsock);
+   char msg[] = "Exceeded MaxStartups\r\n";
 
verbose("drop connection #%d from [%s]:%d "
"on [%s]:%d past MaxStartups", startups,
@@ -1105,6 +1106,7 @@ server_accept_loop(int *sock_in, int *so
laddr, get_local_port(*newsock));
free(laddr);
free(raddr);
+   write(*newsock, msg, strlen(msg));
close(*newsock);
        continue;
}

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: ssh "kex_exchange_identification: Connection closed by remote host"

2019-11-12 Thread Darren Tucker
On Tue, 12 Nov 2019 at 20:47, Darren Tucker  wrote:
> I got this on the second try although the log is not very helpful.
> I'd suggest checking your MaxStartups setting in sshd_config and
> comparing the settings to the numbers of connections you have.

Confirmed that exceeding MaxStartups matches the observed behaviour.
It'll produce the following log message but only at LogLevel verbose
or higher:

drop connection #1 from [127.0.0.1]:45006 on [127.0.0.1]:2022 past MaxStartups

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: ssh "kex_exchange_identification: Connection closed by remote host"

2019-11-12 Thread Darren Tucker
On Tue, 12 Nov 2019 at 20:31, Darren Tucker  wrote:
[..]
> I'd start by cranking up the client side log level (LogLevel debug3 in
> ~/.ssh/config) and use CVS_RSH="ssh -E logfile" or ssh -y to send the
> logs to syslog.
>
> Is this a public mirror, and if so which one?

bleh, it doesn't support spaces, at least not in the obvious way, so
something like

$ cat ~/bin/ssh-with-logging
#!/bin/sh
exec ssh -vvv -E /tmp/ssh.log $@

$ CVS_RSH=~/bin/ssh-with-logging cvs -d
anon...@anoncvs.spacehopper.org:/cvs up -dPA

I got this on the second try although the log is not very helpful.
I'd suggest checking your MaxStartups setting in sshd_config and
comparing the settings to the numbers of connections you have.

$ CVS_RSH=~/bin/ssh-with-logging cvs -d
anon...@anoncvs.spacehopper.org:/cvs co src
cvs [checkout aborted]: end of file from server (consult above messages if any)
$ cat /tmp/ssh.log
OpenSSH_8.1, LibreSSL 3.0.2
debug1: Reading configuration data /home/dtucker/.ssh/config
debug1: /home/dtucker/.ssh/config line 1: Applying options for *
debug1: /home/dtucker/.ssh/config line 3: Deprecated option "useroaming"
debug3: kex names ok: [diffie-hellman-group1-sha1,diffie-hellman-group14-sha1]
debug2: checking match for 'Host gate' host anoncvs.spacehopper.org
originally anoncvs.spacehopper.org
debug3: /home/dtucker/.ssh/config line 99: not matched 'Host
"anoncvs.spacehopper.org"'
debug2: match not found
debug3: kex names ok: [curve25519-sha...@libssh.org,ecdh-sha2-nistp256]
debug3: kex names ok: [ecdh-sha2-nistp521,diffie-hellman-group-exchange-sha256]
debug3: kex names ok: [diffie-hellman-group1-sha1]
debug3: kex names ok: [diffie-hellman-group14-sha1,diffie-hellman-group1-sha1]
debug1: /home/dtucker/.ssh/config line 394: Applying options for *
debug1: Reading configuration data /etc/ssh/ssh_config
debug1: /etc/ssh/ssh_config line 19: Applying options for *
debug1: Security key provider $SSH_SK_PROVIDER did not resolve; disabling
debug2: resolving "anoncvs.spacehopper.org" port 22
debug2: ssh_connect_direct
debug1: Connecting to anoncvs.spacehopper.org [195.95.187.28] port 22.
debug2: fd 4 setting O_NONBLOCK
debug1: fd 4 clearing O_NONBLOCK
debug1: Connection established.
debug3: timeout: 29385 ms remain after connect
debug1: identity file /home/dtucker/.ssh/id_rsa type 0
debug1: identity file /home/dtucker/.ssh/id_rsa-cert type -1
debug1: identity file /home/dtucker/.ssh/id_dsa type 1
debug1: identity file /home/dtucker/.ssh/id_dsa-cert type -1
debug1: identity file /home/dtucker/.ssh/id_ecdsa type 2
debug1: identity file /home/dtucker/.ssh/id_ecdsa-cert type -1
debug1: identity file /home/dtucker/.ssh/id_ecdsa_sk type -1
debug1: identity file /home/dtucker/.ssh/id_ecdsa_sk-cert type -1
debug1: identity file /home/dtucker/.ssh/id_ed25519 type 3
debug1: identity file /home/dtucker/.ssh/id_ed25519-cert type -1
debug1: identity file /home/dtucker/.ssh/id_xmss type -1
debug1: identity file /home/dtucker/.ssh/id_xmss-cert type -1
debug1: Local version string SSH-2.0-OpenSSH_8.1
kex_exchange_identification: Connection closed by remote host

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: ssh "kex_exchange_identification: Connection closed by remote host"

2019-11-12 Thread Darren Tucker
On Tue, 12 Nov 2019 at 20:06, Stuart Henderson  wrote:
> Occasionally I see this when connecting to anoncvs on my mirror,
>
> $ cvs -d $CVSROOT di
> kex_exchange_identification: Connection closed by remote host
> cvs [diff aborted]: end of file from server (consult above messages if any)
>
> On the server side, this is logged:
>
> sshd[13009]: error: kex_exchange_identification: read: Connection reset by 
> peer
>
> And others have reported it too. I haven't noticed it with e.g. http/https
> connections to the server.
>
> Does anyone have advice about tracking it down?

I'd start by cranking up the client side log level (LogLevel debug3 in
~/.ssh/config) and use CVS_RSH="ssh -E logfile" or ssh -y to send the
logs to syslog.

Is this a public mirror, and if so which one?

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: ssh-keygen: tweak error for -b

2019-08-03 Thread Darren Tucker
ok dtucker with one suggestion.

On Fri, 5 Jul 2019 at 06:01, Christian Weisgerber  wrote:
[...]
>  #ifdef WITH_OPENSSL
> -   u_int maxbits, nid;
> +   u_int nid;

nid is only used inside the #ifdef below, you can move this
declaration to the start of the block where it's used and delete this
ifdef.

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: www/64.html - OpenSSH version 7.8 or 7.9?

2018-10-18 Thread Darren Tucker
On 18 October 2018 at 07:20, Markus Lude  wrote:

> Hello,
>
> www/64.html mentions OpenSSH version 7.8.
>
> With snapshot from october 11th on amd64 the version of ssh is 7.9:
>

it's 7.9, which was officially released a couple of hours ago.  The website
will be updated shortly.

$ ssh -V
> OpenSSH_7.9, LibreSSL 2.8.2
>
> Theo bumped the version in src/usr.bin/ssh/version.h r1.83,
> this version is tagged with OPENBSD_6_4_BASE.
>
> On the www.openssh.org the latest OpenSSH release mentioned is 7.8,
> so no newer release notes there to link to.
>
> Regards
> Markus
>
>


-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.


Re: ssh: use getservbyname(3) for port numbers

2018-10-04 Thread Darren Tucker
On Sun, 2 Sep 2018 at 03:16, Theo de Raadt  wrote:
>
> > Is there a reason ssh doesn't consult services(5) for port numbers?
>
> I think I know why but I'm not going to speak about those dark days.

I would be fine with adding this.  I am not sure what the reasoning
behind it was (reduce NIS lookups back in the day?)

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: Show -o and -a in ssh-keygen(1) synopsis

2018-08-13 Thread Darren Tucker
On 4 August 2018 at 18:15, Jeremy Evans  wrote:
[...]
> I checked -A and that also respects -o, so I documented that.  I'm
> not sure how much it matters as the host keys -A generates are not
> password protected, but maybe there are other reasons to use the
> newer format.

The host keys must be unencrypted if you want sshd to be able to start
at boot time, which most people do.

> I think the documentation for -e should be updated to specify it only
> exports public keys (assuming I'm reading the code correctly), or
> ssh-keygen should be updated to write private keys for the RFC4716
> format if the input file is a private key (since that's what the
> documentation currently implies).  But that should probably be a
> separate commit.

I'll check the history but my recollection was that it was supposed to
be able to export private keys as RFC4716 format.

> I also noticed that the -f flag with -A was documented in ssh-keygen(1)
> but not in usage, so I updated usage to match ssh-keygen(1).
>
> OKs for the diff below?

ok dtucker except for:

> +.Op Fl oq

this doesn't look right? -o and -q are distinct orthogonal flags.

[...]
> +   "usage: ssh-keygen [-oq] [-a rounds] [-b bits] [-t dsa | ecdsa | 
> ed25519 | rsa]\n"

ditto.

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



look up interface names for IPv6 "cannot forward" errors

2018-01-02 Thread Darren Tucker
I rearranged my network and ended up getting these in messages:

cannot forward src fe80:9::a691:b1ff:fe1f:d2a0, dst 
2001:44b8:3110:fb00:200:5eff:fe00:10a, nxt 58, rcvif 9, outif 1

I guess I could run ifconfig and count interfaces, but I'm lazy and the
kernel knows what they are.

cannot forward src fe80:9::a691:b1ff:fe1f:d2a0, dst 
2001:44b8:3110:fb00:200:5eff:fe00:10a, nxt 58, rcvif carp0, outif em0

ok?  (caveat: kernel n00b, don't assume I know what I'm doing)

Index: netinet6/ip6_forward.c
===
RCS file: /cvs/src/sys/netinet6/ip6_forward.c,v
retrieving revision 1.95
diff -u -p -r1.95 ip6_forward.c
--- netinet6/ip6_forward.c  30 Jun 2017 11:29:15 -  1.95
+++ netinet6/ip6_forward.c  2 Jan 2018 08:44:38 -
@@ -86,7 +86,7 @@ ip6_forward(struct mbuf *m, struct rtent
 {
struct ip6_hdr *ip6 = mtod(m, struct ip6_hdr *);
struct sockaddr_in6 *dst, sin6;
-   struct ifnet *ifp = NULL;
+   struct ifnet *ifp = NULL, *ifp_dst = NULL;
int error = 0, type = 0, code = 0;
struct mbuf *mcopy = NULL;
 #ifdef IPSEC
@@ -197,12 +197,27 @@ reroute:
ip6_log_time = time_uptime;
inet_ntop(AF_INET6, >ip6_src, src6, sizeof(src6));
inet_ntop(AF_INET6, >ip6_dst, dst6, sizeof(dst6));
-   log(LOG_DEBUG,
-   "cannot forward "
-   "src %s, dst %s, nxt %d, rcvif %u, outif %u\n",
-   src6, dst6,
-   ip6->ip6_nxt,
-   m->m_pkthdr.ph_ifidx, rt->rt_ifidx);
+   ifp = if_get(m->m_pkthdr.ph_ifidx);
+   ifp_dst = if_get(rt->rt_ifidx);
+   if (ifp != NULL && ifp_dst != NULL) {
+   log(LOG_DEBUG,
+   "cannot forward src %s, dst %s, "
+   "nxt %d, rcvif %s, outif %s\n",
+   src6, dst6,
+   ip6->ip6_nxt,
+   ifp->if_xname, ifp_dst->if_xname);
+   } else {
+   log(LOG_DEBUG,
+   "cannot forward src %s, dst %s, "
+   "nxt %d, rcvif %u, outif %u\n",
+   src6, dst6,
+   ip6->ip6_nxt,
+   m->m_pkthdr.ph_ifidx, rt->rt_ifidx);
+   }
+   if_put(ifp_dst);
+   ifp_dst = NULL;
+   if_put(ifp);
+   ifp = NULL;
    }
if (mcopy)
icmp6_error(mcopy, ICMP6_DST_UNREACH,

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: sshd(8) logging of client disconnect from ClientAliveInterval

2017-10-17 Thread Darren Tucker
On Tue, Oct 17, 2017 at 09:10:38PM +0300, Lars Noodén wrote:
> Here is a replacement patch.

I meant reusing the existing function rather than cloning it.  It's
currently static so it needs to be exported but IMO that's better than
duplicating the code.

Index: packet.c
===
RCS file: /cvs/src/usr.bin/ssh/packet.c,v
retrieving revision 1.265
diff -u -p -r1.265 packet.c
--- packet.c13 Oct 2017 21:13:54 -  1.265
+++ packet.c17 Oct 2017 22:49:08 -
@@ -1773,8 +1773,8 @@ ssh_packet_send_debug(struct ssh *ssh, c
fatal("%s: %s", __func__, ssh_err(r));
 }
 
-static void
-fmt_connection_id(struct ssh *ssh, char *s, size_t l)
+void
+sshpkt_fmt_connection_id(struct ssh *ssh, char *s, size_t l)
 {
snprintf(s, l, "%.200s%s%s port %d",
ssh->log_preamble ? ssh->log_preamble : "",
@@ -1790,7 +1790,7 @@ sshpkt_fatal(struct ssh *ssh, const char
 {
char remote_id[512];
 
-   fmt_connection_id(ssh, remote_id, sizeof(remote_id));
+   sshpkt_fmt_connection_id(ssh, remote_id, sizeof(remote_id));
 
switch (r) {
case SSH_ERR_CONN_CLOSED:
@@ -1852,7 +1852,7 @@ ssh_packet_disconnect(struct ssh *ssh, c
 * Format the message.  Note that the caller must make sure the
 * message is of limited size.
 */
-   fmt_connection_id(ssh, remote_id, sizeof(remote_id));
+   sshpkt_fmt_connection_id(ssh, remote_id, sizeof(remote_id));
va_start(args, fmt);
vsnprintf(buf, sizeof(buf), fmt, args);
va_end(args);
Index: packet.h
===
RCS file: /cvs/src/usr.bin/ssh/packet.h,v
retrieving revision 1.82
diff -u -p -r1.82 packet.h
--- packet.h12 Sep 2017 06:32:07 -  1.82
+++ packet.h17 Oct 2017 22:49:08 -
@@ -186,6 +186,7 @@ int sshpkt_get_cstring(struct ssh *ssh, 
 intsshpkt_get_ec(struct ssh *ssh, EC_POINT *v, const EC_GROUP *g);
 intsshpkt_get_bignum2(struct ssh *ssh, BIGNUM *v);
 intsshpkt_get_end(struct ssh *ssh);
+void   sshpkt_fmt_connection_id(struct ssh *ssh, char *s, size_t l);
 const u_char   *sshpkt_ptr(struct ssh *, size_t *lenp);
 
 /* OLD API */
Index: serverloop.c
===
RCS file: /cvs/src/usr.bin/ssh/serverloop.c,v
retrieving revision 1.198
diff -u -p -r1.198 serverloop.c
--- serverloop.c12 Sep 2017 06:35:32 -  1.198
+++ serverloop.c17 Oct 2017 22:49:08 -
@@ -162,10 +162,12 @@ static void
 client_alive_check(struct ssh *ssh)
 {
int channel_id;
+   char remote_id[512];
 
/* timeout, check to see how many we have had */
if (packet_inc_alive_timeouts() > options.client_alive_count_max) {
-   logit("Timeout, client not responding.");
+   sshpkt_fmt_connection_id(ssh, remote_id, sizeof(remote_id));
+   logit("Timeout, client not responding from %s", remote_id);
    cleanup_exit(255);
}
 

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: sshd(8) logging of client disconnect from ClientAliveInterval

2017-10-16 Thread Darren Tucker
On 17 October 2017 at 06:04, Lars Noodén <lars.noo...@gmail.com> wrote:
>
> +   logit("Timeout, client not responding from %s on port %d.",
> +   ssh_remote_ipaddr(ssh), ssh_remote_port(ssh) );
>

probably better to use fmt_connection_id() instead of hand-rolling the
format.

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.


Re: NET_LOCK() pflow panic

2016-12-21 Thread Darren Tucker
On Thu, Dec 22, 2016 at 11:41 AM, Alexander Bluhm
<alexander.bl...@gmx.net> wrote:
> On Thu, Dec 22, 2016 at 11:32:26AM +1100, Darren Tucker wrote:
>> > I don't have a solution for the moment and I want to be sure we know all
>> > recursions before trying to write a fix.  So here's a diff that mark the
>> > recursions with a XXXSMP like in the NFS case.
>
> Looks like visa@'s crash.  I have already OKed his fix.

Thanks, I'll give that diff a go.  (The "XXXSMP" comment mislead me
since my problem was observed on a single processor machine.)

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: NET_LOCK() pflow panic

2016-12-21 Thread Darren Tucker
  0  0  3 0x14200  reaperreaper
 81268  0  0  0  3 0x14200  pgdaemon  pagedaemon
 85494  0  0  0  3 0x14200  bored crynlk
 71588  0  0  0  3 0x14200  bored crypto
 83246  0  0  0  3 0x14200  pftm  pfpurge
 60845  0  0  0  3 0x14200  bored viomb
 96989  0  0  0  3  0x40014200  acpi0 acpi0
 48057  0  0  0  3 0x14200  bored softnet
 10475  0  0  0  3 0x14200  bored systqmp
 85279  0  0  0  3 0x14200  bored systq
 33036  0  0  0  3  0x40014200  bored softclock
 66216  0  0  0  3  0x40014200idle0
 1  0  1  0  30x82  wait  init
 0 -1  0  0  3 0x10200  scheduler swapper
ddb>
-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: make ssh -qO {stop,exit} less noisy

2016-10-19 Thread Darren Tucker
patch applied, thanks.

On Sun, Oct 16, 2016 at 11:20 AM, Tim Kuijsten <i...@netsend.nl> wrote:
> Hi tech@
>
> This makes the tear down of ssh control masters better play with shell
> scripts that are run by cron. If nothing unexpected happens and in quiet
> mode, then don't echo back the command that is being requested.
>
> -Tim
>
>
> Index: mux.c
> ===
> RCS file: /cvs/src/usr.bin/ssh/mux.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 mux.c
> --- mux.c   30 Sep 2016 09:19:13 -  1.62
> +++ mux.c   16 Oct 2016 18:18:53 -
> @@ -2212,7 +2212,8 @@ muxclient(const char *path)
> exit(0);
> case SSHMUX_COMMAND_TERMINATE:
> mux_client_request_terminate(sock);
> -   fprintf(stderr, "Exit request sent.\r\n");
> +   if (options.log_level != SYSLOG_LEVEL_QUIET)
> +   fprintf(stderr, "Exit request sent.\r\n");
> exit(0);
> case SSHMUX_COMMAND_FORWARD:
> if (mux_client_forwards(sock, 0) != 0)
> @@ -2230,7 +2231,8 @@ muxclient(const char *path)
> exit(0);
> case SSHMUX_COMMAND_STOP:
> mux_client_request_stop_listening(sock);
> -   fprintf(stderr, "Stop listening request sent.\r\n");
> +   if (options.log_level != SYSLOG_LEVEL_QUIET)
> +   fprintf(stderr, "Stop listening request sent.\r\n");
> exit(0);
> case SSHMUX_COMMAND_CANCEL_FWD:
> if (mux_client_forwards(sock, 1) != 0)
>



-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: little simpler ssh code

2016-09-18 Thread Darren Tucker
On Mon, Sep 19, 2016 at 5:41 AM, Martin Natano <nat...@natano.net> wrote:

> Two more loops that can be converted to arc4random_buf(). Ok?
> [...]
> +   arc4random_buf(x11_fake_data, data_len);
> for (i = 0; i < data_len; i++) {
>

I'd put that below the for loop so it matches the order of the comment
above ("Extract real authentication data and generate fake data of the same
length.").  Or better yet, change the comment and group all of the
operations on x11_fake_data and x11_fake_data_len together.

[...]

> +   arc4random_buf(session_key, SSH_SESSION_KEY_LENGTH);
>

sizeof(session_key) instead please.

with those, ok dtucker@

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.


Re: [calendar] United States holiday migrations

2016-01-28 Thread Darren Tucker
On Thu, Jan 28, 2016 at 11:30 AM, Ian Mcwilliam
<i.mcwill...@westernsydney.edu.au> wrote:
> Just for the record
>
>  01/30   Australia Day in Australia
> should be
>  01/26   Australia Day in Australia
>
> If someone so wishes to fix it.

Fixed, thanks.

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: ssh-agent: flush stdout before main loop in foreground mode

2015-12-10 Thread Darren Tucker
On Wed, Dec 2, 2015 at 9:12 AM, Dustin Lundquist <dus...@null-ptr.net> wrote:
> When ssh-agent is run in foreground mode with -d and stdout is not a
> terminal, the output including the path to the listening socket and
> PID may not be written before the main loop begins. Since no further
> output is written to stdout, this output may never be written.

Patch applied, thanks!

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re:

2015-05-31 Thread Darren Tucker
(for the record: the problem was seen with -current and was not specific to
brad's diff.  I was testing the diff on a sparc64 and discovered the
pre-existing problem.)

On Sat, May 30, 2015 at 6:07 AM, Mark Kettenis mark.kette...@xs4all.nl
 wrote:

 I think this means that the tx slot was reused without
 bus_dmamap_unload()'ing its packet first.


Thanks.  I'll see if I can figure out what is going on, and failing that
I'll try stepping back revisions on if_vr* and see if I can find where if
was introduced (or not, as the case may be).

On Sat, May 30, 2015 at 6:13 AM, Miod Vallat m...@online.fr wrote:

  Hey Darren, misc@ is for the trolls, so I moved this to tech@.

 But using a vr(4) device is a troll in itself.


Well I was trying to find problems.  Seemed to work pretty well for that :-)

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.


Re: Snapshot install54.iso does not boot under qemu/kvm

2014-01-02 Thread Darren Tucker
On Thu, Jan 2, 2014 at 8:20 PM, Stefan Fritsch s...@sfritsch.de wrote:
 the 2013-12-31 snapshot iso images don't work for me under qemu 1.7.0
 / linux 3.12.6. They get this far and then reboot:

I had the same problem too and it was just fixed by jsing@ in
sys/arch/i386/stand/libsa/random_i386.S rev 1.3 (confirmed working for
me).  The next snapshot should be good.

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: small ssh refinements

2013-12-24 Thread Darren Tucker
On Mon, Dec 23, 2013 at 10:21:59AM -0500, Ted Unangst wrote:
 On Mon, Dec 23, 2013 at 10:25, Jérémie Courrèges-Anglas wrote:
  Ted Unangst t...@tedunangst.com writes:
  
  Part one of this diff eliminates a series of u_long casts in favor of
  just using native size_t types. A few other type adjustments.
  
  Wouldn't that make ssh harder to port to systems where those types
  aren't available?  See for example millert's last commit to
  yacc/skeleton.c.

There's two types of porting pain: one-off and constant.

Not having a type like ptrdiff_t is the former.  It's a problem that
can be dealt with once, eg testing for it in configure and typdef'ing
as appropriate.  Not having %z is in the same boat (although a bit more
work to write a test for) since we already have a vsnprintf implementation
in the compat library and all the log messages use vsnprintf.

Constant pain is when we have to make changes to the portable code and
patches no longer apply.  These are long-term pain because they keep
slowing things down (and, if we get it wrong, cause other problems)
so should be weighed up to make sure the benefit is worth the effort.

I haven't been through the diff yet to see how much of each type is in
there :-)  I do note that %z is in SuSv3 (but not v2) so anything not
having it is likely to be old but not necessarily obsolete.

 What are these systems?
 
 The counter argument is that using long makes the code harder to port
 to systems where long and ptrdiff_t are different sizes. And by harder
 to port, I mean compiles fine but executes incorrectly. I consider
 that a worse problem.
 
 If ptrdiff_t is really out, I think I'd prefer to take my chances
 with ssize_t being the right size. (there are 3 occurrences of
 ptrdiff_t in umac.c too).

Given that it's already in use and we haven't had any complaints that
I'm aware of I have no objection to adding more ptrdiff_t.  I'll look
through the rest of the diff shortly.

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: bzip2

2013-06-07 Thread Darren Tucker
On Thu, Jun 6, 2013 at 11:30 PM, Theo de Raadt dera...@cvs.openbsd.org wrote:
[re Has anyone looked at zopfli]
 If we did add it, it would only benefit the fast architectures, since
 the others cannot afford the additional build time.  Developers would
 use up the space gains quickly.  Right now a few architectures are
 neck and neck regarding which install media are close to full.  Older
 architectures would hit full install media issues first.  A smaller
 contingent of developers who take care of those architectures would
 have to deal with the fallout, creating further friction...

For comparison, on the slowest machine I currently have access to (a
500MHz ALIX) zopfli on the fastest setting (--i1) is 14.2 times slower
than gzip -9 (192 seconds vs 13.5) for the same kernel and produces
output that's 35702 bytes smaller.

Anyway, I'm not the one who has to deal with this either way so I'll
leave it there.

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: bzip2

2013-06-06 Thread Darren Tucker
On Thu, Jun 6, 2013 at 11:48 AM, Theo de Raadt dera...@cvs.openbsd.org wrote:
[...]
 If anyone thinks using this for the install or boot media is going to
 help, don't say a word until you can prove it on all platforms.

Has anyone looked at zopfli[1] for the install media?  It's a (apache
2 licensed) slightly better but much slower gzip compressor that still
produces gzip-compatible output.

For an amd64 ramdiskA it makes a bsd.gz that's 48k smaller than gzip
(in ~16 seconds) and should compatible with the bootloader.  I tried
it once in a vm and it booted ok.

$ time gzip -9cn bsd.strip bsd.gz
real0m0.648s
user0m0.610s
sys 0m0.000s

$ time zopfli -c bsd.strip bsd.zopfli.gz
real0m17.409s
user0m16.810s
sys 0m0.580s

$ ls -l bsd.gz bsd.zopfli.gz
-rw-r--r--  1 dtucker  wsrc  1349508 Jun  6 15:40 bsd.gz
-rw-r--r--  1 dtucker  wsrc  1310302 Jun  6 15:42 bsd.zopfli.gz

[1] https://code.google.com/p/zopfli/

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: vr(4) TX interrupt reduction

2013-01-17 Thread Darren Tucker
On Wed, Jan 16, 2013 at 01:38:01PM +1100, Darren Tucker wrote:
 On Mon, Jan 14, 2013 at 10:10:55PM +1100, Darren Tucker wrote:
  On my ALIX, it increase the IP routing throughput from 80Mbit/s to
  85Mbit/s while reducing the interrupt CPU usage from 99% to 80%.
 
 It turns out that due to an error on my part, most of this improvment
 was due to one of the test kernels being complied without POOL_DEBUG
 while the baseline one was.  Sigh.  I think there's still some gains
 to be had, though.

OK, here's another diff which does seem to help.  It seems that there's
two different bits in the TX descriptor that control interrupts.  Quoting
from the VT6105M spec:

TDES1 bit 23: Interrupt Control. 0: No interrupt when Transmit OK, 1:
Interrupt when Transmit OK

TDES3 bit 0: Interrupt Control. 0: issue interrupt for this packet.  1:
no interrupt generated

Why two bits with apparently similar functionality?  Beats me, but only
the second actually seems to do anything on my VT6105M here.   CPU usage
for routing 85 mbit/s drops CPU usage from about 75% to about 50%
(famous last words).  Your milage may vary and so on.

Index: if_vr.c
===
RCS file: /cvs/src/sys/dev/pci/if_vr.c,v
retrieving revision 1.124
diff -u -p -r1.124 if_vr.c
--- if_vr.c 16 Jan 2013 06:15:50 -  1.124
+++ if_vr.c 17 Jan 2013 04:15:05 -
@@ -724,7 +724,7 @@ vr_list_tx_init(struct vr_softc *sc)
cd = sc-vr_cdata;
ld = sc-vr_ldata;
 
-   cd-vr_tx_cnt = 0;
+   cd-vr_tx_cnt = cd-vr_tx_pkts = 0;
 
for (i = 0; i  VR_TX_LIST_CNT; i++) {
cd-vr_tx_chain[i].vr_ptr = ld-vr_tx_list[i];
@@ -1292,7 +1292,15 @@ vr_encap(struct vr_softc *sc, struct vr_
}
 
/* Set EOP on the last descriptor */
-   f-vr_ctl |= htole32(VR_TXCTL_LASTFRAG | VR_TXCTL_FINT);
+   f-vr_ctl |= htole32(VR_TXCTL_LASTFRAG);
+
+   /* set the disable-interrupt bit except on every Nth packet */
+   if (sc-vr_cdata.vr_tx_pkts++ % 16)
+   f-vr_next |= htole32(VR_TXNEXT_INTDISABLE);
+   else {
+   sc-vr_cdata.vr_tx_pkts = 0;
+   f-vr_ctl |= htole32(VR_TXCTL_FINT);
+   }
 
return (0);
 }
@@ -1581,6 +1589,11 @@ vr_watchdog(struct ifnet *ifp)
struct vr_softc *sc;
 
sc = ifp-if_softc;
+
+   /* Reclaim first as we don't request an interrupt for every packet. */
+   vr_txeof(sc);
+   if (sc-vr_cdata.vr_tx_cnt == 0);
+   return;
 
ifp-if_oerrors++;
printf(%s: watchdog timeout\n, sc-sc_dev.dv_xname);
Index: if_vrreg.h
===
RCS file: /cvs/src/sys/dev/pci/if_vrreg.h,v
retrieving revision 1.33
diff -u -p -r1.33 if_vrreg.h
--- if_vrreg.h  16 Jan 2013 05:25:57 -  1.33
+++ if_vrreg.h  17 Jan 2013 04:15:05 -
@@ -429,6 +429,9 @@ struct vr_desc {
 #define VR_TXCTL_LASTFRAG  0x0040
 #define VR_TXCTL_FINT  0x0080
 
+/* TDES3 aka vr_next */
+#define VR_TXNEXT_INTDISABLE   0x0001
+
 #define VR_MAXFRAGS8
 #define VR_RX_LIST_CNT 128
 #define VR_TX_LIST_CNT 128
@@ -467,6 +470,7 @@ struct vr_chain_data {
struct vr_chain *vr_tx_cons;
struct vr_chain *vr_tx_prod;
int vr_tx_cnt;
+   int vr_tx_pkts;
 };
 
 struct vr_mii_frame {
-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: vr(4) TX interrupt reduction

2013-01-17 Thread Darren Tucker
On Thu, Jan 17, 2013 at 09:34:32PM +1100, Darren Tucker wrote:
 OK, here's another diff which does seem to help.  It seems that there's
 two different bits in the TX descriptor that control interrupts.  Quoting
 from the VT6105M spec:

Thanks to Mark Patruck for noticing that the previous patch didn't
actually help, due to a bug I introduced in a last minute obviously
correct clean up.

Index: dev/pci/if_vr.c
===
RCS file: /cvs/src/sys/dev/pci/if_vr.c,v
retrieving revision 1.124
diff -u -p -r1.124 if_vr.c
--- dev/pci/if_vr.c 16 Jan 2013 06:15:50 -  1.124
+++ dev/pci/if_vr.c 17 Jan 2013 21:54:19 -
@@ -724,7 +724,7 @@ vr_list_tx_init(struct vr_softc *sc)
cd = sc-vr_cdata;
ld = sc-vr_ldata;
 
-   cd-vr_tx_cnt = 0;
+   cd-vr_tx_cnt = cd-vr_tx_pkts = 0;
 
for (i = 0; i  VR_TX_LIST_CNT; i++) {
cd-vr_tx_chain[i].vr_ptr = ld-vr_tx_list[i];
@@ -1292,7 +1292,15 @@ vr_encap(struct vr_softc *sc, struct vr_
}
 
/* Set EOP on the last descriptor */
-   f-vr_ctl |= htole32(VR_TXCTL_LASTFRAG | VR_TXCTL_FINT);
+   f-vr_ctl |= htole32(VR_TXCTL_LASTFRAG);
+
+   /* set the disable-interrupt bit except on every Nth packet */
+   if (sc-vr_cdata.vr_tx_pkts++  8)
+   f-vr_next |= htole32(VR_TXNEXT_INTDISABLE);
+   else {
+   sc-vr_cdata.vr_tx_pkts = 0;
+   f-vr_ctl |= htole32(VR_TXCTL_FINT);
+   }
 
return (0);
 }
@@ -1581,6 +1589,11 @@ vr_watchdog(struct ifnet *ifp)
struct vr_softc *sc;
 
sc = ifp-if_softc;
+
+   /* Reclaim first as we don't request an interrupt for every packet. */
+   vr_txeof(sc);
+   if (sc-vr_cdata.vr_tx_cnt == 0);
+   return;
 
ifp-if_oerrors++;
printf(%s: watchdog timeout\n, sc-sc_dev.dv_xname);

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: vr(4) TX interrupt reduction

2013-01-17 Thread Darren Tucker
On Fri, Jan 18, 2013 at 09:00:25AM +1100, Darren Tucker wrote:
 Thanks to Mark Patruck for noticing that the previous patch didn't
 actually help, due to a bug I introduced in a last minute obviously
 correct clean up.

The turd polishing continues unabated.  This adds the interrupt disable
bit to descriptors where there's more than one mbuf in a packet.

Index: dev/pci/if_vr.c
===
RCS file: /cvs/src/sys/dev/pci/if_vr.c,v
retrieving revision 1.125
diff -u -p -r1.125 if_vr.c
--- dev/pci/if_vr.c 17 Jan 2013 21:49:48 -  1.125
+++ dev/pci/if_vr.c 18 Jan 2013 02:04:55 -
@@ -724,7 +724,7 @@ vr_list_tx_init(struct vr_softc *sc)
cd = sc-vr_cdata;
ld = sc-vr_ldata;
 
-   cd-vr_tx_cnt = 0;
+   cd-vr_tx_cnt = cd-vr_tx_pkts = 0;
 
for (i = 0; i  VR_TX_LIST_CNT; i++) {
cd-vr_tx_chain[i].vr_ptr = ld-vr_tx_list[i];
@@ -1198,7 +1198,7 @@ vr_encap(struct vr_softc *sc, struct vr_
struct vr_chain *c = *cp;
struct vr_desc  *f = NULL;
struct mbuf *m_new = NULL;
-   u_int32_t   vr_ctl = 0, vr_status = 0;
+   u_int32_t   vr_ctl = 0, vr_status = 0, intdisable = 0;
bus_dmamap_ttxmap;
int i, runt = 0;
 
@@ -1259,6 +1259,15 @@ vr_encap(struct vr_softc *sc, struct vr_
}
 #endif
 
+   /*
+* We only want TX completion interrupts on every Nth packet.
+* We need to set VR_TXNEXT_INTDISABLE on every descriptor except
+* for the last discriptor of every Nth packet, where we set
+* VR_TXCTL_FINT.
+*/
+   if (sc-vr_cdata.vr_tx_pkts++ % 8 != 0)
+   intdisable = VR_TXNEXT_INTDISABLE;
+
if (m_new != NULL) {
m_freem(m_head);
 
@@ -1276,7 +1285,7 @@ vr_encap(struct vr_softc *sc, struct vr_
f-vr_ctl |= htole32(VR_TXCTL_FIRSTFRAG);
f-vr_status = htole32(vr_status);
f-vr_data = htole32(txmap-dm_segs[i].ds_addr);
-   f-vr_next = htole32(c-vr_nextdesc-vr_paddr);
+   f-vr_next = htole32(c-vr_nextdesc-vr_paddr | intdisable);
sc-vr_cdata.vr_tx_cnt++;
}
 
@@ -1288,12 +1297,15 @@ vr_encap(struct vr_softc *sc, struct vr_
VR_TXCTL_TLINK | vr_ctl);
f-vr_status = htole32(vr_status);
f-vr_data = 
htole32(sc-sc_zeromap.vrm_map-dm_segs[0].ds_addr);
-   f-vr_next = htole32(c-vr_nextdesc-vr_paddr);
+   f-vr_next = htole32(c-vr_nextdesc-vr_paddr | intdisable);
sc-vr_cdata.vr_tx_cnt++;
}
 
/* Set EOP on the last descriptor */
-   f-vr_ctl |= htole32(VR_TXCTL_LASTFRAG | VR_TXCTL_FINT);
+   f-vr_ctl |= htole32(VR_TXCTL_LASTFRAG);
+
+   if (!intdisable)
+   f-vr_ctl |= htole32(VR_TXCTL_FINT);
 
return (0);
 }
@@ -1582,6 +1594,15 @@ vr_watchdog(struct ifnet *ifp)
struct vr_softc *sc;
 
sc = ifp-if_softc;
+
+   /*
+* Because we're only asking for completion interrupts only every
+* few packets, occasionally the watchdog will fire when we have
+* some TX descriptors to reclaim, so check for that first.
+*/
+   vr_txeof(sc);
+   if (sc-vr_cdata.vr_tx_cnt == 0);
+   return;
 
ifp-if_oerrors++;
printf(%s: watchdog timeout\n, sc-sc_dev.dv_xname);
Index: dev/pci/if_vrreg.h
===
RCS file: /cvs/src/sys/dev/pci/if_vrreg.h,v
retrieving revision 1.33
diff -u -p -r1.33 if_vrreg.h
--- dev/pci/if_vrreg.h  16 Jan 2013 05:25:57 -  1.33
+++ dev/pci/if_vrreg.h  18 Jan 2013 02:04:55 -
@@ -429,6 +429,9 @@ struct vr_desc {
 #define VR_TXCTL_LASTFRAG  0x0040
 #define VR_TXCTL_FINT  0x0080
 
+/* TDES3 aka vr_next */
+#define VR_TXNEXT_INTDISABLE   0x0001
+
 #define VR_MAXFRAGS8
 #define VR_RX_LIST_CNT 128
 #define VR_TX_LIST_CNT 128
@@ -467,6 +470,7 @@ struct vr_chain_data {
struct vr_chain *vr_tx_cons;
struct vr_chain *vr_tx_prod;
int vr_tx_cnt;
+   int vr_tx_pkts;
 };
 
 struct vr_mii_frame {

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: vr(4) TX interrupt reduction

2013-01-17 Thread Darren Tucker
On Fri, Jan 18, 2013 at 01:09:50PM +1100, Darren Tucker wrote:
 The turd polishing continues unabated.

and continues to continue.  This adds a quirk to frob the
interrupt-disable bit for VT6105M only.

After checking all of the spec sheets that I can find, I found the TX
interrupt disable bit (bit 0 in TDES3) in the specs for VT6105M,
VT6102 and VT8235.

It's not in the spec for VT6105 or VT6105LOM, however in those the
specs for the data pointer only include the top 30 bits, and bit 0
is unspecified.  It's not in VT86C100A03 at all.

What does all this mean?  beats me, but I'm concerned that setting the
low bit in an address might confuse chips not expecting it, so this
enables it only for chips that have been tested.  For the others, it'll
still do FINT interrupt reduction, which is similar to what we already
do (and more or less what FreeBSD does now).

Routing TCP through an ALIX (iperf at 85Mbit/s), this reduced CPU usage
from ~70% to ~50% and reduced the total interrupt load from 21k/sec
to 13k/sec.  Locally generating 64 byte UDP packets (the worst case,
basically) increases the output rate by about 50%.

ok?

Index: dev/pci/if_vr.c
===
RCS file: /cvs/src/sys/dev/pci/if_vr.c,v
retrieving revision 1.125
diff -u -p -r1.125 if_vr.c
--- dev/pci/if_vr.c 17 Jan 2013 21:49:48 -  1.125
+++ dev/pci/if_vr.c 18 Jan 2013 05:14:18 -
@@ -163,6 +163,7 @@ int vr_alloc_mbuf(struct vr_softc *, str
 #defineVR_Q_CSUM   (11)
 #defineVR_Q_CAM(12)
 #defineVR_Q_HWTAG  (13)
+#defineVR_Q_INTDISABLE (14)
 
 struct vr_type {
pci_vendor_id_t vr_vid;
@@ -178,7 +179,7 @@ struct vr_type {
{ PCI_VENDOR_VIATECH, PCI_PRODUCT_VIATECH_VT6105,
0 },
{ PCI_VENDOR_VIATECH, PCI_PRODUCT_VIATECH_VT6105M,
-   VR_Q_CSUM | VR_Q_CAM | VR_Q_HWTAG },
+   VR_Q_CSUM | VR_Q_CAM | VR_Q_HWTAG | VR_Q_INTDISABLE },
{ PCI_VENDOR_DELTA, PCI_PRODUCT_DELTA_RHINEII,
VR_Q_NEEDALIGN },
{ PCI_VENDOR_ADDTRON, PCI_PRODUCT_ADDTRON_RHINEII,
@@ -724,7 +725,7 @@ vr_list_tx_init(struct vr_softc *sc)
cd = sc-vr_cdata;
ld = sc-vr_ldata;
 
-   cd-vr_tx_cnt = 0;
+   cd-vr_tx_cnt = cd-vr_tx_pkts = 0;
 
for (i = 0; i  VR_TX_LIST_CNT; i++) {
cd-vr_tx_chain[i].vr_ptr = ld-vr_tx_list[i];
@@ -1198,7 +1199,7 @@ vr_encap(struct vr_softc *sc, struct vr_
struct vr_chain *c = *cp;
struct vr_desc  *f = NULL;
struct mbuf *m_new = NULL;
-   u_int32_t   vr_ctl = 0, vr_status = 0;
+   u_int32_t   vr_ctl = 0, vr_status = 0, intdisable = 0;
bus_dmamap_ttxmap;
int i, runt = 0;
 
@@ -1259,6 +1260,18 @@ vr_encap(struct vr_softc *sc, struct vr_
}
 #endif
 
+   /*
+* We only want TX completion interrupts on every Nth packet.
+* We need to set VR_TXNEXT_INTDISABLE on every descriptor except
+* for the last discriptor of every Nth packet, where we set
+* VR_TXCTL_FINT.  The former is in the specs for only some chips.
+* present: VT6102 VT6105M VT8235M
+* not present: VT86C100 6105LOM
+*/
+   if (++sc-vr_cdata.vr_tx_pkts % VR_TX_INTR_THRESH != 0 
+   sc-vr_quirks  VR_Q_INTDISABLE)
+   intdisable = VR_TXNEXT_INTDISABLE;
+
if (m_new != NULL) {
m_freem(m_head);
 
@@ -1276,7 +1289,7 @@ vr_encap(struct vr_softc *sc, struct vr_
f-vr_ctl |= htole32(VR_TXCTL_FIRSTFRAG);
f-vr_status = htole32(vr_status);
f-vr_data = htole32(txmap-dm_segs[i].ds_addr);
-   f-vr_next = htole32(c-vr_nextdesc-vr_paddr);
+   f-vr_next = htole32(c-vr_nextdesc-vr_paddr | intdisable);
sc-vr_cdata.vr_tx_cnt++;
}
 
@@ -1288,12 +1301,15 @@ vr_encap(struct vr_softc *sc, struct vr_
VR_TXCTL_TLINK | vr_ctl);
f-vr_status = htole32(vr_status);
f-vr_data = 
htole32(sc-sc_zeromap.vrm_map-dm_segs[0].ds_addr);
-   f-vr_next = htole32(c-vr_nextdesc-vr_paddr);
+   f-vr_next = htole32(c-vr_nextdesc-vr_paddr | intdisable);
sc-vr_cdata.vr_tx_cnt++;
}
 
/* Set EOP on the last descriptor */
-   f-vr_ctl |= htole32(VR_TXCTL_LASTFRAG | VR_TXCTL_FINT);
+   f-vr_ctl |= htole32(VR_TXCTL_LASTFRAG);
+
+   if (sc-vr_cdata.vr_tx_pkts % VR_TX_INTR_THRESH == 0)
+   f-vr_ctl |= htole32(VR_TXCTL_FINT);
 
return (0);
 }
@@ -1582,6 +1598,15 @@ vr_watchdog(struct ifnet *ifp)
struct vr_softc *sc;
 
sc = ifp-if_softc;
+
+   /*
+* Since we're only asking for completion interrupts only every
+* few packets, occasionally the watchdog will fire when we

Re: vr(4) TX interrupt reduction

2013-01-15 Thread Darren Tucker
On Mon, Jan 14, 2013 at 10:10:55PM +1100, Darren Tucker wrote:
 On my ALIX, it increase the IP routing throughput from 80Mbit/s to
 85Mbit/s while reducing the interrupt CPU usage from 99% to 80%.

It turns out that due to an error on my part, most of this improvment
was due to one of the test kernels being complied without POOL_DEBUG
while the baseline one was.  Sigh.  I think there's still some gains
to be had, though.

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



vr(4): avoid unnecessary PCI bus transactions

2013-01-15 Thread Darren Tucker
Hi all.

Right now, vr_start() pokes the chip to start transmitting if
vr_cdata.vr_tx_cnt isn't zero.  That's the number of used descriptors in
the TX ring, not the number of packets we just added, so we're
needlessly telling the chip to start transmitting in cases where it
already knows.

This decreases the cpu utilization by about 0.5%

ok?

Index: dev/pci/if_vr.c
===
RCS file: /cvs/src/sys/dev/pci/if_vr.c,v
retrieving revision 1.121
diff -u -p -r1.121 if_vr.c
--- dev/pci/if_vr.c 1 Dec 2012 09:55:03 -   1.121
+++ dev/pci/if_vr.c 16 Jan 2013 02:38:32 -
@@ -1273,6 +1273,7 @@ vr_start(struct ifnet *ifp)
struct vr_softc *sc;
struct mbuf *m_head;
struct vr_chain *cur_tx, *head_tx;
+   unsigned int packets_sent = 0;
 
sc = ifp-if_softc;
 
@@ -1298,6 +1299,7 @@ vr_start(struct ifnet *ifp)
IF_PREPEND(ifp-if_snd, m_head);
break;
}
+   packets_sent++;
 
/* Only set ownership bit on first descriptor */
head_tx-vr_ptr-vr_status |= htole32(VR_TXSTAT_OWN);
@@ -1313,7 +1315,7 @@ vr_start(struct ifnet *ifp)
 #endif
cur_tx = cur_tx-vr_nextdesc;
}
-   if (sc-vr_cdata.vr_tx_cnt != 0) {
+   if (packets_sent != 0) {
sc-vr_cdata.vr_tx_prod = cur_tx;
 
bus_dmamap_sync(sc-sc_dmat, sc-sc_listmap.vrm_map, 0,

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: hardware VLAN tagging for vr(4)

2013-01-15 Thread Darren Tucker
On Mon, Jan 14, 2013 at 02:42:54PM +1100, Darren Tucker wrote:
 Resurrecting this, I've now fixed the problem I introduced when I merged
 in your changes and have tested it.

Updated diff now incorporating feedback from brad jsing mikeb and
probably others.  Having corrected for the mistake of benchmarking
against a POOL_DEBUG kernel, this stil seems marginally better (71.3%
cpu vs 71.5%, which is admittedly within the margin of error).

Index: dev/pci/if_vr.c
===
RCS file: /cvs/src/sys/dev/pci/if_vr.c,v
retrieving revision 1.122
diff -u -p -r1.122 if_vr.c
--- dev/pci/if_vr.c 16 Jan 2013 03:21:14 -  1.122
+++ dev/pci/if_vr.c 16 Jan 2013 04:04:34 -
@@ -62,6 +62,7 @@
  */
 
 #include bpfilter.h
+#include vlan.h
 
 #include sys/param.h
 #include sys/systm.h
@@ -83,6 +84,11 @@
 #include net/if_dl.h
 #include net/if_media.h
 
+#if NVLAN  0
+#include net/if_types.h
+#include net/if_vlan_var.h
+#endif
+
 #if NBPFILTER  0
 #include net/bpf.h
 #endif
@@ -632,6 +638,12 @@ vr_attach(struct device *parent, struct 
ifp-if_capabilities |= IFCAP_CSUM_IPv4 | IFCAP_CSUM_TCPv4 |
IFCAP_CSUM_UDPv4;
 
+#if NVLAN  0
+   /* if the hardware can do VLAN tagging, say so. */
+   if (sc-vr_quirks  VR_Q_HWTAG)
+   ifp-if_capabilities |= IFCAP_VLAN_HWTAGGING;
+#endif
+
 #ifndef SMALL_KERNEL
if (sc-vr_revid = REV_ID_VT3065_A) {
ifp-if_capabilities |= IFCAP_WOL;
@@ -899,6 +911,7 @@ vr_rxeof(struct vr_softc *sc)
 #endif
 
ifp-if_ipackets++;
+
if (sc-vr_quirks  VR_Q_CSUM 
(rxstat  VR_RXSTAT_FRAG) == 0 
(rxctl  VR_RXCTL_IP) != 0) {
@@ -911,6 +924,21 @@ vr_rxeof(struct vr_softc *sc)
M_UDP_CSUM_IN_OK;
}
 
+#if NVLAN  0
+   /*
+* If there's a tagged packet, the 802.1q header will be at the
+* 4-byte boundary following the CRC.  There will be 2 bytes
+* TPID (0x8100) and 2 bytes TCI (including VLAN ID).
+* This isn't in the data sheet.
+*/
+   if (rxctl  VR_RXCTL_TAG) {
+   int offset = ((total_len + 3)  ~3) + ETHER_CRC_LEN + 2;
+   m-m_pkthdr.ether_vtag = htons(*(u_int16_t *)
+   ((u_int8_t *)m-m_data + offset));
+   m-m_flags |= M_VLANTAG;
+   }
+#endif
+
 #if NBPFILTER  0
/*
 * Handle BPF listeners. Let the BPF user see the packet.
@@ -1229,6 +1257,15 @@ vr_encap(struct vr_softc *sc, struct vr_
c-vr_mbuf = m_head;
txmap = c-vr_map;
for (i = 0; i  txmap-dm_nsegs; i++) {
+#if NVLAN  0
+   /* Tell chip to insert VLAN tag if needed. */
+   if (m_head-m_flags  M_VLANTAG) {
+   u_int32_t vtag = m_head-m_pkthdr.ether_vtag;
+   vtag = (vtag  VR_TXSTAT_PQSHIFT)  VR_TXSTAT_PQMASK;
+   vr_status |= vtag;
+   vr_ctl |= htole32(VR_TXCTL_INSERTTAG);
+   }
+#endif
if (i != 0)
*cp = c = c-vr_nextdesc;
f = c-vr_ptr;
@@ -1397,6 +1434,9 @@ vr_init(void *xsc)
 
VR_CLRBIT(sc, VR_TXCFG, VR_TXCFG_TX_THRESH);
VR_SETBIT(sc, VR_TXCFG, VR_TXTHRESH_STORENFWD);
+
+   if (ifp-if_capabilities  IFCAP_VLAN_HWTAGGING)
+   VR_SETBIT(sc, VR_TXCFG, VR_TXCFG_TXTAGEN);
 
/* Init circular RX list. */
if (vr_list_rx_init(sc) == ENOBUFS) {

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



vr(4) TX interrupt reduction

2013-01-14 Thread Darren Tucker
Hi all.

This patch below reduces the number of interrupts on the transmit side
of vr(4).  Currently we set the TX competion interrupt bit on each
outbound packet.  This patch changes it to only set the interrupt bit on
the last packet enqueued.  (One thing of note is that the interrupt bit
is set on the first of potentially several TX descriptors for a packet
whereas currently it's the last.  I'm not sure if this matters, but if
it does I can change it).

On my ALIX, it increase the IP routing throughput from 80Mbit/s to
85Mbit/s while reducing the interrupt CPU usage from 99% to 80%.

Testing on any VIA Rhine chips would be appreciated (especially ones
that are not 6105M like my ALIX).

(Help/suggestions from jsing@ chris@ mikeb@ dlg@...)

Thanks.

Index: dev/pci/if_vr.c
===
RCS file: /cvs/src/sys/dev/pci/if_vr.c,v
retrieving revision 1.121
diff -u -p -r1.121 if_vr.c
--- dev/pci/if_vr.c 1 Dec 2012 09:55:03 -   1.121
+++ dev/pci/if_vr.c 14 Jan 2013 10:50:58 -
@@ -1254,8 +1254,8 @@ vr_encap(struct vr_softc *sc, struct vr_
sc-vr_cdata.vr_tx_cnt++;
}
 
-   /* Set EOP on the last desciptor */
-   f-vr_ctl |= htole32(VR_TXCTL_LASTFRAG | VR_TXCTL_FINT);
+   /* Set EOP on the last descriptor */
+   f-vr_ctl |= htole32(VR_TXCTL_LASTFRAG);
 
return (0);
 }
@@ -1265,6 +1265,13 @@ vr_encap(struct vr_softc *sc, struct vr_
  * to the mbuf data regions directly in the transmit lists. We also save a
  * copy of the pointers since the transmit list fragment pointers are
  * physical addresses.
+ *
+ * Notes:
+ * We only set ownership bit on first descriptor for a each packet.
+ * We'll only want an interrupt for a given packet if either vr_encap fails
+ * (ie the descriptor ring is full) or if the interface input queue is
+ * empty, and we need to set the interrupt bit before setting the owner bit
+ * to turn it over to the hardware.
  */
 
 void
@@ -1272,7 +1279,7 @@ vr_start(struct ifnet *ifp)
 {
struct vr_softc *sc;
struct mbuf *m_head;
-   struct vr_chain *cur_tx, *head_tx;
+   struct vr_chain *cur_tx, *head_tx, *prev_tx = NULL;
 
sc = ifp-if_softc;
 
@@ -1298,9 +1305,9 @@ vr_start(struct ifnet *ifp)
IF_PREPEND(ifp-if_snd, m_head);
break;
}
-
-   /* Only set ownership bit on first descriptor */
-   head_tx-vr_ptr-vr_status |= htole32(VR_TXSTAT_OWN);
+   /* successfully enqueued new packet, we're done with previous */
+   if (prev_tx != NULL)
+   prev_tx-vr_ptr-vr_status |= htole32(VR_TXSTAT_OWN);
 
 #if NBPFILTER  0
/*
@@ -1312,7 +1319,13 @@ vr_start(struct ifnet *ifp)
BPF_DIRECTION_OUT);
 #endif
cur_tx = cur_tx-vr_nextdesc;
+   prev_tx = head_tx;
+   }
+   if (prev_tx != NULL) {
+   prev_tx-vr_ptr-vr_ctl |= htole32(VR_TXCTL_FINT);
+   prev_tx-vr_ptr-vr_status |= htole32(VR_TXSTAT_OWN);
}
+
if (sc-vr_cdata.vr_tx_cnt != 0) {
sc-vr_cdata.vr_tx_prod = cur_tx;
 
-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: vr(4) TX interrupt reduction

2013-01-14 Thread Darren Tucker
On Mon, Jan 14, 2013 at 10:10:55PM +1100, Darren Tucker wrote:
 This patch below reduces the number of interrupts on the transmit side
 of vr(4).  Currently we set the TX competion interrupt bit on each
 outbound packet.  This patch changes it to only set the interrupt bit on
 the last packet enqueued.  (One thing of note is that the interrupt bit
 is set on the first of potentially several TX descriptors for a packet
 whereas currently it's the last.  I'm not sure if this matters, but if
 it does I can change it).

Here's an updated diff where, when a single packet spans multiple TX
descriptors, it'll only sent the interrupt request bit on the final
discriptor (which matches the current behaviour for that case).

Index: dev/pci/if_vr.c
===
RCS file: /cvs/src/sys/dev/pci/if_vr.c,v
retrieving revision 1.121
diff -u -p -r1.121 if_vr.c
--- dev/pci/if_vr.c 1 Dec 2012 09:55:03 -   1.121
+++ dev/pci/if_vr.c 14 Jan 2013 23:23:57 -
@@ -1254,8 +1254,8 @@ vr_encap(struct vr_softc *sc, struct vr_
sc-vr_cdata.vr_tx_cnt++;
}
 
-   /* Set EOP on the last desciptor */
-   f-vr_ctl |= htole32(VR_TXCTL_LASTFRAG | VR_TXCTL_FINT);
+   /* Set EOP on the last descriptor */
+   f-vr_ctl |= htole32(VR_TXCTL_LASTFRAG);
 
return (0);
 }
@@ -1265,6 +1265,13 @@ vr_encap(struct vr_softc *sc, struct vr_
  * to the mbuf data regions directly in the transmit lists. We also save a
  * copy of the pointers since the transmit list fragment pointers are
  * physical addresses.
+ *
+ * Notes:
+ * We only set ownership bit on first descriptor for a each packet.
+ * We'll only want an interrupt for a given packet if either vr_encap fails
+ * (ie the descriptor ring is full) or if the interface input queue is
+ * empty, and we need to set the interrupt bit before setting the owner bit
+ * to turn it over to the hardware.
  */
 
 void
@@ -1273,6 +1280,7 @@ vr_start(struct ifnet *ifp)
struct vr_softc *sc;
struct mbuf *m_head;
struct vr_chain *cur_tx, *head_tx;
+   struct vr_chain *prev_head_tx = NULL, *prev_tail_tx = NULL;
 
sc = ifp-if_softc;
 
@@ -1298,9 +1306,10 @@ vr_start(struct ifnet *ifp)
IF_PREPEND(ifp-if_snd, m_head);
break;
}
-
-   /* Only set ownership bit on first descriptor */
-   head_tx-vr_ptr-vr_status |= htole32(VR_TXSTAT_OWN);
+   /* successfully enqueued new packet, we're done with previous */
+   if (prev_head_tx != NULL)
+   prev_head_tx-vr_ptr-vr_status |=
+   htole32(VR_TXSTAT_OWN);
 
 #if NBPFILTER  0
/*
@@ -1311,9 +1320,22 @@ vr_start(struct ifnet *ifp)
bpf_mtap_ether(ifp-if_bpf, head_tx-vr_mbuf,
BPF_DIRECTION_OUT);
 #endif
+   prev_head_tx = head_tx;
+   prev_tail_tx = cur_tx;
cur_tx = cur_tx-vr_nextdesc;
}
if (sc-vr_cdata.vr_tx_cnt != 0) {
+   /*
+* If we enqueued any packets, we want to set FINT on the last
+* descriptor for the last packet, but set TXOWN on the first
+* descriptor of the last packet.
+*/
+   if (prev_tail_tx != NULL)
+   prev_tail_tx-vr_ptr-vr_ctl |= htole32(VR_TXCTL_FINT);
+   if (prev_head_tx != NULL)
+   prev_head_tx-vr_ptr-vr_status |=
+   htole32(VR_TXSTAT_OWN);
+
sc-vr_cdata.vr_tx_prod = cur_tx;
 
bus_dmamap_sync(sc-sc_dmat, sc-sc_listmap.vrm_map, 0,

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: hardware VLAN tagging for vr(4)

2013-01-13 Thread Darren Tucker
 VR_TXSTAT_OWN  0x8000
+#define VR_TXSTAT_PQSHIFT  16
 
 #define VR_TXCTL_BUFLEN0x07FF
 #define VR_TXCTL_BUFLEN_EXT0x7800

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: [PATCH] OpenSSH: auth.c

2012-12-13 Thread Darren Tucker
On Thu, Dec 13, 2012 at 07:31:46PM +0100, Maxime Villard wrote:
 Hi,
 I was looking at some openssh code when I spotted a mistake

applied, thanks.

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



hardware VLAN tagging for vr(4)

2012-09-26 Thread Darren Tucker
 |= VR_TXCTL_IPCSUM;
@@ -1274,7 +1313,7 @@ vr_start(struct ifnet *ifp)
break;
}
 
-   VR_TXOWN(cur_tx) = htole32(VR_TXSTAT_OWN);
+   VR_TXOWN(cur_tx) |= htole32(VR_TXSTAT_OWN);
 
 #if NBPFILTER  0
/*
@@ -1394,6 +1433,11 @@ vr_init(void *xsc)
 * Program promiscuous mode and multicast filters.
 */
vr_iff(sc);
+
+#if NVLAN  0
+   if (ifp-if_capabilities  IFCAP_VLAN_HWTAGGING)
+   VR_SETBIT(sc, VR_TXCFG, VR_TXCFG_TXTAGEN);
+#endif
 
/*
 * Load the address of the RX list.
Index: sys/dev/pci/if_vrreg.h
===
RCS file: /cvs/src/sys/dev/pci/if_vrreg.h,v
retrieving revision 1.30
diff -u -p -r1.30 if_vrreg.h
--- sys/dev/pci/if_vrreg.h  5 Jan 2012 19:08:25 -   1.30
+++ sys/dev/pci/if_vrreg.h  26 Sep 2012 23:13:45 -
@@ -83,6 +83,9 @@
 #define VR_MPA_CNT 0x7C
 #define VR_CRC_CNT 0x7E
 #define VR_STICKHW 0x83
+#define VR_CAMMASK 0x88 /* 4 bytes */
+#define VR_CAMCTRL 0x92
+#define VR_CAMADDR 0x93
 
 /* Misc Registers */
 #define VR_MISC_CR10x81
@@ -342,6 +345,14 @@
 #define VR_BCR1_VLANFILT_ENB   0x80/* 6105M */
 
 /*
+ * CAM Control registers (VT6105M)
+ */
+#define VR_CAMCTRL_WREN0x01
+#define VR_CAMCTRL_VCAMSEL 0x02
+#define VR_CAMCTRL_WRITE   0x04
+#define VR_CAMCTRL_READ0x08
+
+/*
  * Rhine TX/RX list structure.
  */
 
@@ -404,6 +415,7 @@ struct vr_desc {
 #define VR_TXSTAT_ERRSUM   0x8000
 #define VR_TXSTAT_PQMASK   0x7FFF
 #define VR_TXSTAT_OWN  0x8000
+#define VR_TXSTAT_PQSHIFT  16
 
 #define VR_TXCTL_BUFLEN0x07FF
 #define VR_TXCTL_BUFLEN_EXT0x7800

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: Fix for sftp(1) tab-complete

2012-09-21 Thread Darren Tucker
On Tue, Sep 11, 2012 at 11:27 AM, Jean-Marc Robert
jeanmarc.rob...@gmail.com wrote:
 This is a diff that should fix a few issues I've encountered with sftp's
 tab-complete, and a few others that I found in the process.

Thanks, these have been committed (with some mild style adjustment).

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.