route sourceaddr works with p2p interfaces
Hi,
route(8) sourceaddr is not used with p2p interfaces.
My initial fear was about tunnel interfaces but after some more testing, there
is no need to be so.
Here is the diff:
Index: sbin/route/route.8
===
RCS file: /cvs/src/sbin/route/route.8,v
retrieving revision 1.93
diff -u -p -r1.93 route.8
--- sbin/route/route.8 30 Oct 2020 14:30:51 - 1.93
+++ sbin/route/route.8 2 Nov 2020 19:53:34 -
@@ -234,8 +234,6 @@ The preferred source will not be used wh
.It
destination is on-link
.It
-output interface is point-to-point
-.It
source address is assigned to a disabled interface
.El
.El
Index: sys/netinet/in_pcb.c
===
RCS file: /cvs/src/sys/netinet/in_pcb.c,v
retrieving revision 1.250
diff -u -p -r1.250 in_pcb.c
--- sys/netinet/in_pcb.c29 Oct 2020 21:15:27 - 1.250
+++ sys/netinet/in_pcb.c2 Nov 2020 19:53:36 -
@@ -960,12 +960,10 @@ in_pcbselsrc(struct in_addr **insrc, str
/*
* Use preferred source address if :
* - destination is not onlink
-* - output interface is not PtoP
* - preferred source addresss is set
* - output interface is UP
*/
- if ((ro->ro_rt && !(ro->ro_rt->rt_flags & RTF_LLINFO)) &&
- (ia && !(ia->ia_ifp->if_flags & IFF_POINTOPOINT))) {
+ if (ro->ro_rt && !(ro->ro_rt->rt_flags & RTF_LLINFO)) {
ip4_source = rtable_getsource(rtableid, AF_INET);
if (ip4_source != NULL) {
struct ifaddr *ifa;
Index: sys/netinet6/in6_src.c
===
RCS file: /cvs/src/sys/netinet6/in6_src.c,v
retrieving revision 1.82
diff -u -p -r1.82 in6_src.c
--- sys/netinet6/in6_src.c 29 Oct 2020 21:15:27 - 1.82
+++ sys/netinet6/in6_src.c 2 Nov 2020 19:53:36 -
@@ -220,12 +220,10 @@ in6_pcbselsrc(struct in6_addr **in6src,
/*
* Use preferred source address if :
* - destination is not onlink
-* - output interface is not PtoP
* - preferred source addresss is set
* - output interface is UP
*/
- if ((ro->ro_rt && !(ro->ro_rt->rt_flags & RTF_LLINFO)) &&
- (ia6 && !(ia6->ia_ifp->if_flags & IFF_POINTOPOINT))) {
+ if (ro->ro_rt && !(ro->ro_rt->rt_flags & RTF_LLINFO)) {
ip6_source = rtable_getsource(rtableid, AF_INET6);
if (ip6_source != NULL) {
struct ifaddr *ifa;
Re: accton(8) requires a reboot after being enabled
Ingo Schwarze wrote: > Hi Theo, > > Theo de Raadt wrote on Fri, Oct 30, 2020 at 12:10:41PM -0600: > > > Yes, that diff is a whole bunch of TOCTUO. > > > > If this was going to be changed, it should be in the kernel. > > > > But I don't know if it should be changed yet, which is why I asked > > a bunch of questions. > > > > Stepping back to the man page change, we could decide that accton > > should continue to behave how it does, and this conversation started > > because the man page fell into the trap of documenting the rc scripts > > RATHER than documenting accton. > > Given that nobody seems in a rush to patch the kernel, i suggest to > improve the accton(8) manual page for now. In particular, that > manual page lacks the required EXIT STATUS section, which happens > to be a natural place for mentioning what happens if the file does > not exist. I guess so. > As usual, an EXAMPLES section is not strictly required, but in this > case, it seems useful to me, to save people from having to inspect there are only two ways to use this command. I don't think making one of them an example helps anymore. You have to be really dumb to misuse the command. I think this conversation came from a misguided origin. I think the man page documents accton correctly. The error occurs when it tries to document rc. But even that text is correct, it says: To have accton enabled at boot time Well if you don't reboot, it isn't enabled NOW, or it would have said "to enable and start accounting". I think this is making a mountain out of a mole hill.
Re: accton(8) requires a reboot after being enabled
On Mon, Nov 02, 2020 at 03:27:58PM +0100, Ingo Schwarze wrote: > Hi Theo, > > Theo de Raadt wrote on Fri, Oct 30, 2020 at 12:10:41PM -0600: > > > Yes, that diff is a whole bunch of TOCTUO. > > > > If this was going to be changed, it should be in the kernel. > > > > But I don't know if it should be changed yet, which is why I asked > > a bunch of questions. > > > > Stepping back to the man page change, we could decide that accton > > should continue to behave how it does, and this conversation started > > because the man page fell into the trap of documenting the rc scripts > > RATHER than documenting accton. > > Given that nobody seems in a rush to patch the kernel, i suggest to > improve the accton(8) manual page for now. In particular, that agreed > manual page lacks the required EXIT STATUS section, which happens > to be a natural place for mentioning what happens if the file does > not exist. > > As usual, an EXAMPLES section is not strictly required, but in this > case, it seems useful to me, to save people from having to inspect > /etc/mtree/special for the recommended file permissions. > > OK? > Ingo > i'm less sure here. first off the issue was about whether accton was documenting things it shouldn;t (rcctl etc.). as far as that issue is concerned, our man pages pretty much all got converted to the format accton is currently in. ntpd(8) is an exception. the fact that i prefer the wording in ntpd(8) is not really relevant - unless we agree that the approach is wrong and aim to change all relevant pages, i think the text in accton(8) should stay. as to your diff: - adding EXIT STATUS makes sense. i agree. but i don;t think it should discuss "file". firstly it'd be in isolation, so confuse people (what file?). secondly the logical place would surely be when we discuss the file argument. - adding EXAMPLES seems flawed. it's not an example of how to use it. if anything it belongs in the main body. but i don;t think it makes sense at all. so the only part i'm happy with is adding EXIT STATUS i don;t plan to take any action on the page as-is, for reasons explained above. jmc > > Index: accton.8 > === > RCS file: /cvs/src/usr.sbin/accton/accton.8,v > retrieving revision 1.12 > diff -u -r1.12 accton.8 > --- accton.8 2 Nov 2020 13:58:44 - 1.12 > +++ accton.8 2 Nov 2020 14:19:43 - > @@ -64,6 +64,17 @@ > .It Pa /var/account/acct > default accounting file > .El > +.Sh EXIT STATUS > +.Ex -std > +For example, it is an error if the > +.Ar file > +does not exist. > +.Sh EXAMPLES > +The following commands enable accounting if it was never used before: > +.Bd -literal -offset 4n > +# install -o root -g wheel -m 0644 /dev/null /var/account/acct > +# accton /var/account/acct > +.Ed > .Sh SEE ALSO > .Xr lastcomm 1 , > .Xr acct 2 , >
Re: Fix ilogb(3)
...snip... Here is a diff that fixes those issues by: ...snip... The code reads OK. Needs the manpage update to refer to FP_ILOGB0 not INT_MIN. John
Re: accton(8) requires a reboot after being enabled
Hi Theo, Theo de Raadt wrote on Fri, Oct 30, 2020 at 12:10:41PM -0600: > Yes, that diff is a whole bunch of TOCTUO. > > If this was going to be changed, it should be in the kernel. > > But I don't know if it should be changed yet, which is why I asked > a bunch of questions. > > Stepping back to the man page change, we could decide that accton > should continue to behave how it does, and this conversation started > because the man page fell into the trap of documenting the rc scripts > RATHER than documenting accton. Given that nobody seems in a rush to patch the kernel, i suggest to improve the accton(8) manual page for now. In particular, that manual page lacks the required EXIT STATUS section, which happens to be a natural place for mentioning what happens if the file does not exist. As usual, an EXAMPLES section is not strictly required, but in this case, it seems useful to me, to save people from having to inspect /etc/mtree/special for the recommended file permissions. OK? Ingo Index: accton.8 === RCS file: /cvs/src/usr.sbin/accton/accton.8,v retrieving revision 1.12 diff -u -r1.12 accton.8 --- accton.82 Nov 2020 13:58:44 - 1.12 +++ accton.82 Nov 2020 14:19:43 - @@ -64,6 +64,17 @@ .It Pa /var/account/acct default accounting file .El +.Sh EXIT STATUS +.Ex -std +For example, it is an error if the +.Ar file +does not exist. +.Sh EXAMPLES +The following commands enable accounting if it was never used before: +.Bd -literal -offset 4n +# install -o root -g wheel -m 0644 /dev/null /var/account/acct +# accton /var/account/acct +.Ed .Sh SEE ALSO .Xr lastcomm 1 , .Xr acct 2 ,
acme-client(1): replace httpd(8) location block in manpage by better match
The patch below updates the acme-client(1) manpage by providing a
closer match for the httpd(8) location block accepting acme challenge
responses.
Index: usr.sbin/acme-client/acme-client.1
===
RCS file: /cvs/src/usr.sbin/acme-client/acme-client.1,v
retrieving revision 1.34
diff -u -p -u -p -r1.34 acme-client.1
--- usr.sbin/acme-client/acme-client.1 10 May 2020 12:06:18 - 1.34
+++ usr.sbin/acme-client/acme-client.1 2 Nov 2020 13:18:12 -
@@ -14,7 +14,7 @@
.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
.\"
-.Dd $Mdocdate: May 10 2020 $
+.Dd $Mdocdate: November 2 2020 $
.Dt ACME-CLIENT 1
.Os
.Sh NAME
@@ -58,7 +58,7 @@ can be served by
with this location block,
which will properly map response challenges:
.Bd -literal -offset indent
-location "/.well-known/acme-challenge/*" {
+location found "/.well-known/acme-challenge/*" {
root "/acme"
request strip 2
}
Re: bpf_sysctl for sysctl_int_bounded
On Sun, 01 Nov 2020 19:31:16 -0800, Greg Steuck wrote: > Mildly different in flavor due to the special check. OK? OK millert@ - todd
Re: Move TCPCTL_ALWAYS_KEEPALIVE into tcpctl_vars
On Sun, 01 Nov 2020 19:29:23 -0800, Greg Steuck wrote: > This one was an omission from the earlier conversion, should be an easy > OK? OK millert@ - todd
OpenFlow 1.3 interoperability patch with FAUCET SDN controller
Hello,
Recently I tested OpenBSD's OpenFlow support with the FAUCET SDN controller
(https://faucet.nz).
There were some minor issues - OpenFlow spec ambiguity with length checks,
endianness of the cookie in the packet in message, not able to match a
packet with no VLAN, and matches running against a copy of the packet that
did not have pending set field changes between tables applied.
My tests were done with the FAUCET controller test suite. It passes the
basic switching tests, but there are some limitations I may be able to
address in the future (e.g. it supports only a single controller, and there
is no OFPortStatus message support so the controller doesn't know if a port
goes up or down at runtime).
I attach my patch, and a link to a PR for the FAUCET controller which would
allow us to add OpenBSD as a supported platform, which would be nice:
https://github.com/faucetsdn/faucet/pull/3728
I would appreciate your guidance and feedback, and hope to make further
changes in the future.
Thanks,
diff --git a/sys/net/ofp.h b/sys/net/ofp.h
index 8ba6ce1a90d..9d03a3c418e 100644
--- a/sys/net/ofp.h
+++ b/sys/net/ofp.h
@@ -480,6 +480,11 @@ struct ofp_flow_mod {
struct ofp_match fm_match;
} __packed;
+/* OpenFlow 1.3.5 secion 7.2.3.1 is ambiguous on whether length field,
+ includes length of type and length fields. A controller might send
+ either length 4 or length 0 for no matches. */
+#define OFP_MATCH_MIN_LEN 4
+
/* Flow removed reasons */
#define OFP_FLOWREM_REASON_IDLE_TIMEOUT 0 /* Flow idle time exceeded idle_timeout */
#define OFP_FLOWREM_REASON_HARD_TIMEOUT 1 /* Time exceeded hard_timeout */
diff --git a/sys/net/switchofp.c b/sys/net/switchofp.c
index ba7ee0df95f..32a218ed04c 100644
--- a/sys/net/switchofp.c
+++ b/sys/net/switchofp.c
@@ -2867,24 +2867,27 @@ swofp_ox_match_vlan_vid(struct switch_flow_classify *swfcl,
{
uint16_t in, mth, mask = 0;
+ memcpy(, oxm->oxm_value, sizeof(uint16_t));
+
+/*
+ * OpenFlow Switch Specification ver 1.3.5 says if oxm value
+ * is OFP_XM_VID_NONE, matches only packets without a VLAN tag
+ */
+ if (mth == htons(OFP_XM_VID_NONE)) {
+ if (swfcl->swfcl_vlan == NULL)
+ return (0);
+ }
+
if (swfcl->swfcl_vlan == NULL)
return (1);
- in = swfcl->swfcl_vlan->vlan_vid;
- memcpy(, oxm->oxm_value, sizeof(uint16_t));
-
if (OFP_OXM_GET_HASMASK(oxm))
memcpy(, oxm->oxm_value + sizeof(uint16_t),
sizeof(uint16_t));
else
mask = UINT16_MAX;
- /*
- * OpenFlow Switch Specification ver 1.3.5 says if oxm value
- * is OFP_XM_VID_NONE, matches only packets without a VLAN tag
- */
- if (mth == htons(OFP_XM_VID_NONE))
- return (1);
+ in = swfcl->swfcl_vlan->vlan_vid;
/*
* OpenFlow Switch Specification ver 1.3.5 says if oxm value and mask
@@ -3429,7 +3432,7 @@ swofp_action_output_controller(struct switch_softc *sc, struct mbuf *m0,
pin->pin_buffer_id = htonl(OFP_PKTOUT_NO_BUFFER);
pin->pin_table_id = swpld->swpld_table_id;
- pin->pin_cookie = swpld->swpld_cookie;
+ pin->pin_cookie = htobe64(swpld->swpld_cookie);
pin->pin_reason = reason;
if (frame_max_len) {
@@ -4616,6 +4619,7 @@ swofp_forward_ofs(struct switch_softc *sc, struct switch_flow_classify *swfcl,
TAILQ_FOREACH(swft, >swofs_table_list, swft_table_next) {
if (swft->swft_table_id != next_table_id)
continue;
+ int table_actions_pending = 0;
/* XXX
* The metadata is pipeline parameters but it uses same match
@@ -4646,6 +4650,7 @@ swofp_forward_ofs(struct switch_softc *sc, struct switch_flow_classify *swfcl,
swfe->swfe_apply_actions);
if (m == NULL)
goto out;
+ table_actions_pending = 1;
}
if (swfe->swfe_clear_actions) {
@@ -4658,6 +4663,7 @@ swofp_forward_ofs(struct switch_softc *sc, struct switch_flow_classify *swfcl,
if (swfe->swfe_write_actions) {
error = swofp_write_actions(
swfe->swfe_write_actions, swpld);
+ table_actions_pending = 1;
if (error)
goto out;
}
@@ -4669,6 +4675,10 @@ swofp_forward_ofs(struct switch_softc *sc, struct switch_flow_classify *swfcl,
next_table_id = swfe->swfe_goto_table->igt_table_id;
else
break;
+
+ if (table_actions_pending)
+ if ((m = swofp_apply_set_field(m, swpld)) == NULL)
+goto out;
}
m = swofp_execute_action_set(sc, m, swpld);
@@ -5127,7 +5137,7 @@ swofp_flow_mod_cmd_add(struct switch_softc *sc, struct mbuf *m)
* 2) OXM filters can't be bigger than the packet.
*/
if (omlen < sizeof(*om) ||
- omlen >= (m->m_len - sizeof(*ofm))) {
+ omlen > (m->m_len - sizeof(*ofm) + OFP_MATCH_MIN_LEN)) {
etype = OFP_ERRTYPE_BAD_MATCH;
error = OFP_ERRMATCH_BAD_LEN;
goto ofp_error;
@@ -5249,7 +5259,7 @@ swofp_flow_mod_cmd_common_modify(struct switch_softc *sc, struct mbuf *m,
* 2) OXM filters can't be bigger than the packet.
*/
if (omlen < sizeof(*om) ||
- omlen >= (m->m_len - sizeof(*ofm))) {
+ omlen > (m->m_len - sizeof(*ofm) + OFP_MATCH_MIN_LEN)) {
etype =
