Re: diff: pfctl: error message for nonexisting rtable

2020-09-30 Thread Alexandr Nedvedicky
Hello,

On Wed, Sep 30, 2020 at 11:02:28PM +0200, Klemens Nanni wrote:
> On Sun, Sep 20, 2020 at 07:29:38PM +0200, Klemens Nanni wrote:
> > Rebased diff after yasouka's pfctl commit;  it still takes care of
> > rdomains only, but I'd appreciate folks using `on rdomain' in their
> > pf.conf test this.  If this works out I'd like to put it in shortly
> > after release and work on rtables next.
> Working for me so far, anyone else playing around with it?
> 
> I also checked CVS log and the existing rtable_l2() check goes back to
> claudio's introduction of `on rdomain N' in pf, i.e. it's been there
> from the start as a harmless but needless check and has not been added
> after the fact to fix anything.
> 
> The time is now, so here's an updated diff after sashan pointed out how
> I erroneously checked the rtable ID instead of the rdomain ID in the
> ioctl (copy/pasta mistake).
> 
> Feedback? OK?

fix looks OK to me.

thanks and
regards
sashan



[PATCH] umb(4) fix for X20 (DW5821e) in Dell Latitude 7300

2020-09-30 Thread Bryan Vyhmeister
Gerhard Roth provided a patch to me to get the Qualcomm Snapdragon X20
umb(4) interface in my Dell Latitude 7300 working. It is also known as a
Dell DW5821e interface. I have been using this patch for months now with
AT&T Wireless in the US and it works great just as I have been used to
on my ThinkPad X270 with the older umb(4) interface. This is how it
shows up now in my dmesg:

umb0 at uhub0 port 16 "Dell Inc. DW5821e Snapdragon X20 LTE" rev
3.10/3.18 addr 2

I would love to get this committed at some point so I no longer have to
keep compiling a new kernel for every snapshot to have umb(4) working.

Bryan



