Re: vmd: add minimal DHCP support
On 11/02/17 14:26, Reyk Floeter wrote: Hi, the problem got reported a few times and a similar diff was floating around: vmd's "local interface" implements a simple auto-magic BOOTP server, but udhcpc doesn't support BOOTP, which is the predecessor and a valid subset of DHCP. udhcpc is used by busybox and many embedded Linux distributions, maybe even by Android. The following diff adds minimal DHCP support which works with udhcpc. OK? Reyk Nice patch. +1 +--+ Carlos Index: usr.sbin/vmd/dhcp.c === RCS file: /cvs/src/usr.sbin/vmd/dhcp.c,v retrieving revision 1.3 diff -u -p -u -p -r1.3 dhcp.c --- usr.sbin/vmd/dhcp.c 24 Apr 2017 07:14:27 - 1.3 +++ usr.sbin/vmd/dhcp.c 2 Nov 2017 21:15:03 - @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -38,12 +39,13 @@ extern struct vmd *env; ssize_t dhcp_request(struct vionet_dev *dev, char *buf, size_t buflen, char **obuf) { - unsigned char *respbuf = NULL; + unsigned char *respbuf = NULL, *op, *oe, dhcptype = 0; ssize_t offset, respbuflen = 0; struct packet_ctxpc; struct dhcp_packet req, resp; - struct in_addr in, mask; + struct in_addr server_addr, mask, client_addr, requested_addr; size_t resplen, o; + uint32_t ltime; if (buflen < (ssize_t)(BOOTP_MIN_LEN + sizeof(struct ether_header))) return (-1); @@ -76,24 +78,54 @@ dhcp_request(struct vionet_dev *dev, cha if (req.ciaddr.s_addr != 0 || req.file[0] != '\0' || req.hops != 0) return (-1); + /* Get a few DHCP options (best effort as we fall back to BOOTP) */ + if (memcmp(, + DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN) == 0) { + memset(_addr, 0, sizeof(requested_addr)); + op = req.options + DHCP_OPTIONS_COOKIE_LEN; + oe = req.options + sizeof(req.options); + while (*op != DHO_END && op < oe) { + if (op[0] == DHO_PAD) { + op++; + continue; + } + if (op + 1 + op[1] >= oe) + break; + if (op[0] == DHO_DHCP_MESSAGE_TYPE && + op[1] == 1) + dhcptype = op[2]; + else if (op[0] == DHO_DHCP_REQUESTED_ADDRESS && + op[1] == sizeof(requested_addr)) + memcpy(_addr, [2], + sizeof(requested_addr)); + op += 2 + op[1]; + } + } + memset(, 0, sizeof(resp)); resp.op = BOOTREPLY; resp.htype = req.htype; resp.hlen = req.hlen; resp.xid = req.xid; - if ((in.s_addr = vm_priv_addr(>vmd_cfg.cfg_localprefix, + if ((client_addr.s_addr = + vm_priv_addr(>vmd_cfg.cfg_localprefix, dev->vm_vmid, dev->idx, 1)) == 0) return (-1); - memcpy(, , sizeof(in)); - memcpy((_dst)->sin_addr, , sizeof(in)); + memcpy(, _addr, + sizeof(client_addr)); + memcpy((_dst)->sin_addr, _addr, + sizeof(client_addr)); ss2sin(_dst)->sin_port = htons(CLIENT_PORT); - if ((in.s_addr = vm_priv_addr(>vmd_cfg.cfg_localprefix, + if ((server_addr.s_addr = + vm_priv_addr(>vmd_cfg.cfg_localprefix, dev->vm_vmid, dev->idx, 0)) == 0) return (-1); - memcpy(, , sizeof(in)); - memcpy((_src)->sin_addr, , sizeof(in)); + memcpy(, _addr, + sizeof(server_addr)); + memcpy((_src)->sin_addr, _addr, + sizeof(server_addr)); ss2sin(_src)->sin_port = htons(SERVER_PORT); /* Packet is already allocated */ @@ -124,6 +156,44 @@ dhcp_request(struct vionet_dev *dev, cha DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN); o+= DHCP_OPTIONS_COOKIE_LEN; + /* Did we receive a DHCP request or was it just BOOTP? */ + if (dhcptype) { + /* +* There is no need for a real state machine as we always +* answer with the same client IP and options for the VM. +*/ + if (dhcptype == DHCPDISCOVER) + dhcptype = DHCPOFFER; + else if (dhcptype == DHCPREQUEST && + (requested_addr.s_addr == 0 || + client_addr.s_addr == requested_addr.s_addr)) + dhcptype = DHCPACK; + else + dhcptype = DHCPNAK; + + resp.options[o++] = DHO_DHCP_MESSAGE_TYPE; + resp.options[o++] = sizeof(dhcptype); + memcpy([o], , sizeof(dhcptype)); +
[PATCH] VMD: remove debug message in proc
This debug message has served its duty for finding bugs in shutdown related issues. Ok reyk@ diff --git usr.sbin/vmd/proc.c usr.sbin/vmd/proc.c index afa9b223649..d223d6bcce2 100644 --- usr.sbin/vmd/proc.c +++ usr.sbin/vmd/proc.c @@ -756,8 +756,6 @@ proc_compose_imsg(struct privsep *ps, enum privsep_procid id, int n, proc_range(ps, id, , ); for (; n < m; n++) { - log_debug("%s: about to compose_event to proc %d", - __func__, id); if (imsg_compose_event(>ps_ievs[id][n], type, peerid, ps->ps_instance + 1, fd, data, datalen) == -1) return (-1); -- 2.14.3
[PATCH 1/2 v2] VMD: remove add from switch configuration
Remove configuration items on switches for: * adding static interfaces Adding static interfaces are to be set in hostname.if. Changed rule on rdomain: * vm->interface->rdomain takes precedence over sw->rdomain Update examples/vm.conf and vm.conf manpage to reflect changes. Comments? Ok? diff --git etc/examples/vm.conf etc/examples/vm.conf index f35235e3fba..d61ce8c4977 100644 --- etc/examples/vm.conf +++ etc/examples/vm.conf @@ -10,15 +10,14 @@ sets="/var/www/htdocs/pub/OpenBSD/snapshots/amd64/" # switch "uplink" { - # This interface will default to bridge0, but switch(4) is supported + # This switch will use bridge0, defined by /etc/hostname.bridge0, as + # the underlying interface. switch(4) is also supported #interface switch0 - - # Add additional members - add em0 + interface bridge0 } switch "local" { - add vether0 + interface bridge1 down } diff --git usr.sbin/vmd/parse.y usr.sbin/vmd/parse.y index a0e96545923..18543a74911 100644 --- usr.sbin/vmd/parse.y +++ usr.sbin/vmd/parse.y @@ -88,7 +88,6 @@ intparse_disk(char *); static struct vmop_create_params vmc; static struct vm_create_params *vcp; static struct vmd_switch *vsw; -static struct vmd_if *vif; static struct vmd_vm *vm; static char vsw_type[IF_NAMESIZE]; static int vcp_disable; @@ -193,7 +192,6 @@ switch : SWITCH string { vsw->sw_id = env->vmd_nswitches + 1; vsw->sw_name = $2; vsw->sw_flags = VMIFF_UP; - TAILQ_INIT(>sw_ifs); vcp_disable = 0; } '{' optnl switch_opts_l '}' { @@ -224,21 +222,6 @@ switch_opts_l : switch_opts_l switch_opts nl switch_opts: disable { vcp_disable = $1; } - | ADD string{ - chartype[IF_NAMESIZE]; - - if ((vif = calloc(1, sizeof(*vif))) == NULL) - fatal("could not allocate interface"); - - if (priv_getiftype($2, type, NULL) == -1) { - yyerror("invalid interface: %s", $2); - free($2); - YYERROR; - } - vif->vif_name = $2; - - TAILQ_INSERT_TAIL(>sw_ifs, vif, vif_entry); - } | GROUP string { if (priv_validgroup($2) == -1) { yyerror("invalid group name: %s", $2); diff --git usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c index d585bf75a99..c099bdf4ba5 100644 --- usr.sbin/vmd/priv.c +++ usr.sbin/vmd/priv.c @@ -255,7 +255,17 @@ priv_validgroup(const char *name) } /* - * Called from the process peer + * Called from the Parent process to setup vm interface(s) + * - ensure the interface has the description set (tracking purposes) + * - if interface is to be attached to a switch, attach it + * - check if rdomain is set on interface and switch + * - if interface only or both, use interface rdomain + * - if switch only, use switch rdomain + * - check if group is set on interface and switch + * - if interface, add it + * - if switch, add it + * - ensure the interface is up/down + * - if local interface, set address */ int @@ -279,18 +289,6 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm) sizeof(vfr.vfr_name)) >= sizeof(vfr.vfr_name)) return (-1); - /* Use the configured rdomain or get it from the process */ - if (vif->vif_flags & VMIFF_RDOMAIN) - vfr.vfr_id = vif->vif_rdomain; - else - vfr.vfr_id = getrtable(); - if (vfr.vfr_id != 0) - log_debug("%s: interface %s rdomain %u", __func__, - vfr.vfr_name, vfr.vfr_id); - - proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFRDOMAIN, - , sizeof(vfr)); - /* Description can be truncated */ (void)snprintf(vfr.vfr_value, sizeof(vfr.vfr_value), "vm%u-if%u-%s", vm->vm_vmid, i, vcp->vcp_name); @@ -301,8 +299,16 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm) proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFDESCR, , sizeof(vfr)); - /* Add interface to bridge/switch */ - if ((vsw = switch_getbyname(vif->vif_switch)) != NULL) { + vsw = switch_getbyname(vif->vif_switch); + /* set default rdomain */ + vfr.vfr_id = getrtable(); + + /* Check if switch should exist */ + if (vsw == NULL &&
[PATCH 2/2 v2] VMD: regress tests update for switch configuration
Update regression tests to match new switch configuration diff --git regress/usr.sbin/vmd/config/Makefile regress/usr.sbin/vmd/config/Makefile index 2adc69ae491..3bf124aff56 100644 --- regress/usr.sbin/vmd/config/Makefile +++ regress/usr.sbin/vmd/config/Makefile @@ -5,7 +5,7 @@ VMD ?= /usr/sbin/vmd VMD_PASS=boot-keyword memory-round memory-just-enough VMD_FAIL=kernel-keyword too-few-ram vm-name-too-long too-many-ifs \ boot-name-too-long disk-path-too-long too-many-disks \ -switch-no-interface +switch-no-interface switch-no-add REGRESS_TARGETS= diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.conf regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.conf new file mode 100644 index 000..40117749346 --- /dev/null +++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.conf @@ -0,0 +1,6 @@ +# $OpenBSD$ +# Fail when a switch is attempting to use add +switch "x" { +interface bridge0 +add vether0 +} diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.ok regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.ok new file mode 100644 index 000..c04c2d13732 --- /dev/null +++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.ok @@ -0,0 +1 @@ +5: syntax error diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf index 891d9c88176..f92c09656d6 100644 --- regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf +++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf @@ -1,5 +1,5 @@ # $OpenBSD: vmd-fail-switch-no-interface.conf,v 1.1 2017/10/30 03:49:30 mlarkin Exp $ # Fail when a switch is missing interface name switch "x" { -add vether0 +up } -- 2.14.3
[PATCH 0/2 v2] VMD: switch configuration changes
This patch series makes the following changes to switch configuration: * Removes adding static interfaces (done in /etc/hostname.if) * vm->interface->rdomain take precedence over sw->rdomain Updated regression tests to match vm.conf changes. Updated examples/vm.conf to match vm.conf changes. changes since v1: * v1 either broke some use cases or made some use cases more complicated to achieve (a.k.a "qemu-esque"); thanks to reyk@ for going through them with me. -- 2.14.3
merge nd6_rs_input() and nd6_ra_input()
We are processing Router Solicitation / Advertisement messages only for the Source Link-layer Address Options. Merge nd6_rs_input() and nd6_ra_input() into one generic function. OK? diff --git netinet6/icmp6.c netinet6/icmp6.c index 421280690c9..b5e12169584 100644 --- netinet6/icmp6.c +++ netinet6/icmp6.c @@ -637,32 +637,23 @@ icmp6_input(struct mbuf **mp, int *offp, int proto, int af) break; case ND_ROUTER_SOLICIT: - if (code != 0) - goto badcode; - if (icmp6len < sizeof(struct nd_router_solicit)) - goto badlen; - if ((n = m_copym(m, 0, M_COPYALL, M_DONTWAIT)) == NULL) { - /* give up local */ - nd6_rs_input(m, off, icmp6len); - m = NULL; - goto freeit; - } - nd6_rs_input(n, off, icmp6len); - /* m stays. */ - break; - case ND_ROUTER_ADVERT: if (code != 0) goto badcode; - if (icmp6len < sizeof(struct nd_router_advert)) + if ((icmp6->icmp6_type == ND_ROUTER_SOLICIT && icmp6len < + sizeof(struct nd_router_solicit)) || + (icmp6->icmp6_type == ND_ROUTER_ADVERT && icmp6len < + sizeof(struct nd_router_advert))) goto badlen; + if ((n = m_copym(m, 0, M_COPYALL, M_DONTWAIT)) == NULL) { /* give up local */ - nd6_ra_input(m, off, icmp6len); + nd6_cache_from_rtr_input(m, off, icmp6len, + icmp6->icmp6_type); m = NULL; goto freeit; } - nd6_ra_input(n, off, icmp6len); + nd6_cache_from_rtr_input(n, off, icmp6len, icmp6->icmp6_type); /* m stays. */ break; diff --git netinet6/nd6.h netinet6/nd6.h index fa63806d040..ce273321a25 100644 --- netinet6/nd6.h +++ netinet6/nd6.h @@ -192,9 +192,8 @@ void nd6_ns_output(struct ifnet *, struct in6_addr *, caddr_t nd6_ifptomac(struct ifnet *); void nd6_dad_start(struct ifaddr *); void nd6_dad_stop(struct ifaddr *); -void nd6_ra_input(struct mbuf *, int, int); -void nd6_rs_input(struct mbuf *, int, int); +void nd6_cache_from_rtr_input(struct mbuf *, int, int, int); int in6_ifdel(struct ifnet *, struct in6_addr *); void rt6_flush(struct in6_addr *, struct ifnet *); diff --git netinet6/nd6_rtr.c netinet6/nd6_rtr.c index 51772d5814e..18397b8267b 100644 --- netinet6/nd6_rtr.c +++ netinet6/nd6_rtr.c @@ -60,111 +60,15 @@ int rt6_deleteroute(struct rtentry *, void *, unsigned int); /* - * Receive Router Solicitation Message - just for routers. - * Router solicitation/advertisement is mostly managed by userland program - * (rtadvd) so here we have no function like nd6_ra_output(). - * - * Based on RFC 2461 + * Process Source Link-layer Address Options from + * Router Solicitation / Advertisement Messages. */ void -nd6_rs_input(struct mbuf *m, int off, int icmp6len) +nd6_cache_from_rtr_input(struct mbuf *m, int off, int icmp6len, int icmp6_type) { struct ifnet *ifp; struct ip6_hdr *ip6 = mtod(m, struct ip6_hdr *); struct nd_router_solicit *nd_rs; - struct in6_addr saddr6 = ip6->ip6_src; -#if 0 - struct in6_addr daddr6 = ip6->ip6_dst; -#endif - char *lladdr = NULL; - int lladdrlen = 0; -#if 0 - struct sockaddr_dl *sdl = NULL; - struct llinfo_nd6 *ln = NULL; - struct rtentry *rt = NULL; - int is_newentry; -#endif - union nd_opts ndopts; - char src[INET6_ADDRSTRLEN], dst[INET6_ADDRSTRLEN]; - - /* If I'm not a router, ignore it. XXX - too restrictive? */ - if (!ip6_forwarding) - goto freeit; - - /* Sanity checks */ - if (ip6->ip6_hlim != 255) { - nd6log((LOG_ERR, - "nd6_rs_input: invalid hlim (%d) from %s to %s on %u\n", - ip6->ip6_hlim, - inet_ntop(AF_INET6, >ip6_src, src, sizeof(src)), - inet_ntop(AF_INET6, >ip6_dst, dst, sizeof(dst)), - m->m_pkthdr.ph_ifidx)); - goto bad; - } - - /* -* Don't update the neighbor cache, if src = ::. -* This indicates that the src has no IP address assigned yet. -*/ - if (IN6_IS_ADDR_UNSPECIFIED()) - goto freeit; - - IP6_EXTHDR_GET(nd_rs, struct nd_router_solicit *, m, off, icmp6len); - if (nd_rs == NULL) { - icmp6stat_inc(icp6s_tooshort); - return; - } - - icmp6len -= sizeof(*nd_rs); - nd6_option_init(nd_rs + 1, icmp6len, ); - if (nd6_options() < 0) { - nd6log((LOG_INFO, - "nd6_rs_input: invalid ND option, ignored\n")); -
vmd: add minimal DHCP support
Hi, the problem got reported a few times and a similar diff was floating around: vmd's "local interface" implements a simple auto-magic BOOTP server, but udhcpc doesn't support BOOTP, which is the predecessor and a valid subset of DHCP. udhcpc is used by busybox and many embedded Linux distributions, maybe even by Android. The following diff adds minimal DHCP support which works with udhcpc. OK? Reyk Index: usr.sbin/vmd/dhcp.c === RCS file: /cvs/src/usr.sbin/vmd/dhcp.c,v retrieving revision 1.3 diff -u -p -u -p -r1.3 dhcp.c --- usr.sbin/vmd/dhcp.c 24 Apr 2017 07:14:27 - 1.3 +++ usr.sbin/vmd/dhcp.c 2 Nov 2017 21:15:03 - @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -38,12 +39,13 @@ extern struct vmd *env; ssize_t dhcp_request(struct vionet_dev *dev, char *buf, size_t buflen, char **obuf) { - unsigned char *respbuf = NULL; + unsigned char *respbuf = NULL, *op, *oe, dhcptype = 0; ssize_t offset, respbuflen = 0; struct packet_ctxpc; struct dhcp_packet req, resp; - struct in_addr in, mask; + struct in_addr server_addr, mask, client_addr, requested_addr; size_t resplen, o; + uint32_t ltime; if (buflen < (ssize_t)(BOOTP_MIN_LEN + sizeof(struct ether_header))) return (-1); @@ -76,24 +78,54 @@ dhcp_request(struct vionet_dev *dev, cha if (req.ciaddr.s_addr != 0 || req.file[0] != '\0' || req.hops != 0) return (-1); + /* Get a few DHCP options (best effort as we fall back to BOOTP) */ + if (memcmp(, + DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN) == 0) { + memset(_addr, 0, sizeof(requested_addr)); + op = req.options + DHCP_OPTIONS_COOKIE_LEN; + oe = req.options + sizeof(req.options); + while (*op != DHO_END && op < oe) { + if (op[0] == DHO_PAD) { + op++; + continue; + } + if (op + 1 + op[1] >= oe) + break; + if (op[0] == DHO_DHCP_MESSAGE_TYPE && + op[1] == 1) + dhcptype = op[2]; + else if (op[0] == DHO_DHCP_REQUESTED_ADDRESS && + op[1] == sizeof(requested_addr)) + memcpy(_addr, [2], + sizeof(requested_addr)); + op += 2 + op[1]; + } + } + memset(, 0, sizeof(resp)); resp.op = BOOTREPLY; resp.htype = req.htype; resp.hlen = req.hlen; resp.xid = req.xid; - if ((in.s_addr = vm_priv_addr(>vmd_cfg.cfg_localprefix, + if ((client_addr.s_addr = + vm_priv_addr(>vmd_cfg.cfg_localprefix, dev->vm_vmid, dev->idx, 1)) == 0) return (-1); - memcpy(, , sizeof(in)); - memcpy((_dst)->sin_addr, , sizeof(in)); + memcpy(, _addr, + sizeof(client_addr)); + memcpy((_dst)->sin_addr, _addr, + sizeof(client_addr)); ss2sin(_dst)->sin_port = htons(CLIENT_PORT); - if ((in.s_addr = vm_priv_addr(>vmd_cfg.cfg_localprefix, + if ((server_addr.s_addr = + vm_priv_addr(>vmd_cfg.cfg_localprefix, dev->vm_vmid, dev->idx, 0)) == 0) return (-1); - memcpy(, , sizeof(in)); - memcpy((_src)->sin_addr, , sizeof(in)); + memcpy(, _addr, + sizeof(server_addr)); + memcpy((_src)->sin_addr, _addr, + sizeof(server_addr)); ss2sin(_src)->sin_port = htons(SERVER_PORT); /* Packet is already allocated */ @@ -124,6 +156,44 @@ dhcp_request(struct vionet_dev *dev, cha DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN); o+= DHCP_OPTIONS_COOKIE_LEN; + /* Did we receive a DHCP request or was it just BOOTP? */ + if (dhcptype) { + /* +* There is no need for a real state machine as we always +* answer with the same client IP and options for the VM. +*/ + if (dhcptype == DHCPDISCOVER) + dhcptype = DHCPOFFER; + else if (dhcptype == DHCPREQUEST && + (requested_addr.s_addr == 0 || + client_addr.s_addr == requested_addr.s_addr)) + dhcptype = DHCPACK; + else + dhcptype = DHCPNAK; + + resp.options[o++] = DHO_DHCP_MESSAGE_TYPE; + resp.options[o++] = sizeof(dhcptype); + memcpy([o], , sizeof(dhcptype)); + o += sizeof(dhcptype); + + /* Our lease
Re: [PATCH 1/2] VMD: complete switch configuration overhaul
On Thu, Nov 02, 2017 at 10:53:52AM -0700, Carlos Cardenas wrote: > Removed configuration items on switches for: > * adding interfaces > * setting group > * setting rdomain > > All of these items are now to be set in hostname.if and are > queried by vmd for propagation to the vm's vif(s). > > Update example vm.conf and vm.conf manpage to reflect changes. > > Comments? Ok? > I'm not OK with this change. I see that you want to remove the bridge configuration from vmd, creating the bridge interface and adding static interfaces like em0, but I do want to keep the ability to configure the dynamic interfaces via vm.conf. There is no reason to remove "group" and "rdomain" from the switch context and it also exists in the vm/interface context. Inheriting it from the hostname.if configuration is just a different thing - but I still want be able to configure VMs in vm.conf. switch "foo" { group "bar" # put VM tapX interfaces in this group rdomain 1 # put VM tapX interfaces in this rdomain } So I'm suggesting the following instead: - You removed the functionality to create new bridge interfaces (DONE). - You removed the "add" option to add static interfaces (DONE). - Remove the code that changes group/rdomain of bridge or static interfaces. - Keep the switch group/rdomain options to use it for VM interfaces. So the diff would also be much smaller and you don't have to add these regress tests that check if a switch is configured with group/rdomain. There is no reason for ripping just everything out. Reyk > diff --git etc/examples/vm.conf etc/examples/vm.conf > index f35235e3fba..836a9f5a7c7 100644 > --- etc/examples/vm.conf > +++ etc/examples/vm.conf > @@ -10,15 +10,14 @@ sets="/var/www/htdocs/pub/OpenBSD/snapshots/amd64/" > # > > switch "uplink" { > - # This interface will default to bridge0, but switch(4) is supported > - #interface switch0 > - > - # Add additional members > - add em0 > + # This switch will use bridge0, defined by /etc/hostname.bridge0, as > + # the underlying interface. switch(4) is also supported as > + # interface switchX > + interface bridge0 > } > > switch "local" { > - add vether0 > + interface bridge1 > down > } > > diff --git usr.sbin/vmd/parse.y usr.sbin/vmd/parse.y > index a0e96545923..0bb1b0ceefa 100644 > --- usr.sbin/vmd/parse.y > +++ usr.sbin/vmd/parse.y > @@ -88,7 +88,6 @@ int parse_disk(char *); > static struct vmop_create_params vmc; > static struct vm_create_params *vcp; > static struct vmd_switch *vsw; > -static struct vmd_if *vif; > static struct vmd_vm *vm; > static char vsw_type[IF_NAMESIZE]; > static intvcp_disable; > @@ -193,7 +192,7 @@ switch: SWITCH string { > vsw->sw_id = env->vmd_nswitches + 1; > vsw->sw_name = $2; > vsw->sw_flags = VMIFF_UP; > - TAILQ_INIT(>sw_ifs); > + TAILQ_INIT(>sw_group); > > vcp_disable = 0; > } '{' optnl switch_opts_l '}' { > @@ -224,29 +223,6 @@ switch_opts_l: switch_opts_l switch_opts nl > switch_opts : disable { > vcp_disable = $1; > } > - | ADD string{ > - chartype[IF_NAMESIZE]; > - > - if ((vif = calloc(1, sizeof(*vif))) == NULL) > - fatal("could not allocate interface"); > - > - if (priv_getiftype($2, type, NULL) == -1) { > - yyerror("invalid interface: %s", $2); > - free($2); > - YYERROR; > - } > - vif->vif_name = $2; > - > - TAILQ_INSERT_TAIL(>sw_ifs, vif, vif_entry); > - } > - | GROUP string { > - if (priv_validgroup($2) == -1) { > - yyerror("invalid group name: %s", $2); > - free($2); > - YYERROR; > - } > - vsw->sw_group = $2; > - } > | INTERFACE string { > if (priv_getiftype($2, vsw_type, NULL) == -1 || > priv_findname(vsw_type, vmd_descsw) == -1) { > @@ -266,14 +242,6 @@ switch_opts : disable { > | LOCKED LLADDR { > vsw->sw_flags |= VMIFF_LOCKED; > } > - | RDOMAIN NUMBER{ > - if ($2 < 0 || $2 > RT_TABLEID_MAX) { > - yyerror("invalid rdomain: %lld", $2); > - YYERROR; > - } > -
[PATCH 0/2] VMD: complete switch configuration overhaul
This patch series finishes up the switch configuration to be handled in hostname.if change. It removes from switch configuration in vm.conf: * adding interfaces (done in /etc/hostname.if) * setting group(s) (done in /etc/hostname.if) * setting rdomain (don in /etc/hostname.if) vmd now queries the underlying interface for the switch user-defined group(s) and rdomain for propagation to vm's vifs that attach to it. Same functionality, different way to retrieve it. Updated regression tests to match vm.conf changes. Updated examples/vm.conf to match vm.conf changes. -- 2.14.3
[PATCH 1/2] VMD: complete switch configuration overhaul
Removed configuration items on switches for: * adding interfaces * setting group * setting rdomain All of these items are now to be set in hostname.if and are queried by vmd for propagation to the vm's vif(s). Update example vm.conf and vm.conf manpage to reflect changes. Comments? Ok? diff --git etc/examples/vm.conf etc/examples/vm.conf index f35235e3fba..836a9f5a7c7 100644 --- etc/examples/vm.conf +++ etc/examples/vm.conf @@ -10,15 +10,14 @@ sets="/var/www/htdocs/pub/OpenBSD/snapshots/amd64/" # switch "uplink" { - # This interface will default to bridge0, but switch(4) is supported - #interface switch0 - - # Add additional members - add em0 + # This switch will use bridge0, defined by /etc/hostname.bridge0, as + # the underlying interface. switch(4) is also supported as + # interface switchX + interface bridge0 } switch "local" { - add vether0 + interface bridge1 down } diff --git usr.sbin/vmd/parse.y usr.sbin/vmd/parse.y index a0e96545923..0bb1b0ceefa 100644 --- usr.sbin/vmd/parse.y +++ usr.sbin/vmd/parse.y @@ -88,7 +88,6 @@ intparse_disk(char *); static struct vmop_create_params vmc; static struct vm_create_params *vcp; static struct vmd_switch *vsw; -static struct vmd_if *vif; static struct vmd_vm *vm; static char vsw_type[IF_NAMESIZE]; static int vcp_disable; @@ -193,7 +192,7 @@ switch : SWITCH string { vsw->sw_id = env->vmd_nswitches + 1; vsw->sw_name = $2; vsw->sw_flags = VMIFF_UP; - TAILQ_INIT(>sw_ifs); + TAILQ_INIT(>sw_group); vcp_disable = 0; } '{' optnl switch_opts_l '}' { @@ -224,29 +223,6 @@ switch_opts_l : switch_opts_l switch_opts nl switch_opts: disable { vcp_disable = $1; } - | ADD string{ - chartype[IF_NAMESIZE]; - - if ((vif = calloc(1, sizeof(*vif))) == NULL) - fatal("could not allocate interface"); - - if (priv_getiftype($2, type, NULL) == -1) { - yyerror("invalid interface: %s", $2); - free($2); - YYERROR; - } - vif->vif_name = $2; - - TAILQ_INSERT_TAIL(>sw_ifs, vif, vif_entry); - } - | GROUP string { - if (priv_validgroup($2) == -1) { - yyerror("invalid group name: %s", $2); - free($2); - YYERROR; - } - vsw->sw_group = $2; - } | INTERFACE string { if (priv_getiftype($2, vsw_type, NULL) == -1 || priv_findname(vsw_type, vmd_descsw) == -1) { @@ -266,14 +242,6 @@ switch_opts: disable { | LOCKED LLADDR { vsw->sw_flags |= VMIFF_LOCKED; } - | RDOMAIN NUMBER{ - if ($2 < 0 || $2 > RT_TABLEID_MAX) { - yyerror("invalid rdomain: %lld", $2); - YYERROR; - } - vsw->sw_flags |= VMIFF_RDOMAIN; - vsw->sw_rdomain = $2; - } | updown{ if ($1) vsw->sw_flags |= VMIFF_UP; diff --git usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c index d585bf75a99..6253ccdd785 100644 --- usr.sbin/vmd/priv.c +++ usr.sbin/vmd/priv.c @@ -44,6 +44,9 @@ #include "proc.h" #include "vmd.h" +/* if groups to ignore */ +const char *ifgroup_ignore[] = { "all", "bridge", "switch", NULL }; + int priv_dispatch_parent(int, struct privsep_proc *, struct imsg *); voidpriv_run(struct privsep *, struct privsep_proc *, void *); @@ -264,6 +267,7 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm) struct vmd *env = ps->ps_env; struct vm_create_params *vcp = >vm_params.vmc_params; struct vmd_if *vif; + struct vmd_ifgr *vifgrp; struct vmd_switch *vsw; unsigned int i; struct vmop_ifreqvfr, vfbr; @@ -279,18 +283,6 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm) sizeof(vfr.vfr_name)) >= sizeof(vfr.vfr_name)) return (-1); - /* Use the configured rdomain or get it from
[PATCH 2/2] VMD: regress tests update for switch configuration
Update regression tests to match new switch configuration diff --git regress/usr.sbin/vmd/config/Makefile regress/usr.sbin/vmd/config/Makefile index 2adc69ae491..a337b816acc 100644 --- regress/usr.sbin/vmd/config/Makefile +++ regress/usr.sbin/vmd/config/Makefile @@ -5,7 +5,8 @@ VMD ?= /usr/sbin/vmd VMD_PASS=boot-keyword memory-round memory-just-enough VMD_FAIL=kernel-keyword too-few-ram vm-name-too-long too-many-ifs \ boot-name-too-long disk-path-too-long too-many-disks \ -switch-no-interface +switch-no-interface switch-no-add switch-no-group \ +switch-no-rdomain REGRESS_TARGETS= diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.conf regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.conf new file mode 100644 index 000..58b6ea699f7 --- /dev/null +++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.conf @@ -0,0 +1,5 @@ +# $OpenBSD$ +# Fail when a switch is attempting to add an interface +switch "x" { +add vether0 +} diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.ok regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.ok new file mode 100644 index 000..219cba51f0b --- /dev/null +++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.ok @@ -0,0 +1 @@ +4: syntax error diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-group.conf regress/usr.sbin/vmd/config/vmd-fail-switch-no-group.conf new file mode 100644 index 000..a3fbc27b086 --- /dev/null +++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-group.conf @@ -0,0 +1,6 @@ +# $OpenBSD$ +# Fail when a switch is attempting to set a group +switch "x" { +interface bridge0 +group test +} diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-group.ok regress/usr.sbin/vmd/config/vmd-fail-switch-no-group.ok new file mode 100644 index 000..c04c2d13732 --- /dev/null +++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-group.ok @@ -0,0 +1 @@ +5: syntax error diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf index 891d9c88176..f92c09656d6 100644 --- regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf +++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf @@ -1,5 +1,5 @@ # $OpenBSD: vmd-fail-switch-no-interface.conf,v 1.1 2017/10/30 03:49:30 mlarkin Exp $ # Fail when a switch is missing interface name switch "x" { -add vether0 +up } diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-rdomain.conf regress/usr.sbin/vmd/config/vmd-fail-switch-no-rdomain.conf new file mode 100644 index 000..ade16261abb --- /dev/null +++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-rdomain.conf @@ -0,0 +1,5 @@ +# $OpenBSD$ +# Fail when a switch is attempting to specify a rdomain +switch "x" { +rdomain 1 +} diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-rdomain.ok regress/usr.sbin/vmd/config/vmd-fail-switch-no-rdomain.ok new file mode 100644 index 000..219cba51f0b --- /dev/null +++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-rdomain.ok @@ -0,0 +1 @@ +4: syntax error -- 2.14.3
tedu raw_cb.c
This inlines all the trivial functions in the various places. OK? diff --git sys/conf/files sys/conf/files index 03b4a0e24cc..2771b6f4446 100644 --- sys/conf/files +++ sys/conf/files @@ -788,7 +788,6 @@ file net/switchctl.cswitch file net/switchofp.c switch file net/pipex.c pipex file net/radix.c pf | ipsec | pipex | nfsserver -file net/raw_cb.c file net/raw_usrreq.c file net/rtable.c file net/route.c diff --git sys/net/pfkeyv2.c sys/net/pfkeyv2.c index 9d0194c0570..fc836155a52 100644 --- sys/net/pfkeyv2.c +++ sys/net/pfkeyv2.c @@ -229,12 +229,17 @@ pfkeyv2_attach(struct socket *so, int proto) rp = >rcb; so->so_pcb = rp; - error = raw_attach(so, proto); + error = soreserve(so, RAWSNDQ, RAWRCVQ); + if (error) { free(kp, M_PCB, sizeof(struct keycb)); return (error); } + rp->rcb_socket = so; + rp->rcb_proto.sp_family = so->so_proto->pr_domain->dom_family; + rp->rcb_proto.sp_protocol = proto; + so->so_options |= SO_USELOOPBACK; soisconnected(so); @@ -277,7 +282,9 @@ pfkeyv2_detach(struct socket *so) mtx_leave(_mtx); } - raw_do_detach(>rcb); + so->so_pcb = 0; + sofree(so); + free((caddr_t)(>rcb), M_PCB, 0); return (0); } diff --git sys/net/raw_cb.c sys/net/raw_cb.c deleted file mode 100644 index f2396c704dd..000 --- sys/net/raw_cb.c +++ /dev/null @@ -1,106 +0,0 @@ -/* $OpenBSD: raw_cb.c,v 1.13 2017/11/02 14:01:18 florian Exp $ */ -/* $NetBSD: raw_cb.c,v 1.9 1996/02/13 22:00:39 christos Exp $ */ - -/* - * Copyright (c) 1980, 1986, 1993 - * The Regents of the University of California. All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the above copyright - *notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - *notice, this list of conditions and the following disclaimer in the - *documentation and/or other materials provided with the distribution. - * 3. Neither the name of the University nor the names of its contributors - *may be used to endorse or promote products derived from this software - *without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - * - * @(#)raw_cb.c8.1 (Berkeley) 6/10/93 - */ - -#include -#include -#include -#include -#include -#include -#include -#include - -#include -#include - -/* - * Routines to manage the raw protocol control blocks. - */ - -u_long raw_sendspace = RAWSNDQ; -u_long raw_recvspace = RAWRCVQ; - -/* - * Allocate a control block and a nominal amount - * of buffer space for the socket. - */ -int -raw_attach(struct socket *so, int proto) -{ - struct rawcb *rp = sotorawcb(so); - int error; - - /* -* It is assumed that raw_attach is called -* after space has been allocated for the -* rawcb. -*/ - if (rp == NULL) - return (ENOBUFS); - if ((error = soreserve(so, raw_sendspace, raw_recvspace)) != 0) - return (error); - rp->rcb_socket = so; - rp->rcb_proto.sp_family = so->so_proto->pr_domain->dom_family; - rp->rcb_proto.sp_protocol = proto; - return (0); -} - -int -raw_detach(struct socket *so) -{ - struct rawcb *rp = sotorawcb(so); - - soassertlocked(so); - - if (rp == NULL) - return (EINVAL); - - raw_do_detach(rp); - - return (0); -} - -/* - * Detach the raw connection block and discard - * socket resources. - */ -void -raw_do_detach(struct rawcb *rp) -{ - struct socket *so = rp->rcb_socket; - - so->so_pcb = 0; - sofree(so); - free((caddr_t)(rp), M_PCB, 0); -} diff --git sys/net/raw_cb.h sys/net/raw_cb.h index f284b56044f..dd15ca3d1f4 100644 --- sys/net/raw_cb.h +++ sys/net/raw_cb.h @@ -55,9 +55,6 @@ struct rawcb { #ifdef _KERNEL #define
tedu raw_disconnect()
There is no way SS_NOFDREF is set on a raw socket in raw_usrreq for PRU_DISCONNECT or PRU_ABORT. So raw_disconnect() and sofree() return immediately so remove the dead code. Also the following call to soisdisconnected() would be a use after free. This removes the last calls to raw_disconnect() so tedu it. OK? diff --git sys/net/raw_cb.c sys/net/raw_cb.c index 107ccc37964..f2396c704dd 100644 --- sys/net/raw_cb.c +++ sys/net/raw_cb.c @@ -104,13 +104,3 @@ raw_do_detach(struct rawcb *rp) sofree(so); free((caddr_t)(rp), M_PCB, 0); } - -/* - * Disconnect and possibly release resources. - */ -void -raw_disconnect(struct rawcb *rp) -{ - if (rp->rcb_socket->so_state & SS_NOFDREF) - raw_do_detach(rp); -} diff --git sys/net/raw_cb.h sys/net/raw_cb.h index aba508b1c96..f284b56044f 100644 --- sys/net/raw_cb.h +++ sys/net/raw_cb.h @@ -58,7 +58,6 @@ struct rawcb { int raw_attach(struct socket *, int); int raw_detach(struct socket *); voidraw_do_detach(struct rawcb *); -voidraw_disconnect(struct rawcb *); voidraw_init(void); int raw_usrreq(struct socket *, int, struct mbuf *, struct mbuf *, struct mbuf *, struct proc *); diff --git sys/net/raw_usrreq.c sys/net/raw_usrreq.c index 5f1d1c43479..462fcbbc221 100644 --- sys/net/raw_usrreq.c +++ sys/net/raw_usrreq.c @@ -78,7 +78,6 @@ raw_usrreq(struct socket *so, int req, struct mbuf *m, struct mbuf *nam, error = ENOTCONN; break; } - raw_disconnect(rp); soisdisconnected(so); break; @@ -111,8 +110,6 @@ raw_usrreq(struct socket *so, int req, struct mbuf *m, struct mbuf *nam, break; case PRU_ABORT: - raw_disconnect(rp); - sofree(so); soisdisconnected(so); break; -- I'm not entirely sure you are real.
Re: kevent(2) EVFILT_USER support
On Thu, Nov 02, 2017 at 01:50:01PM +, Stuart Henderson wrote: > On 2017/11/02 14:29, Paul Irofti wrote: > > Here is a backport from FreeBSD to support EVFILT_USER events. > > I can't comment on whether or not this should be added, but if it is, some > ports > will pick it up and need bumps: > > devel/libinotify > devel/libivykis > net/ntp > sysutils/syslog-ng > telephony/asterisk > www/nginx So I guess some ports do use it. I greped for patches removing the functionality, but I guess that's hidden behind ifdefs and autoconf layers so no need for patching. Thanks Stuart!
Re: libfuse: fuse.c null checks and others
On 28/10/17(Sat) 13:57, Helg Bredow wrote: > [...] > I've reverted fuse_loop_mt(). However, I don't feel that this is correct. If > a file system expects it to work then it should fail to make it clear that > the functionality is not implemented. sshfs calls it but because > fuse_parse_cmdline() always returns 0 for multithreaded it never gets called > (you normally need to specify -s on the command line for multithreaded fuse > file systems to run in a single thread. I've tested to see what sshfs does > when fuse_loop_mt() returns -1 and it simply exits with no message, just as > it does when it returns 0. I agree it's incorrect. But sometimes correct changes introduce regressions. So it's simpler to not mix such change with a bigger one like the NULL checks in case we need to revert them. > I also reverted the version macro change. The value for both macros is the > same (26) and they will likely stay in step if the version is incremented. > It's not using the correct macro though. I'd be happy to see you commit these changes now that the NULL checks are in :) > > We can remove fuse_is_lib_option() if nothing in ports use it. A good > > start to search for it is codesearch.debian.net. You can also ask > > porters to do a grep on unpacked port tree. > > > > I didn't know about this site, it will be useful. I was going to search the > ports source tree but this saves me the trouble and also expands the search. > sshfs calls this fuse_is_lib_option() so it will have to stay. However, it > also needs work as it doesn't behave the same as the Linux version. The nice thing is that we can do test-driven development using the linux version as reference (8 > > Could you provide a diff including only the NULL check and the removal > > of "unused" markers? > > > > Updated patch is below. That's in now.
Re: kevent(2) EVFILT_USER support
On 2017/11/02 14:29, Paul Irofti wrote: > Here is a backport from FreeBSD to support EVFILT_USER events. I can't comment on whether or not this should be added, but if it is, some ports will pick it up and need bumps: devel/libinotify devel/libivykis net/ntp sysutils/syslog-ng telephony/asterisk www/nginx
Re: kevent(2) EVFILT_USER support
> Date: Thu, 2 Nov 2017 14:29:30 +0200 > From: Paul Irofti> > Hi, > > Here is a backport from FreeBSD to support EVFILT_USER events. > > %--- > EVFILT_USEREstablishes a user event identified by ident which is >not associated with any kernel mechanism but is trig- >gered by user level code. The lower 24 bits of the >fflags may be used for user defined flags and manipu- >lated using the following: > >NOTE_FFNOP Ignore the input fflags. > >NOTE_FFAND Bitwise AND fflags. > >NOTE_FFOR Bitwise OR fflags. > >NOTE_FFCOPY Copy fflags. > >NOTE_FFCTRLMASK Control mask for fflags. > >NOTE_FFLAGSMASK User defined flag mask for >fflags. > >A user event is triggered for output with the follow- >ing: > >NOTE_TRIGGERCause the event to be triggered. > >On return, fflags contains the users defined flags in >the lower 24 bits. > %--- > > I can split the diff in two if needed: first add the touch > functionality and then the EVFILT_USER support on top. > > Thoughts? I think you need a very good motivation for introducing such functionality in OpenBSD. > Index: sys/event.h > === > RCS file: /cvs/src/sys/sys/event.h,v > retrieving revision 1.26 > diff -u -p -u -p -r1.26 event.h > --- sys/event.h 26 Jun 2017 09:32:32 - 1.26 > +++ sys/event.h 2 Nov 2017 11:58:11 - > @@ -38,8 +38,9 @@ > #define EVFILT_PROC (-5)/* attached to struct process */ > #define EVFILT_SIGNAL(-6)/* attached to struct process */ > #define EVFILT_TIMER (-7)/* timers */ > +#define EVFILT_USER (-8)/* User events */ > > -#define EVFILT_SYSCOUNT 7 > +#define EVFILT_SYSCOUNT 8 > > #define EV_SET(kevp_, a, b, c, d, e, f) do { \ > struct kevent *kevp = (kevp_); \ > @@ -80,6 +81,25 @@ struct kevent { > #define EV_ERROR 0x4000 /* error, data contains errno */ > > /* > + * data/hint flags/masks for EVFILT_USER, shared with userspace > + * > + * On input, the top two bits of fflags specifies how the lower twenty four > + * bits should be applied to the stored value of fflags. > + * > + * On output, the top two bits will always be set to NOTE_FFNOP and the > + * remaining twenty four bits will contain the stored fflags value. > + */ > +#define NOTE_FFNOP 0x /* ignore input fflags */ > +#define NOTE_FFAND 0x4000 /* AND fflags */ > +#define NOTE_FFOR0x8000 /* OR fflags */ > +#define NOTE_FFCOPY 0xc000 /* copy fflags */ > +#define NOTE_FFCTRLMASK 0xc000 /* masks for operations > */ > +#define NOTE_FFLAGSMASK 0x00ff > + > +#define NOTE_TRIGGER 0x0100 /* Cause the event to be > +triggered for output. */ > + > +/* > * hint flag for in-kernel use - must not equal any existing note > */ > #ifdef _KERNEL > @@ -142,11 +162,22 @@ SLIST_HEAD(klist, knote); > */ > #define NOTE_SIGNAL 0x0800 > > +/* > + * Hint values for the optional f_touch event filter. If f_touch is not set > + * to NULL and f_isfd is zero the f_touch filter will be called with the type > + * argument set to EVENT_REGISTER during a kevent() system call. It is also > + * called under the same conditions with the type argument set to > EVENT_PROCESS > + * when the event has been triggered. > + */ > +#define EVENT_REGISTER 1 > +#define EVENT_PROCESS2 > + > struct filterops { > int f_isfd; /* true if ident == filedescriptor */ > int (*f_attach)(struct knote *kn); > void(*f_detach)(struct knote *kn); > int (*f_event)(struct knote *kn, long hint); > + void(*f_touch)(struct knote *kn, struct kevent *kev, long type); > }; > > struct knote { > @@ -164,6 +195,7 @@ struct knote { > } kn_ptr; > const structfilterops *kn_fop; > void*kn_hook; > + int kn_hookid; > #define KN_ACTIVE0x01/* event has been triggered */ > #define KN_QUEUED0x02/* event is on queue */ > #define KN_DISABLED 0x04/* event is disabled */ > Index: kern/kern_event.c > === > RCS file: /cvs/src/sys/kern/kern_event.c,v > retrieving revision 1.81 > diff -u -p -u -p -r1.81
Re: kevent(2) EVFILT_USER support
> Reducing kqueue(2) differences is IMHO good since it makes easier to > port 3rd party software. However you forgot to mention why you want > this. Which ports make use of that? None at the moment. I am currently porting libudev from FreeBSD that makes use of it. > Do FreeBSD has regression test for this new functionality? Or can you > add one to our test suite? They do and I can add it to our regress suite. https://github.com/freebsd/freebsd/blob/master/tests/sys/kqueue/libkqueue/user.c > I also have a tricky pending kqueue(2) for MP work, so we need to > coordinate ourself. Sure.
Re: kevent(2) EVFILT_USER support
On 02/11/17(Thu) 14:29, Paul Irofti wrote: > Hi, > > Here is a backport from FreeBSD to support EVFILT_USER events. > [...] > I can split the diff in two if needed: first add the touch > functionality and then the EVFILT_USER support on top. > > Thoughts? Reducing kqueue(2) differences is IMHO good since it makes easier to port 3rd party software. However you forgot to mention why you want this. Which ports make use of that? Do FreeBSD has regression test for this new functionality? Or can you add one to our test suite? I also have a tricky pending kqueue(2) for MP work, so we need to coordinate ourself. > Index: sys/event.h > === > RCS file: /cvs/src/sys/sys/event.h,v > retrieving revision 1.26 > diff -u -p -u -p -r1.26 event.h > --- sys/event.h 26 Jun 2017 09:32:32 - 1.26 > +++ sys/event.h 2 Nov 2017 11:58:11 - > @@ -38,8 +38,9 @@ > #define EVFILT_PROC (-5)/* attached to struct process */ > #define EVFILT_SIGNAL(-6)/* attached to struct process */ > #define EVFILT_TIMER (-7)/* timers */ > +#define EVFILT_USER (-8)/* User events */ > > -#define EVFILT_SYSCOUNT 7 > +#define EVFILT_SYSCOUNT 8 > > #define EV_SET(kevp_, a, b, c, d, e, f) do { \ > struct kevent *kevp = (kevp_); \ > @@ -80,6 +81,25 @@ struct kevent { > #define EV_ERROR 0x4000 /* error, data contains errno */ > > /* > + * data/hint flags/masks for EVFILT_USER, shared with userspace > + * > + * On input, the top two bits of fflags specifies how the lower twenty four > + * bits should be applied to the stored value of fflags. > + * > + * On output, the top two bits will always be set to NOTE_FFNOP and the > + * remaining twenty four bits will contain the stored fflags value. > + */ > +#define NOTE_FFNOP 0x /* ignore input fflags */ > +#define NOTE_FFAND 0x4000 /* AND fflags */ > +#define NOTE_FFOR0x8000 /* OR fflags */ > +#define NOTE_FFCOPY 0xc000 /* copy fflags */ > +#define NOTE_FFCTRLMASK 0xc000 /* masks for operations > */ > +#define NOTE_FFLAGSMASK 0x00ff > + > +#define NOTE_TRIGGER 0x0100 /* Cause the event to be > +triggered for output. */ > + > +/* > * hint flag for in-kernel use - must not equal any existing note > */ > #ifdef _KERNEL > @@ -142,11 +162,22 @@ SLIST_HEAD(klist, knote); > */ > #define NOTE_SIGNAL 0x0800 > > +/* > + * Hint values for the optional f_touch event filter. If f_touch is not set > + * to NULL and f_isfd is zero the f_touch filter will be called with the type > + * argument set to EVENT_REGISTER during a kevent() system call. It is also > + * called under the same conditions with the type argument set to > EVENT_PROCESS > + * when the event has been triggered. > + */ > +#define EVENT_REGISTER 1 > +#define EVENT_PROCESS2 > + > struct filterops { > int f_isfd; /* true if ident == filedescriptor */ > int (*f_attach)(struct knote *kn); > void(*f_detach)(struct knote *kn); > int (*f_event)(struct knote *kn, long hint); > + void(*f_touch)(struct knote *kn, struct kevent *kev, long type); > }; > > struct knote { > @@ -164,6 +195,7 @@ struct knote { > } kn_ptr; > const structfilterops *kn_fop; > void*kn_hook; > + int kn_hookid; > #define KN_ACTIVE0x01/* event has been triggered */ > #define KN_QUEUED0x02/* event is on queue */ > #define KN_DISABLED 0x04/* event is disabled */ > Index: kern/kern_event.c > === > RCS file: /cvs/src/sys/kern/kern_event.c,v > retrieving revision 1.81 > diff -u -p -u -p -r1.81 kern_event.c > --- kern/kern_event.c 11 Oct 2017 08:06:56 - 1.81 > +++ kern/kern_event.c 2 Nov 2017 11:58:11 - > @@ -2,6 +2,7 @@ > > /*- > * Copyright (c) 1999,2000,2001 Jonathan Lemon> + * Copyright (c) 2009 Apple, Inc. > * All rights reserved. > * > * Redistribution and use in source and binary forms, with or without > @@ -98,6 +99,11 @@ intfilt_timerattach(struct knote *kn); > void filt_timerdetach(struct knote *kn); > int filt_timer(struct knote *kn, long hint); > void filt_seltruedetach(struct knote *kn); > +int filt_userattach(struct knote *kn); > +void filt_userdetach(struct knote *kn); > +int filt_user(struct knote *kn, long hint); > +void filt_usertouch(struct knote *kn, struct kevent *kev, long type); > + > > struct filterops kqread_filtops = > { 1, NULL, filt_kqdetach, filt_kqueue }; > @@ -107,6 +113,9 @@ struct filterops file_filtops = > { 1,
Re: move PRU_DETACH out of pr_usrreq
On 02/11/17(Thu) 09:06, Florian Obser wrote: > this moves PRU_DETACH out of pr_usrreq into per proto pr_detach > functions, like what claudio did to pr_attach. > > Intentionally mostly mechanical. There might be some cleanup here and > there in the functions themselves. I like it, comments inline. > diff --git kern/uipc_socket.c kern/uipc_socket.c > index 615da029414..df08096a9ee 100644 > --- kern/uipc_socket.c > +++ kern/uipc_socket.c > @@ -266,8 +266,13 @@ soclose(struct socket *so) > } > drop: > if (so->so_pcb) { > - int error2 = (*so->so_proto->pr_usrreq)(so, PRU_DETACH, NULL, > - NULL, NULL, curproc); > + int error2; > + > + if (so->so_proto->pr_detach == NULL) > + panic("soclose pr_detach == NULL: so %p, so_type %d", > + so, so->so_type); Since we're now enforcing the existence of pr_detach, a KASSERT() is enough. > + > + error2 = (*so->so_proto->pr_detach)(so); > if (error == 0) > error = error2; > } > diff --git kern/uipc_usrreq.c kern/uipc_usrreq.c > index 07e032ca0a3..35871b7cf54 100644 > --- kern/uipc_usrreq.c > +++ kern/uipc_usrreq.c > @@ -126,10 +126,6 @@ uipc_usrreq(struct socket *so, int req, struct mbuf *m, > struct mbuf *nam, > > switch (req) { > > - case PRU_DETACH: > - unp_detach(unp); > - break; > - > case PRU_BIND: > error = unp_bind(unp, nam, p); > break; > @@ -378,6 +374,21 @@ uipc_attach(struct socket *so, int proto) > return (0); > } > > +int > +uipc_detach(struct socket *so) > +{ > + struct unpcb *unp = sotounpcb(so); > + > + if (unp == NULL) > + return (EINVAL); > + > + NET_ASSERT_UNLOCKED(); > + > + unp_detach(unp); > + > + return (0); > +} > + > void > unp_detach(struct unpcb *unp) > { > diff --git net/pfkeyv2.c net/pfkeyv2.c > index ac593e4d5f1..f52ec9d3692 100644 > --- net/pfkeyv2.c > +++ net/pfkeyv2.c > @@ -159,7 +159,7 @@ static int npromisc = 0; > void pfkey_init(void); > > int pfkeyv2_attach(struct socket *, int); > -int pfkeyv2_detach(struct socket *, struct proc *); > +int pfkeyv2_detach(struct socket *); > int pfkeyv2_usrreq(struct socket *, int, struct mbuf *, struct mbuf *, > struct mbuf *, struct proc *); > int pfkeyv2_output(struct mbuf *, struct socket *, struct sockaddr *, > @@ -192,6 +192,7 @@ static struct protosw pfkeysw[] = { >.pr_output= pfkeyv2_output, >.pr_usrreq= pfkeyv2_usrreq, >.pr_attach= pfkeyv2_attach, > + .pr_detach= pfkeyv2_detach, >.pr_sysctl= pfkeyv2_sysctl, > } > }; > @@ -255,7 +256,7 @@ pfkeyv2_attach(struct socket *so, int proto) > * Close a PF_KEYv2 socket. > */ > int > -pfkeyv2_detach(struct socket *so, struct proc *p) > +pfkeyv2_detach(struct socket *so) > { > struct keycb *kp; > > @@ -276,7 +277,7 @@ pfkeyv2_detach(struct socket *so, struct proc *p) > mtx_leave(_mtx); > } > > - raw_detach(>rcb); > + raw_do_detach(>rcb); Claudio suggested merging the content of the raw functions in pfkey and rtsock. It doesn't make much sense to keep a function for calling sofree(). So I'd get rid of raw_detach() and inline it here. A next step could be to get rid of net/raw_cb.c > return (0); > } > > @@ -284,12 +285,7 @@ int > pfkeyv2_usrreq(struct socket *so, int req, struct mbuf *mbuf, > struct mbuf *nam, struct mbuf *control, struct proc *p) > { > - switch (req) { > - case PRU_DETACH: > - return (pfkeyv2_detach(so, p)); > - default: > - return (raw_usrreq(so, req, mbuf, nam, control, p)); > - } > + return (raw_usrreq(so, req, mbuf, nam, control, p)); > } > > int > diff --git net/raw_cb.c net/raw_cb.c > index e77da50b039..55c3fd21d2f 100644 > --- net/raw_cb.c > +++ net/raw_cb.c > @@ -76,12 +76,27 @@ raw_attach(struct socket *so, int proto) > return (0); > } > > +int > +raw_detach(struct socket *so) > +{ > + struct rawcb *rp = sotorawcb(so); > + > + soassertlocked(so); > + > + if (rp == NULL) > + return (EINVAL); > + > + raw_do_detach(rp); > + > + return (0); > +} > + > /* > * Detach the raw connection block and discard > * socket resources. > */ > void > -raw_detach(struct rawcb *rp) > +raw_do_detach(struct rawcb *rp) > { > struct socket *so = rp->rcb_socket; > > @@ -97,5 +112,5 @@ void > raw_disconnect(struct rawcb *rp) > { > if (rp->rcb_socket->so_state & SS_NOFDREF) > - raw_detach(rp); > + raw_do_detach(rp); > } > diff --git net/raw_cb.h net/raw_cb.h > index 00c3d30f98a..a3e964d807f 100644 > --- net/raw_cb.h > +++ net/raw_cb.h > @@ -56,7 +56,8 @@ struct rawcb { > > #define sotorawcb(so) ((struct rawcb *)(so)->so_pcb) > int raw_attach(struct socket *, int); > -void raw_detach(struct rawcb *); > +int
kevent(2) EVFILT_USER support
Hi, Here is a backport from FreeBSD to support EVFILT_USER events. %--- EVFILT_USER Establishes a user event identified by ident which is not associated with any kernel mechanism but is trig- gered by user level code. The lower 24 bits of the fflags may be used for user defined flags and manipu- lated using the following: NOTE_FFNOP Ignore the input fflags. NOTE_FFAND Bitwise AND fflags. NOTE_FFOR Bitwise OR fflags. NOTE_FFCOPY Copy fflags. NOTE_FFCTRLMASK Control mask for fflags. NOTE_FFLAGSMASK User defined flag mask for fflags. A user event is triggered for output with the follow- ing: NOTE_TRIGGERCause the event to be triggered. On return, fflags contains the users defined flags in the lower 24 bits. %--- I can split the diff in two if needed: first add the touch functionality and then the EVFILT_USER support on top. Thoughts? Index: sys/event.h === RCS file: /cvs/src/sys/sys/event.h,v retrieving revision 1.26 diff -u -p -u -p -r1.26 event.h --- sys/event.h 26 Jun 2017 09:32:32 - 1.26 +++ sys/event.h 2 Nov 2017 11:58:11 - @@ -38,8 +38,9 @@ #define EVFILT_PROC(-5)/* attached to struct process */ #define EVFILT_SIGNAL (-6)/* attached to struct process */ #define EVFILT_TIMER (-7)/* timers */ +#define EVFILT_USER(-8)/* User events */ -#define EVFILT_SYSCOUNT7 +#define EVFILT_SYSCOUNT8 #define EV_SET(kevp_, a, b, c, d, e, f) do { \ struct kevent *kevp = (kevp_); \ @@ -80,6 +81,25 @@ struct kevent { #define EV_ERROR 0x4000 /* error, data contains errno */ /* + * data/hint flags/masks for EVFILT_USER, shared with userspace + * + * On input, the top two bits of fflags specifies how the lower twenty four + * bits should be applied to the stored value of fflags. + * + * On output, the top two bits will always be set to NOTE_FFNOP and the + * remaining twenty four bits will contain the stored fflags value. + */ +#define NOTE_FFNOP 0x /* ignore input fflags */ +#define NOTE_FFAND 0x4000 /* AND fflags */ +#define NOTE_FFOR 0x8000 /* OR fflags */ +#define NOTE_FFCOPY0xc000 /* copy fflags */ +#define NOTE_FFCTRLMASK0xc000 /* masks for operations */ +#define NOTE_FFLAGSMASK0x00ff + +#define NOTE_TRIGGER 0x0100 /* Cause the event to be + triggered for output. */ + +/* * hint flag for in-kernel use - must not equal any existing note */ #ifdef _KERNEL @@ -142,11 +162,22 @@ SLIST_HEAD(klist, knote); */ #define NOTE_SIGNAL0x0800 +/* + * Hint values for the optional f_touch event filter. If f_touch is not set + * to NULL and f_isfd is zero the f_touch filter will be called with the type + * argument set to EVENT_REGISTER during a kevent() system call. It is also + * called under the same conditions with the type argument set to EVENT_PROCESS + * when the event has been triggered. + */ +#define EVENT_REGISTER 1 +#define EVENT_PROCESS 2 + struct filterops { int f_isfd; /* true if ident == filedescriptor */ int (*f_attach)(struct knote *kn); void(*f_detach)(struct knote *kn); int (*f_event)(struct knote *kn, long hint); + void(*f_touch)(struct knote *kn, struct kevent *kev, long type); }; struct knote { @@ -164,6 +195,7 @@ struct knote { } kn_ptr; const structfilterops *kn_fop; void*kn_hook; + int kn_hookid; #define KN_ACTIVE 0x01/* event has been triggered */ #define KN_QUEUED 0x02/* event is on queue */ #define KN_DISABLED0x04/* event is disabled */ Index: kern/kern_event.c === RCS file: /cvs/src/sys/kern/kern_event.c,v retrieving revision 1.81 diff -u -p -u -p -r1.81 kern_event.c --- kern/kern_event.c 11 Oct 2017 08:06:56 - 1.81 +++ kern/kern_event.c 2 Nov 2017 11:58:11 - @@ -2,6 +2,7 @@ /*- * Copyright (c) 1999,2000,2001 Jonathan Lemon+ * Copyright (c) 2009 Apple, Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or
Re: [PATCH] etc/ksh.kshrc - unify command substitution
Hi all, So there's one OK - anyone else? Would anyone be so kind as to commit it, please? :^) Regards, Raf On Mon, Oct 23, 2017 at 06:32:03PM BST, Alexander Hall wrote: > I'm OK with this. > > /Alexander > > > On October 23, 2017 3:29:57 PM GMT+02:00, Raf Czlonka> wrote: > >What say you? > > > >On Tue, Aug 29, 2017 at 08:44:43PM BST, Raf Czlonka wrote: > >> Ping. > >> > >> Anyone? > >> > >> On Sun, Jul 16, 2017 at 01:43:32PM BST, Raf Czlonka wrote: > >> > Hi all, > >> > > >> > Further simplification - 'ps | grep' can be replaced by pgrep(1) > >> > and if-then-fi by &&. > >> > > >> > > While there: > >> > > > >> > > - remove ':' (null utility) from the very first line of the file > >- > >> > > I *do* understand what it does but it doesn't seem like it's > >needed > >> > > at all, unless I'm missing something (as is the case with some > >idioms) > >> > > [...] > >> > > > >> > > >> > As it transpired, this does indeed seem to be an old idiom denoting > >> > a Bourne shell script. > >> > > >> > To quote rpe@: "I guess it's fine to remove the : line in 2017." > >> > > >> > I agree. > >> > > >> > Thanks again to Robert for all the feedback and suggestions. > >> > > >> > Regards, > >> > > >> > Raf > >> > > >> > Index: etc/ksh.kshrc > >> > === > >> > RCS file: /cvs/src/etc/ksh.kshrc,v > >> > retrieving revision 1.28 > >> > diff -u -p -r1.28 ksh.kshrc > >> > --- etc/ksh.kshrc15 Jul 2017 07:11:42 - 1.28 > >> > +++ etc/ksh.kshrc16 Jul 2017 11:49:55 - > >> > @@ -1,4 +1,3 @@ > >> > -: > >> > # $OpenBSD: ksh.kshrc,v 1.28 2017/07/15 07:11:42 tb Exp $ > >> > # > >> > # NAME: > >> > @@ -74,9 +73,7 @@ case "$-" in > >> > xterm*) > >> > ILS='\033]1;'; ILE='\007' > >> > WLS='\033]2;'; WLE='\007' > >> > -if ps -p $PPID -o command | grep -q telnet; then > >> > -export TERM=xterms > >> > -fi > >> > +pgrep -qxs $PPID telnet && export TERM=xterms > >> > ;; > >> > *) ;; > >> > esac
move PRU_DETACH out of pr_usrreq
this moves PRU_DETACH out of pr_usrreq into per proto pr_detach functions, like what claudio did to pr_attach. Intentionally mostly mechanical. There might be some cleanup here and there in the functions themselves. OK? diff --git kern/uipc_proto.c kern/uipc_proto.c index 1e86120f374..8797f0d6322 100644 --- kern/uipc_proto.c +++ kern/uipc_proto.c @@ -56,6 +56,7 @@ struct protosw unixsw[] = { .pr_flags= PR_CONNREQUIRED|PR_WANTRCVD|PR_RIGHTS, .pr_usrreq = uipc_usrreq, .pr_attach = uipc_attach, + .pr_detach = uipc_detach, }, { .pr_type = SOCK_SEQPACKET, @@ -64,6 +65,7 @@ struct protosw unixsw[] = { .pr_flags= PR_ATOMIC|PR_CONNREQUIRED|PR_WANTRCVD|PR_RIGHTS, .pr_usrreq = uipc_usrreq, .pr_attach = uipc_attach, + .pr_detach = uipc_detach, }, { .pr_type = SOCK_DGRAM, @@ -72,6 +74,7 @@ struct protosw unixsw[] = { .pr_flags= PR_ATOMIC|PR_ADDR|PR_RIGHTS, .pr_usrreq = uipc_usrreq, .pr_attach = uipc_attach, + .pr_detach = uipc_detach, } }; diff --git kern/uipc_socket.c kern/uipc_socket.c index 615da029414..df08096a9ee 100644 --- kern/uipc_socket.c +++ kern/uipc_socket.c @@ -266,8 +266,13 @@ soclose(struct socket *so) } drop: if (so->so_pcb) { - int error2 = (*so->so_proto->pr_usrreq)(so, PRU_DETACH, NULL, - NULL, NULL, curproc); + int error2; + + if (so->so_proto->pr_detach == NULL) + panic("soclose pr_detach == NULL: so %p, so_type %d", + so, so->so_type); + + error2 = (*so->so_proto->pr_detach)(so); if (error == 0) error = error2; } diff --git kern/uipc_usrreq.c kern/uipc_usrreq.c index 07e032ca0a3..35871b7cf54 100644 --- kern/uipc_usrreq.c +++ kern/uipc_usrreq.c @@ -126,10 +126,6 @@ uipc_usrreq(struct socket *so, int req, struct mbuf *m, struct mbuf *nam, switch (req) { - case PRU_DETACH: - unp_detach(unp); - break; - case PRU_BIND: error = unp_bind(unp, nam, p); break; @@ -378,6 +374,21 @@ uipc_attach(struct socket *so, int proto) return (0); } +int +uipc_detach(struct socket *so) +{ + struct unpcb *unp = sotounpcb(so); + + if (unp == NULL) + return (EINVAL); + + NET_ASSERT_UNLOCKED(); + + unp_detach(unp); + + return (0); +} + void unp_detach(struct unpcb *unp) { diff --git net/pfkeyv2.c net/pfkeyv2.c index ac593e4d5f1..f52ec9d3692 100644 --- net/pfkeyv2.c +++ net/pfkeyv2.c @@ -159,7 +159,7 @@ static int npromisc = 0; void pfkey_init(void); int pfkeyv2_attach(struct socket *, int); -int pfkeyv2_detach(struct socket *, struct proc *); +int pfkeyv2_detach(struct socket *); int pfkeyv2_usrreq(struct socket *, int, struct mbuf *, struct mbuf *, struct mbuf *, struct proc *); int pfkeyv2_output(struct mbuf *, struct socket *, struct sockaddr *, @@ -192,6 +192,7 @@ static struct protosw pfkeysw[] = { .pr_output= pfkeyv2_output, .pr_usrreq= pfkeyv2_usrreq, .pr_attach= pfkeyv2_attach, + .pr_detach= pfkeyv2_detach, .pr_sysctl= pfkeyv2_sysctl, } }; @@ -255,7 +256,7 @@ pfkeyv2_attach(struct socket *so, int proto) * Close a PF_KEYv2 socket. */ int -pfkeyv2_detach(struct socket *so, struct proc *p) +pfkeyv2_detach(struct socket *so) { struct keycb *kp; @@ -276,7 +277,7 @@ pfkeyv2_detach(struct socket *so, struct proc *p) mtx_leave(_mtx); } - raw_detach(>rcb); + raw_do_detach(>rcb); return (0); } @@ -284,12 +285,7 @@ int pfkeyv2_usrreq(struct socket *so, int req, struct mbuf *mbuf, struct mbuf *nam, struct mbuf *control, struct proc *p) { - switch (req) { - case PRU_DETACH: - return (pfkeyv2_detach(so, p)); - default: - return (raw_usrreq(so, req, mbuf, nam, control, p)); - } + return (raw_usrreq(so, req, mbuf, nam, control, p)); } int diff --git net/raw_cb.c net/raw_cb.c index e77da50b039..55c3fd21d2f 100644 --- net/raw_cb.c +++ net/raw_cb.c @@ -76,12 +76,27 @@ raw_attach(struct socket *so, int proto) return (0); } +int +raw_detach(struct socket *so) +{ + struct rawcb *rp = sotorawcb(so); + + soassertlocked(so); + + if (rp == NULL) + return (EINVAL); + + raw_do_detach(rp); + + return (0); +} + /* * Detach the raw connection block and discard * socket resources. */ void -raw_detach(struct rawcb *rp) +raw_do_detach(struct rawcb *rp) { struct socket *so = rp->rcb_socket; @@ -97,5 +112,5 @@ void raw_disconnect(struct rawcb *rp) { if (rp->rcb_socket->so_state & SS_NOFDREF) - raw_detach(rp); + raw_do_detach(rp); } diff --git net/raw_cb.h net/raw_cb.h index 00c3d30f98a..a3e964d807f 100644 --- net/raw_cb.h +++ net/raw_cb.h @@ -56,7 +56,8