Re: is this rge crash known? - fixed

2022-12-19 Thread Nick Owens
patch also works fine on my odroid-h2+. it worked fine before, but
there is no regression here. thanks!

On Mon, Dec 19, 2022 at 10:04 PM Geoff Steckel  wrote:
>
> On 12/19/22 21:07, Kevin Lo wrote:
> > On Mon, Dec 19, 2022 at 03:50:45PM -0500, Geoff Steckel wrote:
> >> Thanks for all the suggestions:
> >>
> >> sysctl kern.pool_debug=1 = no change
> >> known working board in same slot = no change
> >>
> >> hardware version is indeed 0609
> >> em(4) in same slot = works
> >> test using old rge(4) board between two Linux systems = works
> >>
> >> Are any other drivers similar enough for me to compare with if_rge.c?
> >> Perhaps the AMD 5600G or the B550 chipset have quirks not seen before?
> >>
> >> I could possibly install FreeBSD if that would give any information.
> > The diff below syncs up the Rx descriptor setup code to the upstream.
> > It should fix the problem.
> >
> > Index: sys/dev/pci/if_rge.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/if_rge.c,v
> > retrieving revision 1.20
> > diff -u -p -u -p -r1.20 if_rge.c
> > --- sys/dev/pci/if_rge.c  20 Nov 2022 23:47:51 -  1.20
> > +++ sys/dev/pci/if_rge.c  20 Dec 2022 01:54:30 -
> > @@ -1104,24 +1104,16 @@ rge_newbuf(struct rge_queues *q)
> >   /* Map the segments into RX descriptors. */
> >   r = >q_rx.rge_rx_list[idx];
> >
> > - if (RGE_OWN(r)) {
> > - printf("%s: tried to map busy RX descriptor\n",
> > - sc->sc_dev.dv_xname);
> > - m_freem(m);
> > - return (ENOBUFS);
> > - }
> > -
> >   rxq->rxq_mbuf = m;
> >
> > - r->rge_extsts = 0;
> > - r->rge_addrlo = htole32(RGE_ADDR_LO(rxmap->dm_segs[0].ds_addr));
> > - r->rge_addrhi = htole32(RGE_ADDR_HI(rxmap->dm_segs[0].ds_addr));
> > + r->hi_qword1.rx_qword4.rge_extsts = 0;
> > + r->hi_qword0.rge_addr = htole64(rxmap->dm_segs[0].ds_addr);
> >
> > - r->rge_cmdsts = htole32(rxmap->dm_segs[0].ds_len);
> > + r->hi_qword1.rx_qword4.rge_cmdsts = htole32(rxmap->dm_segs[0].ds_len);
> >   if (idx == RGE_RX_LIST_CNT - 1)
> > - r->rge_cmdsts |= htole32(RGE_RDCMDSTS_EOR);
> > + r->hi_qword1.rx_qword4.rge_cmdsts |= 
> > htole32(RGE_RDCMDSTS_EOR);
> >
> > - r->rge_cmdsts |= htole32(RGE_RDCMDSTS_OWN);
> > + r->hi_qword1.rx_qword4.rge_cmdsts |= htole32(RGE_RDCMDSTS_OWN);
> >
> >   bus_dmamap_sync(sc->sc_dmat, q->q_rx.rge_rx_list_map,
> >   idx * sizeof(struct rge_rx_desc), sizeof(struct rge_rx_desc),
> > @@ -1140,11 +1132,11 @@ rge_discard_rxbuf(struct rge_queues *q,
> >
> >   r = >q_rx.rge_rx_list[idx];
> >
> > - r->rge_cmdsts = htole32(RGE_JUMBO_FRAMELEN);
> > - r->rge_extsts = 0;
> > + r->hi_qword1.rx_qword4.rge_cmdsts = htole32(RGE_JUMBO_FRAMELEN);
> > + r->hi_qword1.rx_qword4.rge_extsts = 0;
> >   if (idx == RGE_RX_LIST_CNT - 1)
> > - r->rge_cmdsts |= htole32(RGE_RDCMDSTS_EOR);
> > - r->rge_cmdsts |= htole32(RGE_RDCMDSTS_OWN);
> > + r->hi_qword1.rx_qword4.rge_cmdsts |= 
> > htole32(RGE_RDCMDSTS_EOR);
> > + r->hi_qword1.rx_qword4.rge_cmdsts |= htole32(RGE_RDCMDSTS_OWN);
> >
> >   bus_dmamap_sync(sc->sc_dmat, q->q_rx.rge_rx_list_map,
> >   idx * sizeof(struct rge_rx_desc), sizeof(struct rge_rx_desc),
> > @@ -1219,8 +1211,8 @@ rge_rxeof(struct rge_queues *q)
> >   if (RGE_OWN(cur_rx))
> >   break;
> >
> > - rxstat = letoh32(cur_rx->rge_cmdsts);
> > - extsts = letoh32(cur_rx->rge_extsts);
> > + rxstat = letoh32(cur_rx->hi_qword1.rx_qword4.rge_cmdsts);
> > + extsts = letoh32(cur_rx->hi_qword1.rx_qword4.rge_extsts);
> >
> >   total_len = RGE_RXBYTES(cur_rx);
> >   rxq = >q_rx.rge_rxq[i];
> > @@ -1282,16 +1274,16 @@ rge_rxeof(struct rge_queues *q)
> >   (total_len - ETHER_CRC_LEN);
> >
> >   /* Check IP header checksum. */
> > - if (!(rxstat & RGE_RDCMDSTS_IPCSUMERR) &&
> > + if (!(extsts & RGE_RDEXTSTS_IPCSUMERR) &&
> >   (extsts & RGE_RDEXTSTS_IPV4))
> >   m->m_pkthdr.csum_flags |= M_IPV4_CSUM_IN_OK;
> >
> >   /* Check TCP/UDP checksum. */
> >   if ((extsts & (RGE_RDEXTSTS_IPV4 | RGE_RDEXTSTS_IPV6)) &&
> > - (((rxstat & RGE_RDCMDSTS_TCPPKT) &&
> > - !(rxstat & RGE_RDCMDSTS_TCPCSUMERR)) ||
> > - ((rxstat & RGE_RDCMDSTS_UDPPKT) &&
> > - !(rxstat & RGE_RDCMDSTS_UDPCSUMERR
> > + (((extsts & RGE_RDEXTSTS_TCPPKT) &&
> > + !(extsts & RGE_RDEXTSTS_TCPCSUMERR)) ||
> > + ((extsts & RGE_RDEXTSTS_UDPPKT) &&
> > + !(extsts & RGE_RDEXTSTS_UDPCSUMERR
> >   m->m_pkthdr.csum_flags |= M_TCP_CSUM_IN_OK |
> >   M_UDP_CSUM_IN_OK;
> >
> > Index: 

Re: ifconfig.8: document veb(4)

2021-03-19 Thread Nick Owens
On Thu, Mar 11, 2021 at 2:15 AM David Gwynne  wrote:
>
>
>
> > On 11 Mar 2021, at 8:06 pm, Klemens Nanni  wrote:
> >
> > On Thu, Mar 11, 2021 at 01:46:34PM +1000, David Gwynne wrote:
> >
> >>> +.It Cm link0
> >>> +Disable the filtering of 802.1Q VLAN and QinQ SVLAN packets.
> >>> +.It Cm -link0
> >>> +Enable the filtering of 802.1Q VLAN and QinQ SVLAN packets.
> >>> +Packets will appear to enter or leave the member port interfaces.
> >>
> >> I dont think this line above should be here.
> > I removed this line, but it is also present in TPMR from which I copied
> > it?  Should it stay there?
>
> I don't think so. Free commit!
>
> >
> >>> +This is the default.
> >>> +.It Cm link1
> >>> +Enable the filtering of IPv4 and IPv6 packets with
> >>> +.Xr pf 4 .
> >>> +.It Cm -link1
> >>> +Disable the filtering of IPv4 and IPv6 packets with
> >>> +.Xr pf 4 .
> >>> +This is the default.
> >>
> >> Is it worth noting that vport(4) members of a veb(4) are different to 
> >> other ports? The answer to this question doesnt affect my ok, the diff 
> >> should go in and we can tweak this later.
> > I'll leave that to a separate diff.
>
> Cool.

ping. i set up veb recently with much success, but was disappointed by
the lack of documentation in ifconfig.



pf pledge and ioctls

2018-04-30 Thread Nick Owens
hi tech@,

i've written a program in go which gathers information from pf for
exporting to prometheus. my go program invokes several ioctls on /dev/pf to
do this. however, i recently looked at pledge'ing my program, but i've
noticed that not all of the pf ioctls that i use are included in the pf
pledge. currently the missing ioctls my program needs are DIOCGETQUEUES,
and DIOCGETQSTATS, but i wonder if there is a reason why these and other
ioctls are not included in the pf pledge.

so some questions - would there be a problem with adding these ioctls? what
about adding the rest of pf ioctls to the pf pledge? and last, would it
make sense to split the pf pledge to "pf" for full control and "rpf" for
read only access such as what my metrics gathering program needs?

nick


Re: vmd: reset queue_size if queue_select is invalid

2017-08-03 Thread Nick Owens
ping?

On Thu, Jul 27, 2017 at 10:20 AM, Mike Larkin <mlar...@azathoth.net> wrote:
> On Wed, Jul 26, 2017 at 09:37:30PM -0700, Nick Owens wrote:
>> hello tech@,
>>
>> here is a diff that will follow the virtio spec a little closer, and
>> allows 9front's (http://9front.org) virtio-blk driver to correctly find
>> the number of queues. i know that virtio-blk only has one queue, but
>> the virtio probing code is shared between virtio-blk and virtio-scsi.
>>
>> without this change, the size of the first queue is used for all
>> subsequently probed queues.
>>
>> for completeness i've changed rng and net to do the same as blk.
>>
>> some bits from the spec:
>>
>> 4.1.4.3.1 - "The device MUST present a 0 in queue_size if the virtqueue
>> corresponding to the current queue_select is unavailable."
>>
>> 4.1.5.1.3 - "Write the virtqueue index (first queue is 0) to
>> queue_select. Read the virtqueue size from queue_size. This controls
>> how big the virtqueue is (see 2.4 Virtqueues). If this field is 0, the
>> virtqueue does not exist."
>>
>
> vmd diffs are always welcome, thanks. I'll take a look at this later today.
>
> -ml
>
>
>> Index: virtio.c
>> ===
>> RCS file: /cvs/src/usr.sbin/vmd/virtio.c,v
>> retrieving revision 1.49
>> diff -u -p -u -p -r1.49 virtio.c
>> --- virtio.c  30 May 2017 17:56:47 -  1.49
>> +++ virtio.c  27 Jul 2017 04:35:46 -
>> @@ -150,8 +150,10 @@ void
>>  viornd_update_qs(void)
>>  {
>>   /* Invalid queue? */
>> - if (viornd.cfg.queue_select > 0)
>> + if (viornd.cfg.queue_select > 0) {
>> + viornd.cfg.queue_size = 0;
>>   return;
>> + }
>>
>>   /* Update queue address/size based on queue select */
>>   viornd.cfg.queue_address =
>> viornd.vq[viornd.cfg.queue_select].qa; @@ -324,8 +326,10 @@ void
>>  vioblk_update_qs(struct vioblk_dev *dev)
>>  {
>>   /* Invalid queue? */
>> - if (dev->cfg.queue_select > 0)
>> + if (dev->cfg.queue_select > 0) {
>> + dev->cfg.queue_size = 0;
>>   return;
>> + }
>>
>>   /* Update queue address/size based on queue select */
>>   dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;
>> @@ -1037,8 +1041,10 @@ void
>>  vionet_update_qs(struct vionet_dev *dev)
>>  {
>>   /* Invalid queue? */
>> - if (dev->cfg.queue_select > 1)
>> + if (dev->cfg.queue_select > 1) {
>> + dev->cfg.queue_size = 0;
>>   return;
>> + }
>>
>>   /* Update queue address/size based on queue select */
>>   dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;
>>



Re: [patch] httpd: don't add date header if already set

2017-07-29 Thread Nick Owens
ping?

On Jul 18, 2017 19:01, "Nick Owens" <misch...@offblast.org> wrote:

hello tech@,

here is a diff that will cause httpd's fcgi code to not set the HTTP
date header if it has already been set. the code i am using for an fcgi
server
(https://github.com/golang/go/blob/master/src/net/http/fcgi/child.go#L102)
unconditionally sets the Date header, so with httpd there is a
duplicate "Date:" header in responses.

quick glances at lighttpd and apache2 seem to agree with this behavior.

Index: server_fcgi.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
retrieving revision 1.74
diff -u -p -u -p -r1.74 server_fcgi.c
--- server_fcgi.c   21 Jan 2017 11:32:04 -  1.74
+++ server_fcgi.c   18 Jul 2017 21:31:01 -
@@ -661,8 +661,10 @@ server_fcgi_header(struct client *clt, u
}

/* Date header is mandatory and should be added as late as possible
*/
-   if (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
-   kv_add(>http_headers, "Date", tmbuf) == NULL)
+   key.kv_key = "Date";
+   if ((kv = kv_find(>http_headers, )) == NULL &&
+   (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
+   kv_add(>http_headers, "Date", tmbuf) == NULL))
return (-1);

/* Write initial header (fcgi might append more) */


vmd: reset queue_size if queue_select is invalid

2017-07-26 Thread Nick Owens
hello tech@,

here is a diff that will follow the virtio spec a little closer, and
allows 9front's (http://9front.org) virtio-blk driver to correctly find
the number of queues. i know that virtio-blk only has one queue, but
the virtio probing code is shared between virtio-blk and virtio-scsi.

without this change, the size of the first queue is used for all
subsequently probed queues.

for completeness i've changed rng and net to do the same as blk.

some bits from the spec:

4.1.4.3.1 - "The device MUST present a 0 in queue_size if the virtqueue
corresponding to the current queue_select is unavailable."

4.1.5.1.3 - "Write the virtqueue index (first queue is 0) to
queue_select. Read the virtqueue size from queue_size. This controls
how big the virtqueue is (see 2.4 Virtqueues). If this field is 0, the
virtqueue does not exist."

Index: virtio.c
===
RCS file: /cvs/src/usr.sbin/vmd/virtio.c,v
retrieving revision 1.49
diff -u -p -u -p -r1.49 virtio.c
--- virtio.c30 May 2017 17:56:47 -  1.49
+++ virtio.c27 Jul 2017 04:35:46 -
@@ -150,8 +150,10 @@ void
 viornd_update_qs(void)
 {
/* Invalid queue? */
-   if (viornd.cfg.queue_select > 0)
+   if (viornd.cfg.queue_select > 0) {
+   viornd.cfg.queue_size = 0;
return;
+   }
 
/* Update queue address/size based on queue select */
viornd.cfg.queue_address =
viornd.vq[viornd.cfg.queue_select].qa; @@ -324,8 +326,10 @@ void
 vioblk_update_qs(struct vioblk_dev *dev)
 {
/* Invalid queue? */
-   if (dev->cfg.queue_select > 0)
+   if (dev->cfg.queue_select > 0) {
+   dev->cfg.queue_size = 0;
return;
+   }
 
/* Update queue address/size based on queue select */
dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;
@@ -1037,8 +1041,10 @@ void
 vionet_update_qs(struct vionet_dev *dev)
 {
/* Invalid queue? */
-   if (dev->cfg.queue_select > 1)
+   if (dev->cfg.queue_select > 1) {
+   dev->cfg.queue_size = 0;
return;
+   }
 
/* Update queue address/size based on queue select */
dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;



[patch] httpd: don't add date header if already set

2017-07-18 Thread Nick Owens
hello tech@,

here is a diff that will cause httpd's fcgi code to not set the HTTP 
date header if it has already been set. the code i am using for an fcgi 
server 
(https://github.com/golang/go/blob/master/src/net/http/fcgi/child.go#L102) 
unconditionally sets the Date header, so with httpd there is a
duplicate "Date:" header in responses.

quick glances at lighttpd and apache2 seem to agree with this behavior.

Index: server_fcgi.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
retrieving revision 1.74
diff -u -p -u -p -r1.74 server_fcgi.c
--- server_fcgi.c   21 Jan 2017 11:32:04 -  1.74
+++ server_fcgi.c   18 Jul 2017 21:31:01 -
@@ -661,8 +661,10 @@ server_fcgi_header(struct client *clt, u
}
 
/* Date header is mandatory and should be added as late as possible */
-   if (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
-   kv_add(>http_headers, "Date", tmbuf) == NULL)
+   key.kv_key = "Date";
+   if ((kv = kv_find(>http_headers, )) == NULL &&
+   (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
+   kv_add(>http_headers, "Date", tmbuf) == NULL))
return (-1);
 
/* Write initial header (fcgi might append more) */