Index: sys/dev/usb/if_umb.c
===
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.35
diff -u -p -u -p -r1.35 if_umb.c
--- sys/dev/usb/if_umb.c10 Jul 2020 13:26:41 -  1.35
+++ sys/dev/usb/if_umb.c16 Jul 2020 12:44:33 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: if_umb.c,v 1.35 2020/07/10 13:26:41 patrick Exp $ */
+/* $OpenBSD: if_umb.c,v 1.34 2020/05/04 14:41:03 gerhard Exp $ */
 
 /*
  * Copyright (c) 2016 genua mbH
@@ -225,6 +225,26 @@ const struct cfattach umb_ca = {
 int umb_delay = 4000;
 
 /*
+ * Normally, MBIM devices are detected by their interface class and subclass.
+ * But for some models that have multiple configurations, it is better to
+ * match by vendor and product id so that we can select the desired
+ * configuration ourselves.
+ *
+ * OTOH, some devices identifiy themself als an MBIM device but fail to speak
+ * the MBIM protocol.
+ */
+struct umb_products {
+   struct usb_devno dev;
+   int  confno;
+};
+const struct umb_products umb_devs[] = {
+   { { USB_VENDOR_DELL, 0x81d7 }, 2 }, /* XXX FIXME */
+};
+
+#define umb_lookup(vid, pid)   \
+   ((const struct umb_products *)usb_lookup(umb_devs, vid, pid))
+
+/*
  * These devices require an "FCC Authentication" command.
  */
 const struct usb_devno umb_fccauth_devs[] = {
@@ -263,6 +283,8 @@ umb_match(struct device *parent, void *m
struct usb_attach_arg *uaa = aux;
usb_interface_descriptor_t *id;
 
+   if (umb_lookup(uaa->vendor, uaa->product) != NULL)
+   return UMATCH_VENDOR_PRODUCT;
if (!uaa->iface)
return UMATCH_NONE;
if ((id = usbd_get_interface_descriptor(uaa->iface)) == NULL)
@@ -315,6 +337,43 @@ umb_attach(struct device *parent, struct
sc->sc_ctrl_ifaceno = uaa->ifaceno;
ml_init(&sc->sc_tx_ml);
 
+   if (uaa->configno < 0) {
+   /*
+* In case the device was matched by VID/PID instead of
+* InterfaceClass/InterfaceSubClass, we have to pick the
+* correct configuration ourself.
+*/
+   uaa->configno = umb_lookup(uaa->vendor, uaa->product)->confno;
+   DPRINTF("%s: switching to config #%d\n", DEVNAM(sc),
+   uaa->configno);
+   status = usbd_set_config_no(sc->sc_udev, uaa->configno, 1);
+   if (status) {
+   printf("%s: failed to switch to config #%d: %s\n",
+   DEVNAM(sc), uaa->configno, usbd_errstr(status));
+   goto fail;
+   }
+   usbd_delay_ms(sc->sc_udev, 200);
+
+   /*
+* Need to do some manual setups that usbd_probe_and_attach()
+* would do for us otherwise.
+*/
+   uaa->nifaces = uaa->device->cdesc->bNumInterface;
+   for (i = 0; i < uaa->nifaces; i++) {
+   if (usbd_iface_claimed(sc->sc_udev, i))
+   continue;
+   id = 
usbd_get_interface_descriptor(&uaa->device->ifaces[i]);
+   if (id != NULL && id->bInterfaceClass == UICLASS_CDC &&
+   id->bInterfaceSubClass ==
+   UISUBCLASS_MOBILE_BROADBAND_INTERFACE_MODEL) {
+   uaa->iface = &uaa->device->ifaces[i];
+   uaa->ifaceno = 
uaa->iface->idesc->bInterfaceNumber;
+   sc->sc_ctrl_ifaceno = uaa->ifaceno;
+   break;
+   }
+   }
+   }
+
/*
 * Some MBIM hardware does not provide the mandatory CDC Union
 * Descriptor, so we also look at matching Interface
@@ -382,9 +441,9 @@ umb_attach(struct device *parent, struct
for (i = 0; i < uaa->nifaces; i++) {
if (usbd_iface_claimed(sc->sc_udev, i))
continue;
-   id = usbd_get_interface_descriptor(uaa->ifaces[i]);
+   id = usbd_get_interface_descriptor(&sc->sc_udev->ifaces[i]);
if (id != NULL && id->bInterfaceNumber == data_ifaceno) {
-   sc->sc_data_iface = uaa->ifaces[i];
+

Re: diff: pfctl: error message for nonexisting rtable

2020-09-30 Thread Klemens Nanni
On Sun, Sep 20, 2020 at 07:29:38PM +0200, Klemens Nanni wrote:
> Rebased diff after yasouka's pfctl commit;  it still takes care of
> rdomains only, but I'd appreciate folks using `on rdomain' in their
> pf.conf test this.  If this works out I'd like to put it in shortly
> after release and work on rtables next.
Working for me so far, anyone else playing around with it?

I also checked CVS log and the existing rtable_l2() check goes back to
claudio's introduction of `on rdomain N' in pf, i.e. it's been there
from the start as a harmless but needless check and has not been added
after the fact to fix anything.

The time is now, so here's an updated diff after sashan pointed out how
I erroneously checked the rtable ID instead of the rdomain ID in the
ioctl (copy/pasta mistake).

Feedback? OK?


Index: sys/net/pf_ioctl.c
===
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.356
diff -u -p -r1.356 pf_ioctl.c
--- sys/net/pf_ioctl.c  24 Aug 2020 15:41:15 -  1.356
+++ sys/net/pf_ioctl.c  30 Sep 2020 20:48:34 -
@@ -2820,10 +2820,8 @@ pf_rule_copyin(struct pf_rule *from, str
if (to->rtableid >= 0 && !rtable_exists(to->rtableid))
return (EBUSY);
to->onrdomain = from->onrdomain;
-   if (to->onrdomain >= 0 && !rtable_exists(to->onrdomain))
-   return (EBUSY);
-   if (to->onrdomain >= 0) /* make sure it is a real rdomain */
-   to->onrdomain = rtable_l2(to->onrdomain);
+   if (to->onrdomain < 0 || to->onrdomain > RT_TABLEID_MAX)
+   return (EINVAL);
 
for (i = 0; i < PFTM_MAX; i++)
to->timeout[i] = from->timeout[i];
Index: sbin/pfctl/parse.y
===
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.703
diff -u -p -r1.703 parse.y
--- sbin/pfctl/parse.y  17 Sep 2020 14:26:59 -  1.703
+++ sbin/pfctl/parse.y  20 Sep 2020 17:28:10 -
@@ -1216,7 +1216,7 @@ antispoof_opt : LABEL label   {
if ($2 < 0 || $2 > RT_TABLEID_MAX) {
yyerror("invalid rtable id");
YYERROR;
-   } else if (lookup_rtable($2) < 1) {
+   } else if (!lookup_rtable($2)) {
yyerror("rtable %lld does not exist", $2);
YYERROR;
}
@@ -2003,7 +2003,7 @@ filter_opt: USER uids {
if ($2 < 0 || $2 > RT_TABLEID_MAX) {
yyerror("invalid rtable id");
YYERROR;
-   } else if (lookup_rtable($2) < 1) {
+   } else if (!lookup_rtable($2)) {
yyerror("rtable %lld does not exist", $2);
YYERROR;
}
@@ -2481,8 +2481,6 @@ if_item   : STRING{
| RDOMAIN NUMBER{
if ($2 < 0 || $2 > RT_TABLEID_MAX)
yyerror("rdomain %lld outside range", $2);
-   else if (lookup_rtable($2) != 2)
-   yyerror("rdomain %lld does not exist", $2);
 
$$ = calloc(1, sizeof(struct node_if));
if ($$ == NULL)
@@ -5899,10 +5897,6 @@ lookup_rtable(u_int rtableid)
return 0;
}
err(1, "%s", __func__);
-   }
-   if (info.rti_domainid == rtableid) {
-   found[rtableid] = 2;
-   return 2;
}
found[rtableid] = 1;
return 1;