pfsync: comparing duration when "bulk-end"

2020-07-24 Thread YASUOKA Masahiko
Hi,

pfsync does "bulk update" just after boot, I noticed it sometimes
fails.  When finishing "bulk update", the duration in the "bulk-end"
packet and our duration based on uptime are compared, but that
comparision should be fixed.  It must consider the values are rounded
in a second.

ok?

Consider being rounded in a second when comparing the duration in
"bulk-end"  packet and the duration based on our uptime.  This fixes
the problem the carp demote count sometimes becomes 33 after reboot.

Index: sys/net/if_pfsync.c
===
RCS file: /cvs/src/sys/net/if_pfsync.c,v
retrieving revision 1.274
diff -u -p -r1.274 if_pfsync.c
--- sys/net/if_pfsync.c 10 Jul 2020 13:26:42 -  1.274
+++ sys/net/if_pfsync.c 25 Jul 2020 05:09:47 -
@@ -1169,8 +1169,7 @@ pfsync_in_bus(caddr_t buf, int len, int 
break;
 
case PFSYNC_BUS_END:
-   if (getuptime() - ntohl(bus->endtime) >=
-   sc->sc_ureq_sent) {
+   if (ntohl(bus->endtime) <= getuptime() + 1 - sc->sc_ureq_sent) {
/* that's it, we're happy */
sc->sc_ureq_sent = 0;
sc->sc_bulk_tries = 0;



Re: Add ability to set control values with video(1)

2020-07-24 Thread Marcus Glocker
On Fri, 24 Jul 2020 15:13:19 +0100
Laurence Tratt  wrote:

> On Thu, Jul 23, 2020 at 09:56:39PM +0200, Marcus Glocker wrote:
> 
> Hello Marcus,
> 
> > We read the current value of the white balance temperature auto
> > control (since this gets set correctly during the reset), and just
> > reset it again on the next cam start up after the video stream has
> > been started? By that we avoid the video stream on/off dance when
> > using '-c'.  
> 
> I missed an obvious scenario :/ "video -c reset" then "use the webcam
> in a browser" (or ffplay or whatever). Depending on your webcam, this
> won't appear to have done a full reset in such a scenario?

That's true and basically out of our control I would say ...

Anyway, matthieu@ came up with an idea yesterday which I like and also
think would be the better approach finally:

Instead of introducing the CLI parameter controls in video(1), we could
introduce videoctl(1) in base, same as we have with audioctl(1).  This
also gives us the possibility to only display the current video
parameters.

For things like the auto white balance temperature control we could
implement tweaks easier there, like only turning the stream on before we
set back auto white balance temperature to 1.

I also want to sound this idea here to check if somebody has objections
with that approach?

> 
> Laurie
> 

Marcus



Re: tcp_close: can we delay the reaper for 1 tick?

2020-07-24 Thread Alexander Bluhm
On Fri, Jul 24, 2020 at 01:20:29PM -0500, Scott Cheloha wrote:
> tcp_close() schedules the reaper timeout to run immediately.
> Does it need to run *immediately*?  Can it wait for one tick?

It does not matter.  Free has to happen after timeout thread has
been run.  Some other timeout may be waiting for netlock and reference
the TCB.  So I added the delay.

Just don't wait too long to avoid wasting resources.

OK bluhm@

> Index: tcp_subr.c
> ===
> RCS file: /cvs/src/sys/netinet/tcp_subr.c,v
> retrieving revision 1.174
> diff -u -p -r1.174 tcp_subr.c
> --- tcp_subr.c4 Oct 2018 17:33:41 -   1.174
> +++ tcp_subr.c24 Jul 2020 18:19:00 -
> @@ -518,7 +518,7 @@ tcp_close(struct tcpcb *tp)
>  
>   m_free(tp->t_template);
>   /* Free tcpcb after all pending timers have been run. */
> - TCP_TIMER_ARM(tp, TCPT_REAPER, 0);
> + TCP_TIMER_ARM(tp, TCPT_REAPER, 1);
>  
>   inp->inp_ppcb = NULL;
>   soisdisconnected(so);



tcp_close: can we delay the reaper for 1 tick?

2020-07-24 Thread Scott Cheloha
tcp_close() schedules the reaper timeout to run immediately.

Does it need to run *immediately*?  Can it wait for one tick?

I'm trying to eliminate these zero-tick timeouts because they
rely on undocumented behavior and impede an optimization I want
to make in the timeout layer.

This is the one I'm hitting most often so I'll start here.

Index: tcp_subr.c
===
RCS file: /cvs/src/sys/netinet/tcp_subr.c,v
retrieving revision 1.174
diff -u -p -r1.174 tcp_subr.c
--- tcp_subr.c  4 Oct 2018 17:33:41 -   1.174
+++ tcp_subr.c  24 Jul 2020 18:19:00 -
@@ -518,7 +518,7 @@ tcp_close(struct tcpcb *tp)
 
m_free(tp->t_template);
/* Free tcpcb after all pending timers have been run. */
-   TCP_TIMER_ARM(tp, TCPT_REAPER, 0);
+   TCP_TIMER_ARM(tp, TCPT_REAPER, 1);
 
inp->inp_ppcb = NULL;
soisdisconnected(so);



Re: acme-client config grammar improvements

2020-07-24 Thread sven falempin
On Fri, Jul 24, 2020 at 10:47 AM Florian Obser  wrote:

> On Thu, Jul 16, 2020 at 07:40:35AM +0200, Daniel Eisele wrote:
> > Also it would be nice to have a feature to update all domains of the
> > config file. I currently do that in a shell script by parsing the output
> > of acme-client -nv with sed and then calling acme-client multiple times.
> >
> > Maybe an easy solution would be an option that prints the list of all
> > domains, so I can avoid the sed parsing, as this is prone to breaking.
>
> I'm not opposed to that. You will probably need to output some form of
> csv.
>
> Consider this:
>
> domain handle1-example.com {
> domain name example.com
> alternative names { www.example.com secure.example.com }
> domain key "/etc/ssl..." rsa
> }
> domain handle2-example.com {
> domain name example.com
> alternative names { mail.example.com }
> domain key "/etc/ssl..." ecdsa
> }
>
> Should it be output like this?
>
> handle1-example.com; example.com; www.example.com, secure.example.com
> handle2-example.com; example.com; mail.example.com
>
> Or this?
>
> handle1-example.com; example.com; www.example.com
> handle1-example.com; example.com; secure.example.com
> handle2-example.com; example.com; mail.example.com
>
>
> >
> > Another solution is obviously to just add an "update all" command line
> > option (or maybe even in the config?), but that is probably more
> > complicated to implement.
>
> I'm more worried that you will very soon end up with some form of exec
> plugin mechanism. Typically you need to do something when a cert is
> renewed (restart daemon).
>
> My acme-client.conf is generate by a config management system which
> also creates individual cronjobs for each renew job and knows how to
> handle a cert renew.
>
> >
> > What do you think about that?
> >
>

A management system may auto reload services when the configuration files
changes , an update all would be convenient.

Moreover

Acme-client update && rcctl reload nginx

Once a week is easy , as acme-client will not return 0 if nothing is
changed .




> --
> I'm not entirely sure you are real.
>
> --
--
-
Knowing is not enough; we must apply. Willing is not enough; we must do


Re: acme-client config grammar improvements

2020-07-24 Thread Florian Obser
On Thu, Jul 16, 2020 at 07:40:35AM +0200, Daniel Eisele wrote:
> Also it would be nice to have a feature to update all domains of the
> config file. I currently do that in a shell script by parsing the output
> of acme-client -nv with sed and then calling acme-client multiple times.
> 
> Maybe an easy solution would be an option that prints the list of all
> domains, so I can avoid the sed parsing, as this is prone to breaking.

I'm not opposed to that. You will probably need to output some form of
csv.

Consider this:

domain handle1-example.com {
domain name example.com
alternative names { www.example.com secure.example.com }
domain key "/etc/ssl..." rsa
}
domain handle2-example.com {
domain name example.com
alternative names { mail.example.com }
domain key "/etc/ssl..." ecdsa
}

Should it be output like this?

handle1-example.com; example.com; www.example.com, secure.example.com
handle2-example.com; example.com; mail.example.com

Or this?

handle1-example.com; example.com; www.example.com
handle1-example.com; example.com; secure.example.com
handle2-example.com; example.com; mail.example.com


> 
> Another solution is obviously to just add an "update all" command line
> option (or maybe even in the config?), but that is probably more
> complicated to implement.

I'm more worried that you will very soon end up with some form of exec
plugin mechanism. Typically you need to do something when a cert is
renewed (restart daemon).

My acme-client.conf is generate by a config management system which
also creates individual cronjobs for each renew job and knows how to
handle a cert renew.

> 
> What do you think about that?
> 

-- 
I'm not entirely sure you are real.



Re: acme-client config grammar improvements

2020-07-24 Thread Florian Obser
On Tue, Jul 21, 2020 at 10:24:25PM +0200, Sebastian Benoit wrote:
> Daniel Eisele(daniel_eis...@gmx.de) on 2020.07.16 07:40:35 +0200:
> > Am 15.07.2020 um 23:51 schrieb Sebastian Benoit:
> > >> src/usr.sbin/acme-client/parse.y:
> > >> * The grammar allows the user to omit the newline after the first line
> > >>   in a domain or authority block.
> > >
> > > Yes. I'm still pnodering this. What are the chances that someone does 
> > > that?
> > >
> > > Probably no one does, but it worthwhile to break someones config for such 
> > > a
> > > change?
> > 
> > I can't imagine that anyone uses this, I only noticed it by reading the
> > grammar. But still not an important fix, I just noticed it and wanted to
> > point it out...
> > 
> > >> src/usr.sbin/acme-client/dbg.c doesn't build because in the included
> > >> header file extern.h the type pid_t is missing (unistd.h).
> > >
> > > extern.h should #include  for that, no?
> > 
> > Yes, sys/types.h is better, sorry about that.
> 
> So here is a patch i would like to commit.
> 
> ok?
> 
> diff --git usr.sbin/acme-client/dnsproc.c usr.sbin/acme-client/dnsproc.c
> index 664ef8d9b8b..72a0ea6b30e 100644
> --- usr.sbin/acme-client/dnsproc.c
> +++ usr.sbin/acme-client/dnsproc.c
> @@ -16,6 +16,7 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  
>  #include 
> diff --git usr.sbin/acme-client/extern.h usr.sbin/acme-client/extern.h
> index 529d3350205..76c86b5cce0 100644
> --- usr.sbin/acme-client/extern.h
> +++ usr.sbin/acme-client/extern.h
> @@ -17,6 +17,8 @@
>  #ifndef EXTERN_H
>  #define EXTERN_H
>  
> +#include 
> +
>  #include "parse.h"
> 

these two chunks are OK florian
 
>  #define MAX_SERVERS_DNS 8
> diff --git usr.sbin/acme-client/parse.y usr.sbin/acme-client/parse.y
> index 120f253a63f..0b96794f8da 100644
> --- usr.sbin/acme-client/parse.y
> +++ usr.sbin/acme-client/parse.y
> @@ -170,11 +170,11 @@ varset  : STRING '=' string {
>   }
>   ;
>  
> -optnl: '\n' optnl
> +optnl: nl
>   |
>   ;
>  
> -nl   : '\n' optnl/* one newline or more */
> +nl   : optnl '\n'/* one newline or more */
>   ;
> 

I'm not a fan of this.

We currently have two different optnl implementations in 24 parsers
(hostappd and radiusd are weird):

optnl   : '\n' optnl
|
;

and

optnl   : /* empty */
| '\n' optnl
;

I don't think we need a third one. Also it shouldn't depend on nl
because nl is not implemented in all parsers.

Can we go with this and leave nl alone because nl is not recursive
itself so there is no problem there.

optnl   : /* empty */
| optnl '\n'
;

I think we can copy that to all other parse.y.

The rest is OK florian
 
>  comma: ','
> @@ -190,7 +190,7 @@ authority : AUTHORITY STRING {
>   yyerror("authority already defined");
>   YYERROR;
>   }
> - } '{' optnl authorityopts_l '}' {
> + } '{' optnl authorityopts_l optnl '}' {
>   if (auth->api == NULL) {
>   yyerror("authority %s: no api URL specified",
>   auth->name);
> @@ -205,8 +205,8 @@ authority : AUTHORITY STRING {
>   }
>   ;
>  
> -authorityopts_l  : authorityopts_l authorityoptsl nl
> - | authorityoptsl optnl
> +authorityopts_l  : authorityopts_l nl authorityoptsl
> + | authorityoptsl
>   ;
>  
>  authorityoptsl   : API URL STRING {
> @@ -246,7 +246,7 @@ domain: DOMAIN STRING {
>   yyerror("domain already defined");
>   YYERROR;
>   }
> - } '{' optnl domainopts_l '}' {
> + } '{' optnl domainopts_l optnl '}' {
>   if (domain->domain == NULL) {
>   if ((domain->domain = strdup(domain->handle))
>   == NULL)
> @@ -273,8 +273,8 @@ keytype   : RSA   { $$ = KT_RSA; }
>   |   { $$ = KT_RSA; }
>   ;
>  
> -domainopts_l : domainopts_l domainoptsl nl
> - | domainoptsl optnl
> +domainopts_l : domainopts_l nl domainoptsl
> + | domainoptsl
>   ;
>  
>  domainoptsl  : ALTERNATIVE NAMES '{' altname_l '}'
> @@ -385,7 +385,7 @@ domainoptsl   : ALTERNATIVE NAMES '{' altname_l '}'
>   }
>   ;
>  
> -altname_l: altname comma altname_l
> +altname_l: altname_l comma altname
>   | altname
>   ;
>  

-- 
I'm not entirely sure you are real.



Re: pf: route-to {random,srchash} in an anchor

2020-07-24 Thread YASUOKA Masahiko
Hi,

On Thu, 23 Jul 2020 18:44:43 +0200
Alexandr Nedvedicky  wrote:
> On Thu, Jul 23, 2020 at 08:01:18PM +0900, YASUOKA Masahiko wrote:
>> Hi,
>> 
>> Last month, I fixed the problem "route-to least-state" in an anchor
>> didn't work.
>> 
>> https://marc.info/?t=15911745782=1=2
>> 
>> I noticed the same problem happens on "random" and "srchash" as well.
>> 
>> ok?
> 
> change looks good. I have just one nit-pick comment. I leave decision
> whether it's worth to adjust your diff or commit as-is up to you.
> 
> see in-line further below.

I can't remember why I used "null == false" logic, since I usually
avoid using that.

I'll commit the ajusted diff below.

Index: sys/net/pf_lb.c
===
RCS file: /cvs/src/sys/net/pf_lb.c,v
retrieving revision 1.65
diff -u -p -r1.65 pf_lb.c
--- sys/net/pf_lb.c 24 Jul 2020 14:06:33 -  1.65
+++ sys/net/pf_lb.c 24 Jul 2020 14:13:42 -
@@ -353,6 +353,7 @@ pf_map_addr(sa_family_t af, struct pf_ru
struct pf_addr   faddr;
struct pf_addr  *raddr = >addr.v.a.addr;
struct pf_addr  *rmask = >addr.v.a.mask;
+   struct pfr_ktable   *kt;
struct pfi_kif  *kif;
u_int64_tstates;
u_int16_tweight;
@@ -405,18 +406,17 @@ pf_map_addr(sa_family_t af, struct pf_ru
pf_poolmask(naddr, raddr, rmask, saddr, af);
break;
case PF_POOL_RANDOM:
-   if (rpool->addr.type == PF_ADDR_TABLE) {
-   cnt = rpool->addr.p.tbl->pfrkt_cnt;
-   if (cnt == 0)
-   rpool->tblidx = 0;
+   if (rpool->addr.type == PF_ADDR_TABLE ||
+   rpool->addr.type == PF_ADDR_DYNIFTL) {
+   if (rpool->addr.type == PF_ADDR_TABLE)
+   kt = rpool->addr.p.tbl;
else
-   rpool->tblidx = (int)arc4random_uniform(cnt);
-   memset(>counter, 0, sizeof(rpool->counter));
-   if (pfr_pool_get(rpool, , , af))
+   kt = rpool->addr.p.dyn->pfid_kt;
+   kt = pfr_ktable_select_active(kt);
+   if (kt == NULL)
return (1);
-   pf_addrcpy(naddr, >counter, af);
-   } else if (rpool->addr.type == PF_ADDR_DYNIFTL) {
-   cnt = rpool->addr.p.dyn->pfid_kt->pfrkt_cnt;
+
+   cnt = kt->pfrkt_cnt;
if (cnt == 0)
rpool->tblidx = 0;
else
@@ -462,18 +462,18 @@ pf_map_addr(sa_family_t af, struct pf_ru
case PF_POOL_SRCHASH:
hashidx =
pf_hash(saddr, (struct pf_addr *), >key, af);
-   if (rpool->addr.type == PF_ADDR_TABLE) {
-   cnt = rpool->addr.p.tbl->pfrkt_cnt;
-   if (cnt == 0)
-   rpool->tblidx = 0;
+
+   if (rpool->addr.type == PF_ADDR_TABLE ||
+   rpool->addr.type == PF_ADDR_DYNIFTL) {
+   if (rpool->addr.type == PF_ADDR_TABLE)
+   kt = rpool->addr.p.tbl;
else
-   rpool->tblidx = (int)(hashidx % cnt);
-   memset(>counter, 0, sizeof(rpool->counter));
-   if (pfr_pool_get(rpool, , , af))
+   kt = rpool->addr.p.dyn->pfid_kt;
+   kt = pfr_ktable_select_active(kt);
+   if (kt == NULL)
return (1);
-   pf_addrcpy(naddr, >counter, af);
-   } else if (rpool->addr.type == PF_ADDR_DYNIFTL) {
-   cnt = rpool->addr.p.dyn->pfid_kt->pfrkt_cnt;
+
+   cnt = kt->pfrkt_cnt;
if (cnt == 0)
rpool->tblidx = 0;
else
Index: sys/net/pf_table.c
===
RCS file: /cvs/src/sys/net/pf_table.c,v
retrieving revision 1.133
diff -u -p -r1.133 pf_table.c
--- sys/net/pf_table.c  24 Jun 2020 22:03:43 -  1.133
+++ sys/net/pf_table.c  24 Jul 2020 14:13:42 -
@@ -2108,9 +2108,8 @@ pfr_kentry_byaddr(struct pfr_ktable *kt,
struct sockaddr_in6  tmp6;
 #endif /* INET6 */
 
-   if (!(kt->pfrkt_flags & PFR_TFLAG_ACTIVE) && kt->pfrkt_root != NULL)
-   kt = kt->pfrkt_root;
-   if (!(kt->pfrkt_flags & PFR_TFLAG_ACTIVE))
+   kt = pfr_ktable_select_active(kt);
+   if (kt == NULL)
return (0);
 
switch (af) {
@@ -2153,9 +2152,8 @@ pfr_update_stats(struct pfr_ktable *kt, 
int  dir_idx = 

Re: Add ability to set control values with video(1)

2020-07-24 Thread Laurence Tratt
On Thu, Jul 23, 2020 at 09:56:39PM +0200, Marcus Glocker wrote:

Hello Marcus,

> We read the current value of the white balance temperature auto control
> (since this gets set correctly during the reset), and just reset it again
> on the next cam start up after the video stream has been started?
> By that we avoid the video stream on/off dance when using '-c'.

I missed an obvious scenario :/ "video -c reset" then "use the webcam in a
browser" (or ffplay or whatever). Depending on your webcam, this won't appear
to have done a full reset in such a scenario?


Laurie



Re: pf: route-to least-states

2020-07-24 Thread Alexandr Nedvedicky
Hello Yasuoka,

> 
> Yes.
> 
> Let me simplify the block for "least-states".
> 


thanks for your explanation. It helped me to understand the code.

I'm OK with your fix.

thanks and
regards
sashan



Re: pf: route-to least-states

2020-07-24 Thread YASUOKA Masahiko
Hi,

Thank you for your review.

On Fri, 24 Jul 2020 01:25:42 +0200
Alexandr Nedvedicky  wrote:
>> - interface is not selected properly if selected table entry specifies
>>   an interface.
> 
> to be honest I don't quite understand what's going on here.
> can you share some details of configuration/scenario, which
> triggers the bug your diff is fixing?

You seem to have understood the scenario correctly.

> the part of your change, which I'm not able to figure out is
> this single line:
> 
>> +if (pf_map_addr_states_increase(af, rpool, naddr) == -1)
>> +return (1);
>> +/* revert the kif which was set by pfr_pool_get() */
>> +rpool->kif = kif;
>>  break;
>>  }
> 
> your fix changes behavior, which is there since least-state
> option has been introduced. I believe it does not matter
> for case when route-to specifies single interface such as:
> 
>   route-to 192.168.1.10@em0 least-states
> 
> I'm not sure what will happen in situation, when there are more interfaces
> specified in combination with sticky-address:
>   
>   route-to {192.168.1.10@em0, 192.168.1.20@em1} last-states sticky-address

Yes.  This is a senario.

> the resulting code does not look quite right with your diff applied:
> 
> 602 } while (pf_match_addr(1, , rmask, >counter, 
> af) &&
> 603 (states > 0));
> 604 
> 605 if (pf_map_addr_states_increase(af, rpool, naddr) == -1)
> 606 return (1);
> 607 /* revert the kif which was set by pfr_pool_get() */
> 608 rpool->kif = kif;
> 609 break;
> 610 }
> 611 
> 612 if (rpool->opts & PF_POOL_STICKYADDR) {
> 613 if (sns[type] != NULL) {
> 614 pf_remove_src_node(sns[type]);
> 615 sns[type] = NULL;
> 616 }
> 617 if (pf_insert_src_node([type], r, type, af, saddr, 
> naddr,
> 618 rpool->kif))
> 619 return (1);
> 620 }
> 
> 
> at line 608 new code reverts kif set by pfr_pool_get(). At line 617
> (executed when sticky-address is set) the original code passes kif chosen 
> be
> pfr_pool_get(). You diff changes that behavior. So my question is simple:
>   is that intentional change?

Yes.

Let me simplify the block for "least-states".

535   case PF_POOL_LEASTSTATES:
539   pfr_pool_get(rpool);  // fist entry
 :
561   faddr = rpool->counter;   //record as final
 :
557   load = rpool->states / rpool->weight;
563   naddr = rpool->counter;
 :
571  do {
572  rpool->counter++;
575  pfr_pool_get(rpool);   /* next entry */
 :
585  cload = rpool->states / rpool->weight;
 :
 :   /* find lc minimum */
591  if (cload < load) {
595 load = cload;
597 naddr = rpool->counter;
601  }
603   } while (raddr->counter != faddr); // loop until final

the loop #571:606 is to find the minimum (least-states) entry.  If the
last entry is not the minimum, after the loop,

   rpool <= the last entry
   naddr <= the minimum entry

Also see the pfr_pool_get():

2272 int
2273 pfr_pool_get(struct pf_pool *rpool, struct pf_addr **raddr,
2274 struct pf_addr **rmask, sa_family_t af)
2275 {
(snip)
2417 rpool->states = 0;
2418 if (ke->pfrke_counters != NULL)
2419 rpool->states = ke->pfrke_counters->states;
2420 switch (ke->pfrke_type) {
2421 case PFRKE_COST:
2422 rpool->weight =
2423 ((struct pfr_kentry_cost *)ke)->weight;
2424 /* FALLTHROUGH */
2425 case PFRKE_ROUTE:
2426 rpool->kif = ((struct pfr_kentry_route 
*)ke)->kif;
2427 break;
2428 default:
2429 rpool->weight = 1;
2430 break;
2431 }

some fields of rpool (states, weight, kif) are set by the values of
the selected table entry.

So,

|  rpool <= the last entry
|  naddr <= the minimum entry

rpool->kif is the interface of the last entery.  It might be different
than the inteface of the minimum entry.

The diff is to keep kif of the minimum entry,

+   kif = rpool->kif;

revert rpool->kif by it after the loop.

+   /* revert the kif which was set by pfr_pool_get() */
+   rpool->kif = kif;




Re: change ktime to nanoseconds in drm

2020-07-24 Thread Jonathan Gray
On Tue, Jul 21, 2020 at 05:10:02PM -0500, Scott Cheloha wrote:
> On Tue, Jul 21, 2020 at 07:33:21PM +1000, Jonathan Gray wrote:
> > Change from using timevals for ktime to 64 bit count of nanoseconds
> > to closer match linux.  From a discussion with cheloha@
> > 
> > 
> > 
> > @@ -67,70 +65,66 @@ ktime_get_raw_ns(void)
> >  }
> >  
> >  static inline struct timespec64
> > -ktime_to_timespec64(struct timeval tv)
> > +ktime_to_timespec64(ktime_t k)
> >  {
> > struct timespec64 ts;
> > -   ts.tv_sec = tv.tv_sec;
> > -   ts.tv_nsec = tv.tv_usec * NSEC_PER_USEC;
> > +   ts.tv_sec = k / NSEC_PER_SEC;
> > +   ts.tv_nsec = k % NSEC_PER_SEC;
> 
> This isn't quite right.  You need to handle negative values of k, too.
> 
>   ts.tv_sec = k / NSEC_PER_SEC;
>   ts.tv_nsec = k % NSEC_PER_SEC;
>   if (ts.tv_nsec < 0) {
>   ts.tv_sec--;
>   ts.tv_nsec += NSEC_PER_SEC;
>   }
> 
> 

ah right, tv_nsec should not be negative in that case

Index: drm_vblank.c
===
RCS file: /cvs/src/sys/dev/pci/drm/drm_vblank.c,v
retrieving revision 1.5
diff -u -p -r1.5 drm_vblank.c
--- drm_vblank.c8 Jun 2020 04:47:58 -   1.5
+++ drm_vblank.c24 Jul 2020 05:44:19 -
@@ -184,7 +184,7 @@ static void drm_reset_vblank_timestamp(s
 * interrupt and assign 0 for now, to mark the vblanktimestamp as 
invalid.
 */
if (!rc)
-   t_vblank = (struct timeval) {0, 0};
+   t_vblank = 0;
 
/*
 * +1 to make sure user will never see the same
@@ -293,7 +293,7 @@ static void drm_update_vblank_count(stru
 * for now, to mark the vblanktimestamp as invalid.
 */
if (!rc && !in_vblank_irq)
-   t_vblank = (struct timeval) {0, 0};
+   t_vblank = 0;
 
store_vblank(dev, pipe, diff, t_vblank, cur_vblank);
 }
@@ -871,7 +871,7 @@ static u64 drm_vblank_count_and_time(str
unsigned int seq;
 
if (WARN_ON(pipe >= dev->num_crtcs)) {
-   *vblanktime = (struct timeval) {0, 0};
+   *vblanktime = 0;
return 0;
}
 
Index: include/linux/ktime.h
===
RCS file: /cvs/src/sys/dev/pci/drm/include/linux/ktime.h,v
retrieving revision 1.4
diff -u -p -r1.4 ktime.h
--- include/linux/ktime.h   7 Jul 2020 04:05:25 -   1.4
+++ include/linux/ktime.h   24 Jul 2020 05:45:35 -
@@ -22,42 +22,40 @@
 #include 
 #include 
 
-typedef struct timeval ktime_t;
+typedef int64_t ktime_t;
 
-static inline struct timeval
+static inline ktime_t
 ktime_get(void)
 {
-   struct timeval tv;
-   
-   microuptime();
-   return tv;
+   struct timespec ts;
+   nanouptime();
+   return TIMESPEC_TO_NSEC();
 }
 
-static inline struct timeval
+static inline ktime_t
 ktime_get_raw(void)
 {
-   struct timeval tv;
-   
-   microuptime();
-   return tv;
+   struct timespec ts;
+   nanouptime();
+   return TIMESPEC_TO_NSEC();
 }
 
 static inline int64_t
-ktime_to_ms(struct timeval tv)
+ktime_to_ms(ktime_t k)
 {
-   return timeval_to_ms();
+   return k / NSEC_PER_MSEC;
 }
 
 static inline int64_t
-ktime_to_us(struct timeval tv)
+ktime_to_us(ktime_t k)
 {
-   return timeval_to_us();
+   return k / NSEC_PER_USEC;
 }
 
 static inline int64_t
-ktime_to_ns(struct timeval tv)
+ktime_to_ns(ktime_t k)
 {
-   return timeval_to_ns();
+   return k;
 }
 
 static inline int64_t
@@ -67,70 +65,70 @@ ktime_get_raw_ns(void)
 }
 
 static inline struct timespec64
-ktime_to_timespec64(struct timeval tv)
+ktime_to_timespec64(ktime_t k)
 {
struct timespec64 ts;
-   ts.tv_sec = tv.tv_sec;
-   ts.tv_nsec = tv.tv_usec * NSEC_PER_USEC;
+   ts.tv_sec = k / NSEC_PER_SEC;
+   ts.tv_nsec = k % NSEC_PER_SEC;
+   if (ts.tv_nsec < 0) {
+   ts.tv_sec--;
+   ts.tv_nsec += NSEC_PER_SEC;
+   }
return ts;
 }
 
-static inline struct timeval
-ktime_sub(struct timeval a, struct timeval b)
+static inline ktime_t
+ktime_sub(ktime_t a, ktime_t b)
 {
-   struct timeval res;
-   timersub(, , );
-   return res;
+   return a - b;
 }
 
-static inline struct timeval
-ktime_add(struct timeval a, struct timeval b)
+static inline ktime_t
+ktime_add(ktime_t a, ktime_t b)
 {
-   struct timeval res;
-   timeradd(, , );
-   return res;
+   return a + b;
 }
 
-static inline struct timeval
-ktime_add_us(struct timeval tv, int64_t us)
+static inline ktime_t
+ktime_add_us(ktime_t k, uint64_t us)
 {
-   return ns_to_timeval(timeval_to_ns() + (us * NSEC_PER_USEC));
+   return k + (us * NSEC_PER_USEC);
 }
 
-static inline struct timeval
-ktime_add_ns(struct timeval tv, int64_t ns)
+static inline ktime_t
+ktime_add_ns(ktime_t k, int64_t ns)
 {
-   return ns_to_timeval(timeval_to_ns() + ns);
+   return k + ns;
 }