* [PATCH 03/10] BUGFIX: check for existence of ifup
From: Jens Osterkamp @ 2010-11-22 15:50 UTC (permalink / raw)
To: e1000-eedc, virtualization, evb; +Cc: chrisw, Jens Osterkamp
In-Reply-To: <1290441057-16729-1-git-send-email-jens@linux.vnet.ibm.com>
added an additional check for existence of the ifup operation.
If it does not exist, just skip it.
Signed-off-by: Jens Osterkamp <jens@linux.vnet.ibm.com>
---
config.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/config.c b/config.c
index 5cfac83..26afe3b 100644
--- a/config.c
+++ b/config.c
@@ -370,8 +370,10 @@ void init_ports(void)
} else if (check_link_status(p->if_name)) {
add_adapter(p->if_name);
- LIST_FOREACH(np, &lldp_head, lldp)
- np->ops->lldp_mod_ifup(p->if_name);
+ LIST_FOREACH(np, &lldp_head, lldp) {
+ if (np->ops->lldp_mod_ifup)
+ np->ops->lldp_mod_ifup(p->if_name);
+ }
set_lldp_port_enable_state(p->if_name, 1);
}
p++;
--
1.7.2.3
^ permalink raw reply related
* [PATCH 02/10] implementation of IEEE 802.1Qbg in lldpad, part 1
From: Jens Osterkamp @ 2010-11-22 15:50 UTC (permalink / raw)
To: e1000-eedc, virtualization, evb; +Cc: chrisw, Jens Osterkamp
In-Reply-To: <1290441057-16729-1-git-send-email-jens@linux.vnet.ibm.com>
This patch contains the first part of an initial implementation of the
IEEE 802.1Qbg standard: it implements code for the exchange of EVB
capabilities between a host with virtual machines and an adjacent switch.
For this it adds a new EVB TLV to LLDP.
Exchange of EVB TLV may be enabled or disabled on a per port basis.
Information about the information negotiated by the protocol can be
queried on the commandline with lldptool.
This patch adds support for querying and setting parameters used in
the exchange of EVB TLV messages.
The parameters that can be set are:
- forwarding mode
- host protocol capabilities (RTE, ECP, VDP)
- no. of supported VSIs
- retransmission timer exponent (RTE)
The parameters are implemented as a local policy: all frames received by
an adjacent switch are validated against this policy and taken over where
appropriate. Negotiated parameters are stored in lldpads config, picked up
again and used at the next start.
The patch applies to lldpad 0.9.38 and still contains code to log protocol
activity more verbosely than it would be necessary in the final version.
Signed-off-by: Jens Osterkamp <jens@linux.vnet.ibm.com>
---
Makefile.am | 10 +-
include/lldp.h | 19 ++
include/lldp_evb.h | 80 ++++++
include/lldp_evb_clif.h | 51 ++++
include/lldp_evb_cmds.h | 31 +++
include/lldp_tlv.h | 1 +
lldp_evb.c | 672 +++++++++++++++++++++++++++++++++++++++++++++++
lldp_evb_clif.c | 226 ++++++++++++++++
lldp_evb_cmds.c | 519 ++++++++++++++++++++++++++++++++++++
lldpad.c | 2 +
lldptool.c | 2 +
11 files changed, 1609 insertions(+), 4 deletions(-)
create mode 100644 include/lldp_evb.h
create mode 100644 include/lldp_evb_clif.h
create mode 100644 include/lldp_evb_cmds.h
create mode 100644 lldp_evb.c
create mode 100644 lldp_evb_clif.c
create mode 100644 lldp_evb_cmds.c
diff --git a/Makefile.am b/Makefile.am
index 743e16f..d59a6fa 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -37,7 +37,7 @@ lldpad_include_HEADERS = include/dcb_types.h include/dcbtool.h \
include/dcb_osdep.h include/clif.h include/lldp_dcbx_cmds.h include/common.h \
include/lldpad.h include/os.h include/includes.h include/lldp_mand_cmds.h \
include/clif_msgs.h include/lldp_basman_cmds.h include/lldp_8023_cmds.h \
-include/lldp_med_cmds.h include/lldp_dcbx_cfg.h
+include/lldp_med_cmds.h include/lldp_dcbx_cfg.h include/lldp_evb_cmds.h
noinst_HEADERS = include/config.h include/ctrl_iface.h \
include/dcb_driver_if_types.h include/dcb_driver_interface.h \
@@ -47,7 +47,7 @@ include/event_iface.h include/messages.h include/parse_cli.h include/version.h \
include/lldptool_cli.h include/list.h \
include/lldp_mand_clif.h include/lldp_basman_clif.h include/lldp_med_clif.h \
include/lldp_8023_clif.h include/lldp_dcbx_clif.h include/lldptool.h \
-include/lldp_rtnl.h
+include/lldp_rtnl.h include/lldp_evb_clif.h
lldpad_SOURCES = lldpad.c config.c drv_cfg.c ctrl_iface.c event_iface.c eloop.c \
common.c os_unix.c lldp_dcbx_cmds.c log.c lldpad_shm.c \
@@ -62,10 +62,12 @@ lldp_dcbx_cfg.c include/lldp_dcbx_cfg.h \
lldp_util.c include/lldp_util.h \
lldp_mand.c include/lldp_mand.h \
lldp_mand_cmds.c lldp_basman_cmds.c lldp_8023_cmds.c lldp_med_cmds.c \
+lldp_evb_cmds.c \
lldp_tlv.c include/lldp_tlv.h \
lldp_basman.c include/lldp_basman.h \
lldp_med.c include/lldp_med.h \
-lldp_8023.c include/lldp_8023.h
+lldp_8023.c include/lldp_8023.h \
+lldp_evb.c include/lldp_evb.h
@@ -74,7 +76,7 @@ $(lldpad_include_HEADERS) $(noinst_HEADERS)
lldptool_SOURCES = lldptool.c clif.c lldptool_cmds.c common.c os_unix.c \
lldp_mand_clif.c lldp_basman_clif.c lldp_med_clif.c lldp_8023_clif.c \
-lldp_dcbx_clif.c $(lldpad_include_HEADERS) $(noinst_HEADERS)
+lldp_dcbx_clif.c lldp_evb_clif.c $(lldpad_include_HEADERS) $(noinst_HEADERS)
nltest_SOURCES = nltest.c nltest.h
diff --git a/include/lldp.h b/include/lldp.h
index 66532bd..0371844 100644
--- a/include/lldp.h
+++ b/include/lldp.h
@@ -45,6 +45,8 @@
/* Telecommunications Industry Association TR-41 Committee */
#define OUI_TIA_TR41 0x0012bb
+#define OUI_IEEE_8021Qbg 0x001b3f
+
/* IEEE 802.3AB Clause 9: TLV Types */
#define CHASSIS_ID_TLV 1
#define PORT_ID_TLV 2
@@ -186,5 +188,22 @@ enum {
#define LLDP_8023_LINKAGG_CAPABLE (1 << 0)
#define LLDP_8023_LINKAGG_ENABLED (1 << 1)
+/* IEEE 802.1Qbg subtype */
+#define LLDP_EVB_SUBTYPE 0
+
+/* forwarding mode */
+#define LLDP_EVB_CAPABILITY_FORWARD_STANDARD (1 << 7)
+#define LLDP_EVB_CAPABILITY_FORWARD_REFLECTIVE_RELAY (1 << 6)
+
+/* EVB supported protocols */
+#define LLDP_EVB_CAPABILITY_PROTOCOL_RTE (1 << 2)
+#define LLDP_EVB_CAPABILITY_PROTOCOL_ECP (1 << 1)
+#define LLDP_EVB_CAPABILITY_PROTOCOL_VDP (1 << 0)
+
+/* EVB specific values */
+#define LLDP_EVB_DEFAULT_MAX_VSI 65535
+#define LLDP_EVB_DEFAULT_SVSI 3295
+#define LLDP_EVB_DEFAULT_RTE 15
+
void somethingChangedLocal(char *ifname);
#endif /* _LLDP_H */
diff --git a/include/lldp_evb.h b/include/lldp_evb.h
new file mode 100644
index 0000000..667f9ad
--- /dev/null
+++ b/include/lldp_evb.h
@@ -0,0 +1,80 @@
+/*******************************************************************************
+
+ implementation of EVB TLVs for LLDP
+ (c) Copyright IBM Corp. 2010
+
+ Author(s): Jens Osterkamp <jens@linux.vnet.ibm.com>
+
+ This program is free software; you can redistribute it and/or modify it
+ under the terms and conditions of the GNU General Public License,
+ version 2, as published by the Free Software Foundation.
+
+ This program is distributed in the hope it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ more details.
+
+ You should have received a copy of the GNU General Public License along with
+ this program; if not, write to the Free Software Foundation, Inc.,
+ 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+
+ The full GNU General Public License is included in this distribution in
+ the file called "COPYING".
+
+*******************************************************************************/
+
+#ifndef _LLDP_EVB_H
+#define _LLDP_EVB_H
+
+#include "lldp_mod.h"
+
+#define LLDP_MOD_EVB OUI_IEEE_8021Qbg
+#define LLDP_OUI_SUBTYPE { 0x00, 0x1b, 0x3f, 0x00 }
+
+typedef enum {
+ EVB_OFFER_CAPABILITIES = 0,
+ EVB_CONFIGURE,
+ EVB_CONFIRMATION
+} evb_state;
+
+struct tlv_info_evb {
+ u8 oui[3];
+ u8 sub;
+ /* supported forwarding mode */
+ u8 smode;
+ /* supported capabilities */
+ u8 scap;
+ /* currently configured forwarding mode */
+ u8 cmode;
+ /* currently configured capabilities */
+ u8 ccap;
+ /* supported no. of vsi */
+ u16 svsi;
+ /* currently configured no. of vsi */
+ u16 cvsi;
+ /* retransmission exponent */
+ u8 rte;
+} __attribute__ ((__packed__));
+
+struct evb_data {
+ char ifname[IFNAMSIZ];
+ struct unpacked_tlv *evb;
+ struct tlv_info_evb *tie;
+ /* local policy */
+ struct tlv_info_evb *policy;
+ int state;
+ LIST_ENTRY(evb_data) entry;
+};
+
+struct evb_user_data {
+ LIST_HEAD(evb_head, evb_data) head;
+};
+
+struct lldp_module *evb_register(void);
+void evb_unregister(struct lldp_module *mod);
+struct packed_tlv *evb_gettlv(struct port *port);
+void evb_ifdown(char *);
+void evb_ifup(char *);
+struct evb_data *evb_data(char *ifname);
+
+#endif /* _LLDP_EVB_H */
diff --git a/include/lldp_evb_clif.h b/include/lldp_evb_clif.h
new file mode 100644
index 0000000..acaee5e
--- /dev/null
+++ b/include/lldp_evb_clif.h
@@ -0,0 +1,51 @@
+/*******************************************************************************
+
+ implementation of EVB TLVs for LLDP
+ (c) Copyright IBM Corp. 2010
+
+ Author(s): Jens Osterkamp <jens@linux.vnet.ibm.com>
+
+ This program is free software; you can redistribute it and/or modify it
+ under the terms and conditions of the GNU General Public License,
+ version 2, as published by the Free Software Foundation.
+
+ This program is distributed in the hope it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ more details.
+
+ You should have received a copy of the GNU General Public License along with
+ this program; if not, write to the Free Software Foundation, Inc.,
+ 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+
+ The full GNU General Public License is included in this distribution in
+ the file called "COPYING".
+
+*******************************************************************************/
+
+#ifndef _LLDP_EVB_CLIF_H
+#define _LLDP_EVB_CLIF_H
+
+struct lldp_module *evb_cli_register(void);
+void evb_cli_unregister(struct lldp_module *);
+int evb_print_tlv(u32, u16, char *);
+
+#define EVB_BUF_SIZE 256
+
+#define ARG_EVB_FORWARDING_MODE "fmode"
+
+#define VAL_EVB_FMODE_BRIDGE "bridge"
+#define VAL_EVB_FMODE_REFLECTIVE_RELAY "reflectiverelay"
+
+#define ARG_EVB_CAPABILITIES "capabilities"
+
+#define VAL_EVB_CAPA_RTE "rte"
+#define VAL_EVB_CAPA_ECP "ecp"
+#define VAL_EVB_CAPA_VDP "vdp"
+#define VAL_EVB_CAPA_NONE "none"
+
+#define ARG_EVB_VSIS "vsis"
+
+#define ARG_EVB_RTE "rte"
+
+#endif
diff --git a/include/lldp_evb_cmds.h b/include/lldp_evb_cmds.h
new file mode 100644
index 0000000..1367e5d
--- /dev/null
+++ b/include/lldp_evb_cmds.h
@@ -0,0 +1,31 @@
+/*******************************************************************************
+
+ implementation of EVB TLVs for LLDP
+ (c) Copyright IBM Corp. 2010
+
+ Author(s): Jens Osterkamp <jens@linux.vnet.ibm.com>
+
+ This program is free software; you can redistribute it and/or modify it
+ under the terms and conditions of the GNU General Public License,
+ version 2, as published by the Free Software Foundation.
+
+ This program is distributed in the hope it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ more details.
+
+ You should have received a copy of the GNU General Public License along with
+ this program; if not, write to the Free Software Foundation, Inc.,
+ 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+
+ The full GNU General Public License is included in this distribution in
+ the file called "COPYING".
+
+*******************************************************************************/
+
+#ifndef _LLDP_EVB_CMDS_H
+#define _LLDP_EVB_CMDS_H
+
+struct arg_handlers *evb_get_arg_handlers();
+
+#endif
diff --git a/include/lldp_tlv.h b/include/lldp_tlv.h
index a32cc71..fe3a75a 100644
--- a/include/lldp_tlv.h
+++ b/include/lldp_tlv.h
@@ -144,6 +144,7 @@ int tlv_ok(struct unpacked_tlv *tlv);
#define TLVID_8021(sub) TLVID(OUI_IEEE_8021, (sub))
#define TLVID_8023(sub) TLVID(OUI_IEEE_8023, (sub))
#define TLVID_MED(sub) TLVID(OUI_TIA_TR41, (sub))
+#define TLVID_8021Qbg(sub) TLVID(OUI_IEEE_8021Qbg, (sub))
/* the size in bytes needed for a packed tlv from unpacked tlv */
#define TLVSIZE(t) ((t) ? (2 + (t)->length) : 0)
diff --git a/lldp_evb.c b/lldp_evb.c
new file mode 100644
index 0000000..50ee17d
--- /dev/null
+++ b/lldp_evb.c
@@ -0,0 +1,672 @@
+/*******************************************************************************
+
+ implementation of EVB TLVs for LLDP
+ (c) Copyright IBM Corp. 2010
+
+ Author(s): Jens Osterkamp <jens@linux.vnet.ibm.com>
+
+ This program is free software; you can redistribute it and/or modify it
+ under the terms and conditions of the GNU General Public License,
+ version 2, as published by the Free Software Foundation.
+
+ This program is distributed in the hope it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ more details.
+
+ You should have received a copy of the GNU General Public License along with
+ this program; if not, write to the Free Software Foundation, Inc.,
+ 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+
+ The full GNU General Public License is included in this distribution in
+ the file called "COPYING".
+
+*******************************************************************************/
+
+#include <net/if.h>
+#include <sys/queue.h>
+#include <sys/socket.h>
+#include <sys/ioctl.h>
+#include <sys/utsname.h>
+#include <linux/if_bridge.h>
+#include <string.h>
+#include "lldp.h"
+#include "lldp_evb.h"
+#include "messages.h"
+#include "config.h"
+#include "common.h"
+#include "lldp_mand_clif.h"
+#include "lldp_evb_clif.h"
+#include "lldp_evb_cmds.h"
+
+extern struct lldp_head lldp_head;
+
+struct evb_data *evb_data(char *ifname)
+{
+ struct evb_user_data *ud;
+ struct evb_data *ed = NULL;
+
+ ud = find_module_user_data_by_if(ifname, &lldp_head, LLDP_MOD_EVB);
+ if (ud) {
+ LIST_FOREACH(ed, &ud->head, entry) {
+ if (!strncmp(ifname, ed->ifname, IFNAMSIZ))
+ return ed;
+ }
+ }
+ return NULL;
+}
+
+static void evb_print_tlvinfo(struct tlv_info_evb *tie)
+{
+ LLDPAD_INFO("%s(%i): supported forwarding mode: %02x\n", __FILE__, __LINE__, tie->smode);
+ LLDPAD_INFO("%s(%i): configured forwarding mode: %02x\n", __FILE__, __LINE__, tie->cmode);
+ LLDPAD_INFO("%s(%i): supported capabilities: %02x\n", __FILE__, __LINE__, tie->scap);
+ LLDPAD_INFO("%s(%i): configured capabilities: %02x\n", __FILE__, __LINE__, tie->ccap);
+ LLDPAD_INFO("%s(%i): supported no. of vsis: %04i\n", __FILE__, __LINE__, tie->svsi);
+ LLDPAD_INFO("%s(%i): configured no. of vsis: %04i\n", __FILE__, __LINE__, tie->cvsi);
+ LLDPAD_INFO("%s(%i): rte: %02i\n", __FILE__, __LINE__, tie->rte);
+}
+
+static void evb_dump_tlv(struct unpacked_tlv *tlv)
+{
+ int i;
+ char *s, *t;
+
+ s = t = malloc(256);
+
+ if (!s) {
+ LLDPAD_ERR("%s(%i): unable to allocate string !\n", __func__, __LINE__);
+ }
+
+ LLDPAD_DBG("%s:type %i, length %i, info ", __func__, tlv->type, tlv->length);
+
+ for (i=0; i < tlv->length; i++) {
+ int c;
+ c = sprintf(s, "%02x ", tlv->info[i]);
+ s += c;
+ }
+
+ LLDPAD_DBG("%s\n", t);
+
+ free(t);
+}
+
+/*
+ * evb_bld_cfg_tlv - build the EVB TLV
+ * @ed: the evb data struct
+ *
+ * Returns 0 on success
+ */
+static int evb_bld_cfg_tlv(struct evb_data *ed)
+{
+ int rc = 0;
+ int i;
+ struct unpacked_tlv *tlv = NULL;
+
+ /* free ed->evb if it exists */
+ FREE_UNPKD_TLV(ed, evb);
+
+ if (!is_tlv_txenabled(ed->ifname, TLVID_8021Qbg(LLDP_EVB_SUBTYPE))) {
+ LLDPAD_ERR("%s:%s:EVB tx is currently disabled !\n",
+ __func__, ed->ifname);
+ rc = EINVAL;
+ goto out_err;
+ }
+
+ if (ed->tie->smode != ed->policy->smode) {
+ ed->tie->smode = ed->policy->smode;
+ ed->state = EVB_OFFER_CAPABILITIES;
+ }
+
+ tlv = create_tlv();
+ if (!tlv)
+ goto out_err;
+
+ tlv->type = ORG_SPECIFIC_TLV;
+ tlv->length = sizeof(struct tlv_info_evb);
+ tlv->info = (u8 *)malloc(tlv->length);
+ if(!tlv->info) {
+ free(tlv);
+ tlv = NULL;
+ rc = ENOMEM;
+ goto out_err;
+ }
+ memcpy(tlv->info, ed->tie, tlv->length);
+
+ LLDPAD_DBG("%s(%i): TLV about to be sent out:", __func__, __LINE__);
+ evb_dump_tlv(tlv);
+
+ ed->evb = tlv;
+out_err:
+ return rc;
+}
+
+static void evb_free_tlv(struct evb_data *ed)
+{
+ if (ed)
+ FREE_UNPKD_TLV(ed, evb);
+}
+
+/* evb_init_cfg_tlv:
+ *
+ * fill up tlv_info_evb structure with reasonable info
+ */
+static int evb_init_cfg_tlv(struct evb_data *ed)
+{
+ char arg_path[EVB_BUF_SIZE];
+ char *param;
+
+ /* load policy from config */
+ ed->policy = (struct tlv_info_evb *) calloc(1, sizeof(struct tlv_info_evb));
+
+ if (!ed->policy)
+ return ENOMEM;
+
+ /* set defaults */
+ hton24(ed->policy->oui, LLDP_MOD_EVB);
+ ed->policy->smode = LLDP_EVB_CAPABILITY_FORWARD_STANDARD;
+ ed->policy->scap = LLDP_EVB_CAPABILITY_PROTOCOL_RTE | LLDP_EVB_CAPABILITY_PROTOCOL_ECP |
+ LLDP_EVB_CAPABILITY_PROTOCOL_VDP;
+ ed->policy->cmode = 0;
+ ed->policy->ccap = 0;
+ ed->policy->svsi = LLDP_EVB_DEFAULT_SVSI;
+ ed->policy->rte = LLDP_EVB_DEFAULT_RTE;
+
+ /* pull forwarding mode into policy */
+ snprintf(arg_path, sizeof(arg_path), "%s%08x.fmode",
+ TLVID_PREFIX, TLVID_8021Qbg(LLDP_EVB_SUBTYPE));
+
+ if (get_cfg(ed->ifname, arg_path, (void *) ¶m, CONFIG_TYPE_STRING)) {
+ LLDPAD_WARN("%s:%s: loading EVB policy for forwarding mode failed, using default.\n",
+ __func__, ed->ifname);
+ } else {
+ if (strcasestr(param, VAL_EVB_FMODE_BRIDGE)) {
+ ed->policy->smode = LLDP_EVB_CAPABILITY_FORWARD_STANDARD;
+ }
+
+ if (strcasestr(param, VAL_EVB_FMODE_REFLECTIVE_RELAY)) {
+ ed->policy->smode = LLDP_EVB_CAPABILITY_FORWARD_REFLECTIVE_RELAY;
+ }
+
+ LLDPAD_DBG("%s:%s: policy param fmode = %s.\n", __func__, ed->ifname, param);
+ LLDPAD_DBG("%s:%s: policy param smode = %x.\n", __func__, ed->ifname, ed->policy->smode);
+ }
+
+ /* pull capabilities into policy */
+ snprintf(arg_path, sizeof(arg_path), "%s%08x.capabilities",
+ TLVID_PREFIX, TLVID_8021Qbg(LLDP_EVB_SUBTYPE));
+
+ if (get_cfg(ed->ifname, arg_path, (void *) ¶m, CONFIG_TYPE_STRING)) {
+ LLDPAD_WARN("%s:%s: loading EVB policy for capabilities failed, using default.\n",
+ __func__, ed->ifname);
+ } else {
+ if (strcasestr(param, VAL_EVB_CAPA_RTE)) {
+ ed->policy->scap |= LLDP_EVB_CAPABILITY_PROTOCOL_RTE;
+ }
+
+ if (strcasestr(param, VAL_EVB_CAPA_ECP)) {
+ ed->policy->scap |= LLDP_EVB_CAPABILITY_PROTOCOL_ECP;
+ }
+
+ if (strcasestr(param, VAL_EVB_CAPA_VDP)) {
+ ed->policy->scap |= LLDP_EVB_CAPABILITY_PROTOCOL_VDP;
+ }
+
+ LLDPAD_DBG("%s:%s: policy param capabilities = %s.\n", __func__, ed->ifname, param);
+ LLDPAD_DBG("%s:%s: policy param scap = %x.\n", __func__, ed->ifname, ed->policy->scap);
+ }
+
+ /* pull rte into policy */
+ snprintf(arg_path, sizeof(arg_path), "%s%08x.rte",
+ TLVID_PREFIX, TLVID_8021Qbg(LLDP_EVB_SUBTYPE));
+
+ if (get_cfg(ed->ifname, arg_path, (void *) ¶m, CONFIG_TYPE_STRING)) {
+ LLDPAD_WARN("%s:%s: loading EVB policy for rte failed, using default.\n",
+ __func__, ed->ifname);
+ } else {
+ ed->policy->rte = atoi(param);
+
+ LLDPAD_DBG("%s:%s: policy param rte = %s.\n", __func__, ed->ifname, param);
+ LLDPAD_DBG("%s:%s: policy param rte = %i.\n", __func__, ed->ifname, ed->policy->rte);
+ }
+
+ /* pull vsis into policy */
+ snprintf(arg_path, sizeof(arg_path), "%s%08x.vsis",
+ TLVID_PREFIX, TLVID_8021Qbg(LLDP_EVB_SUBTYPE));
+
+ if (get_cfg(ed->ifname, arg_path, (void *) ¶m, CONFIG_TYPE_STRING)) {
+ LLDPAD_WARN("%s:%s: loading EVB policy for vsis failed, using default.\n",
+ __func__, ed->ifname);
+ } else {
+ ed->policy->svsi = atoi(param);
+
+ LLDPAD_DBG("%s:%s: policy param vsis = %s.\n", __func__, ed->ifname, param);
+ LLDPAD_DBG("%s:%s: policy param vsis = %i.\n", __func__, ed->ifname, ed->policy->svsi);
+ }
+
+ /* load last used EVB TLV ... */
+ ed->tie = (struct tlv_info_evb *) calloc(1, sizeof(struct tlv_info_evb));
+
+ if (!ed->tie) {
+ free(ed->policy);
+ return ENOMEM;
+ }
+
+ if (get_config_tlvinfo_bin(ed->ifname, TLVID_8021Qbg(LLDP_EVB_SUBTYPE),
+ (void *)ed->tie, sizeof(struct tlv_info_evb))) {
+ LLDPAD_INFO("%s:%s: loading last used EVB TLV failed, using default.\n",
+ __func__, ed->ifname);
+ hton24(ed->tie->oui, LLDP_MOD_EVB);
+ ed->tie->smode = ed->policy->smode;
+ ed->tie->cmode = 0x0;
+ ed->tie->scap = ed->policy->scap;
+ ed->tie->ccap = 0x0;
+ ed->tie->svsi = LLDP_EVB_DEFAULT_SVSI;
+ ed->tie->cvsi = 0x0;
+ ed->tie->rte = LLDP_EVB_DEFAULT_RTE;
+ } else {
+ LLDPAD_INFO("%s(%i): loaded last used EVB TLV from file.\n", __FILE__, __LINE__);
+ }
+
+ return 0;
+}
+
+static int evb_bld_tlv(struct evb_data *ed)
+{
+ int rc = 0;
+
+ if (!port_find_by_name(ed->ifname)) {
+ rc = EEXIST;
+ goto out_err;
+ }
+
+ if (!init_cfg()) {
+ rc = ENOENT;
+ goto out_err;
+ }
+
+ if (evb_bld_cfg_tlv(ed)) {
+ LLDPAD_ERR("%s:%s:evb_bld_cfg_tlv() failed\n",
+ __func__, ed->ifname);
+ rc = EINVAL;
+ goto out_err_destroy;
+ }
+
+ return rc;
+
+out_err_destroy:
+ destroy_cfg();
+out_err:
+ return rc;
+}
+
+static void evb_free_data(struct evb_user_data *ud)
+{
+ struct evb_data *ed;
+ if (ud) {
+ while (!LIST_EMPTY(&ud->head)) {
+ ed = LIST_FIRST(&ud->head);
+ LIST_REMOVE(ed, entry);
+ evb_free_tlv(ed);
+ free(ed);
+ }
+ }
+}
+
+struct packed_tlv *evb_gettlv(struct port *port)
+{
+ int size;
+ struct evb_data *ed;
+ struct packed_tlv *ptlv = NULL;
+
+ ed = evb_data(port->ifname);
+ if (!ed)
+ goto out_err;
+
+ evb_free_tlv(ed);
+
+ if (evb_bld_tlv(ed)) {
+ LLDPAD_ERR("%s:%s evb_bld_tlv failed\n",
+ __func__, port->ifname);
+ goto out_err;
+ }
+
+ size = TLVSIZE(ed->evb);
+
+ if (!size)
+ goto out_err;
+
+ ptlv = create_ptlv();
+ if (!ptlv)
+ goto out_err;
+
+ ptlv->tlv = malloc(size);
+ if (!ptlv->tlv)
+ goto out_free;
+
+ ptlv->size = 0;
+ PACK_TLV_AFTER(ed->evb, ptlv, size, out_free);
+ return ptlv;
+
+out_free:
+ /* FIXME: free function returns pointer ? */
+ ptlv = free_pkd_tlv(ptlv);
+out_err:
+ LLDPAD_ERR("%s:%s: failed\n", __func__, port->ifname);
+ return NULL;
+}
+
+/* evb_check_and_fill
+ *
+ * checks values received in TLV and takes over some values
+ */
+int evb_check_and_fill(struct evb_data *ed, struct tlv_info_evb *tie)
+{
+ /* sanity check of received data in tie */
+ if ((tie->smode & (LLDP_EVB_CAPABILITY_FORWARD_STANDARD |
+ LLDP_EVB_CAPABILITY_FORWARD_REFLECTIVE_RELAY)) == 0) {
+ LLDPAD_ERR("Neither standard nor rr set as forwarding mode !");
+ return TLV_ERR;
+ }
+
+ if ((tie->svsi < 0) || (tie->svsi > LLDP_EVB_DEFAULT_MAX_VSI)) {
+ LLDPAD_ERR("nr of supported vsis (%i) exceeds allow value range !", tie->svsi);
+ return TLV_ERR;
+ }
+
+ if ((tie->cvsi < 0) || (tie->cvsi > LLDP_EVB_DEFAULT_MAX_VSI)) {
+ LLDPAD_ERR("nr of configured vsis (%i) exceeds allow value range !", tie->cvsi);
+ return TLV_ERR;
+ }
+
+ /* check bridge capabilities against local policy*/
+ /* if bridge supports RR and we support it as well, request it
+ * by setting smode in tlv to be sent out (ed->tie->smode) */
+ if ( (tie->smode & ed->policy->smode) ==
+ LLDP_EVB_CAPABILITY_FORWARD_REFLECTIVE_RELAY ) {
+ ed->tie->smode = LLDP_EVB_CAPABILITY_FORWARD_REFLECTIVE_RELAY;
+ } else {
+ ed->tie->smode = LLDP_EVB_CAPABILITY_FORWARD_STANDARD;
+ }
+
+ /* If both sides support RTE, set it */
+ if ((tie->scap & ed->policy->scap) & LLDP_EVB_CAPABILITY_PROTOCOL_RTE)
+ ed->tie->scap |= LLDP_EVB_CAPABILITY_PROTOCOL_RTE;
+
+ /* If both sides support ECP, set it */
+ if ((tie->scap & ed->policy->scap) & LLDP_EVB_CAPABILITY_PROTOCOL_ECP)
+ ed->tie->scap |= LLDP_EVB_CAPABILITY_PROTOCOL_ECP;
+
+ /* If both sides support VDP, set it */
+ if ((tie->scap & ed->policy->scap) & LLDP_EVB_CAPABILITY_PROTOCOL_VDP)
+ ed->tie->scap |= LLDP_EVB_CAPABILITY_PROTOCOL_VDP;
+
+ /* If supported caps include VDP take over min value of both */
+ if (ed->tie->scap & LLDP_EVB_CAPABILITY_PROTOCOL_VDP)
+ ed->tie->cvsi = MIN(ed->policy->svsi,tie->svsi);
+
+ /* If both sides support RTE and value offer is > 0, set it */
+ if ((ed->tie->scap & LLDP_EVB_CAPABILITY_PROTOCOL_RTE) &&
+ (tie->rte > 0) && (ed->policy->rte > 0))
+ ed->tie->rte = MAX(ed->policy->rte,tie->rte);
+
+ if (set_config_tlvinfo_bin(ed->ifname, TLVID_8021Qbg(LLDP_EVB_SUBTYPE),
+ (void *)ed->tie, sizeof(struct tlv_info_evb))) {
+ LLDPAD_ERR("%s(%i): error saving tlv_info_evb !\n", __FILE__, __LINE__);
+ } else {
+ LLDPAD_INFO("%s(%i): saved tlv_info_evb to config !\n", __FILE__, __LINE__);
+ }
+
+ /* maybe switch has already set the mode based on the saved info sent
+ * out on ifup */
+
+ if (tie->cmode == ed->tie->smode)
+ ed->tie->cmode = tie->cmode;
+
+ return TLV_OK;
+}
+
+/* evb_compare
+ *
+ * compare our own and received tlv_info_evb
+ */
+static int evb_compare(struct evb_data *ed, struct tlv_info_evb *tie)
+{
+ LLDPAD_DBG("%s(%i): \n", __func__, __LINE__);
+
+ if (ed->tie->cmode == tie->cmode)
+ return 0;
+ else
+ return 1;
+}
+
+/* evb_statemachine:
+ *
+ * handle possible states during EVB capabilities exchange
+ *
+ * possible states: EVB_OFFER_CAPABILITIES
+ * EVB_CONFIGURE
+ * EVB_CONFIRMATION
+ */
+static void evb_statemachine(struct evb_data *ed, struct tlv_info_evb *tie)
+{
+ switch(ed->state) {
+ case EVB_OFFER_CAPABILITIES:
+ /* waiting for valid packets to pour in
+ * if valid packet was received,
+ * - check parameters with what we have offered for this if,
+ * - fill structure with data,
+ * - enable local tx
+ * - switch to EVB_CONFIGURE
+ */
+ LLDPAD_DBG("%s: state -> EVB_OFFER_CAPABILITIES\n", __func__);
+ if (evb_check_and_fill(ed, tie) != TLV_OK) {
+ LLDPAD_ERR("Invalid contents of EVB Cfg TLV !\n");
+ return;
+ }
+ somethingChangedLocal(ed->ifname); /* trigger tx with new values */
+ ed->state = EVB_CONFIGURE;
+ break;
+ case EVB_CONFIGURE:
+ /* we received a valid packet, if contents is same with our local settings
+ * we can switch state to EVB_CONFIRMATION.*/
+ LLDPAD_DBG("%s: state -> EVB_CONFIGURE\n", __func__);
+ if (evb_compare(ed, tie)) {
+ ed->state= EVB_OFFER_CAPABILITIES;
+ } else {
+ LLDPAD_ERR("tlv_info_evb now equal !\n");
+ ed->state = EVB_CONFIRMATION;
+ }
+ somethingChangedLocal(ed->ifname);
+ break;
+ case EVB_CONFIRMATION:
+ /* we are already in confirmation and received a new packet with
+ * different parameters ? Check parameters. switch state back to
+ * EVB_CONFIGURE ? */
+ LLDPAD_DBG("%s: state -> EVB_CONFIRMATION\n", __func__);
+ break;
+ default:
+ LLDPAD_ERR("EVB statemachine reached invalid state !\n");
+ break;
+ }
+}
+
+/*
+ * evb_rchange: process RX TLV LLDPDU
+ *
+ * TLV not consumed on error
+ */
+static int evb_rchange(struct port *port, struct unpacked_tlv *tlv)
+{
+ int i;
+ struct evb_data *ed;
+ struct tlv_info_evb *tie = (struct tlv_info_evb *) tlv->info;
+ u8 oui_subtype[OUI_SUB_SIZE] = LLDP_OUI_SUBTYPE;
+
+ if (!init_cfg()) {
+ return SUBTYPE_INVALID;
+ }
+
+ ed = evb_data(port->ifname);
+
+ if (!ed)
+ return SUBTYPE_INVALID;
+
+ if (tlv->type == TYPE_127) {
+ /* check for length */
+ if (tlv->length < (OUI_SUB_SIZE)) {
+ return TLV_ERR;
+ }
+
+ /* check for oui */
+ if (memcmp(tlv->info, &oui_subtype, OUI_SUB_SIZE)) {
+ return SUBTYPE_INVALID;
+ }
+
+ /* disable rx if tx has been disabled by administrator */
+ if (!is_tlv_txenabled(ed->ifname, TLVID_8021Qbg(LLDP_EVB_SUBTYPE))) {
+ LLDPAD_WARN("%s:%s:EVB Config disabled\n",
+ __func__, ed->ifname);
+ return TLV_OK;
+ }
+
+ LLDPAD_DBG("%s(%i): received tlv:", __func__, __LINE__);
+ evb_dump_tlv(tlv);
+ evb_print_tlvinfo(tie);
+
+ /* change state */
+ evb_statemachine(ed, tie);
+
+ /* check which values have been taken over */
+ LLDPAD_DBG("%s(%i): new tlv:", __func__, __LINE__);
+ evb_print_tlvinfo(ed->tie);
+ }
+
+ return TLV_OK;
+}
+
+void evb_ifdown(char *ifname)
+{
+ struct evb_data *ed;
+
+ LLDPAD_DBG("%s called !\n", __func__);
+
+ ed = evb_data(ifname);
+ if (!ed)
+ goto out_err;
+
+ free(ed->policy);
+ free(ed->tie);
+ LIST_REMOVE(ed, entry);
+ evb_free_tlv(ed);
+ free(ed);
+ LLDPAD_INFO("%s:port %s removed\n", __func__, ifname);
+ return;
+out_err:
+ LLDPAD_ERR("%s:port %s remove failed\n", __func__, ifname);
+
+ return;
+}
+
+void evb_ifup(char *ifname)
+{
+ struct evb_data *ed;
+ struct evb_user_data *ud;
+
+ ed = evb_data(ifname);
+ if (ed) {
+ LLDPAD_DBG("%s:%s exists\n", __func__, ifname);
+ goto out_err;
+ }
+
+ /* not found, alloc/init per-port tlv data */
+ ed = (struct evb_data *) calloc(1, sizeof(struct evb_data));
+ if (!ed) {
+ LLDPAD_ERR("%s:%s malloc %ld failed\n",
+ __func__, ifname, sizeof(*ed));
+ goto out_err;
+ }
+ strncpy(ed->ifname, ifname, IFNAMSIZ);
+
+ if (!init_cfg()) {
+ LLDPAD_ERR("%s:%s init_cfg failed\n", __func__, ifname);
+ goto out_free;
+ }
+
+ if (evb_init_cfg_tlv(ed)) {
+ LLDPAD_ERR("%s:%s evb_init_cfg_tlv failed\n", __func__, ifname);
+ goto out_free;
+ }
+
+ ed->state = EVB_OFFER_CAPABILITIES;
+
+ if (evb_bld_tlv(ed)) {
+ LLDPAD_ERR("%s:%s evb_bld_tlv failed\n", __func__, ifname);
+ goto out_free;
+ }
+
+ ud = find_module_user_data_by_if(ifname, &lldp_head, LLDP_MOD_EVB);
+ LIST_INSERT_HEAD(&ud->head, ed, entry);
+ LLDPAD_DBG("%s:port %s added\n", __func__, ifname);
+ return;
+
+out_free:
+ free(ed->tie);
+ free(ed->policy);
+ free(ed);
+
+out_err:
+ return;
+}
+
+static const struct lldp_mod_ops evb_ops = {
+ .lldp_mod_register = evb_register,
+ .lldp_mod_unregister = evb_unregister,
+ .lldp_mod_gettlv = evb_gettlv,
+ .lldp_mod_rchange = evb_rchange,
+ .lldp_mod_ifup = evb_ifup,
+ .lldp_mod_ifdown = evb_ifdown,
+ .get_arg_handler = evb_get_arg_handlers,
+};
+
+struct lldp_module *evb_register(void)
+{
+ struct lldp_module *mod;
+ struct evb_user_data *ud;
+
+ mod = malloc(sizeof(*mod));
+ if (!mod) {
+ LLDPAD_ERR("failed to malloc module data\n");
+ log_message(MSG_ERR_SERVICE_START_FAILURE,
+ "%s", "failed to malloc module data");
+ goto out_err;
+ }
+ ud = malloc(sizeof(struct evb_user_data));
+ if (!ud) {
+ free(mod);
+ LLDPAD_ERR("failed to malloc module user data\n");
+ log_message(MSG_ERR_SERVICE_START_FAILURE,
+ "%s", "failed to malloc module user data");
+ goto out_err;
+ }
+ LIST_INIT(&ud->head);
+ mod->id = LLDP_MOD_EVB;
+ mod->ops = &evb_ops;
+ mod->data = ud;
+ LLDPAD_DBG("%s:done\n", __func__);
+ return mod;
+
+out_err:
+ LLDPAD_ERR("%s:failed\n", __func__);
+ return NULL;
+}
+
+void evb_unregister(struct lldp_module *mod)
+{
+ if (mod->data) {
+ evb_free_data((struct evb_user_data *) mod->data);
+ free(mod->data);
+ }
+ free(mod);
+ LLDPAD_DBG("%s:done", __func__);
+}
diff --git a/lldp_evb_clif.c b/lldp_evb_clif.c
new file mode 100644
index 0000000..7479d74
--- /dev/null
+++ b/lldp_evb_clif.c
@@ -0,0 +1,226 @@
+/*******************************************************************************
+
+ implementation of EVB TLVs for LLDP
+ (c) Copyright IBM Corp. 2010
+
+ Author(s): Jens Osterkamp <jens@linux.vnet.ibm.com>
+
+ This program is free software; you can redistribute it and/or modify it
+ under the terms and conditions of the GNU General Public License,
+ version 2, as published by the Free Software Foundation.
+
+ This program is distributed in the hope it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ more details.
+
+ You should have received a copy of the GNU General Public License along with
+ this program; if not, write to the Free Software Foundation, Inc.,
+ 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+
+ The full GNU General Public License is included in this distribution in
+ the file called "COPYING".
+
+*******************************************************************************/
+
+#include "includes.h"
+#include "common.h"
+#include <stdio.h>
+#include <syslog.h>
+#include <sys/un.h>
+#include <sys/stat.h>
+#include "lldp_mod.h"
+#include "lldptool.h"
+#include "lldp.h"
+#include "lldp_evb.h"
+#include "lldp_evb_clif.h"
+
+void evb_print_cfg_tlv(u16, char *info);
+int evb_print_help();
+
+u32 evb_lookup_tlv_name(char *tlvid_str);
+
+static const struct lldp_mod_ops evb_ops_clif = {
+ .lldp_mod_register = evb_cli_register,
+ .lldp_mod_unregister = evb_cli_unregister,
+ .print_tlv = evb_print_tlv,
+ .lookup_tlv_name = evb_lookup_tlv_name,
+ .print_help = evb_print_help,
+};
+
+struct type_name_info evb_tlv_names[] = {
+ { (LLDP_MOD_EVB << 8) | LLDP_EVB_SUBTYPE,
+ "EVB Configuration TLV",
+ "evbCfg", evb_print_cfg_tlv },
+ { INVALID_TLVID, NULL, NULL }
+};
+
+int evb_print_help()
+{
+ struct type_name_info *tn = &evb_tlv_names[0];
+
+ while (tn->type != INVALID_TLVID) {
+ if (tn->key && strlen(tn->key) && tn->name) {
+ printf(" %s", tn->key);
+ if (strlen(tn->key)+3 <= 8)
+ printf("\t");
+ printf("\t: %s\n", tn->name);
+ }
+ tn++;
+ }
+
+ return 0;
+}
+
+struct lldp_module *evb_cli_register(void)
+{
+ struct lldp_module *mod;
+
+ mod = malloc(sizeof(*mod));
+ if (!mod) {
+ fprintf(stderr, "failed to malloc module data\n");
+ return NULL;
+ }
+ mod->id = LLDP_MOD_EVB;
+ mod->ops = &evb_ops_clif;
+
+ return mod;
+}
+
+void evb_cli_unregister(struct lldp_module *mod)
+{
+ free(mod);
+}
+
+void evb_print_cfg_tlv(u16 len, char *info)
+{
+ u8 smode;
+ u8 scap;
+ u8 cmode;
+ u8 ccap;
+ u16 svsi;
+ u16 cvsi;
+ u8 rte;
+
+ if (len != 9) {
+ printf("Bad Cfg TLV: %s\n", info);
+ return;
+ }
+
+ if (!hexstr2bin(info, &smode, sizeof(smode))) {
+ printf("supported forwarding mode: (0x%02hhx)", smode);
+
+ if (smode & LLDP_EVB_CAPABILITY_FORWARD_REFLECTIVE_RELAY)
+ printf(" reflective relay");
+
+ if (smode & LLDP_EVB_CAPABILITY_FORWARD_STANDARD)
+ printf(" standard 802.1Q");
+
+ printf("\n");
+ } else {
+ printf("Unable to decode smode !\n");
+ }
+
+ if (!hexstr2bin(info+2, &scap, sizeof(scap))) {
+ printf("\tsupported capabilities: (0x%02hhx)", scap);
+
+ if ( scap & LLDP_EVB_CAPABILITY_PROTOCOL_RTE)
+ printf(" RTE");
+
+ if ( scap & LLDP_EVB_CAPABILITY_PROTOCOL_ECP)
+ printf(" ECP");
+
+ if ( scap & LLDP_EVB_CAPABILITY_PROTOCOL_VDP)
+ printf(" VDP");
+
+ printf("\n");
+ } else {
+ printf("Unable to decode scap !\n");
+ }
+
+ if (!hexstr2bin(info+4, &cmode, sizeof(cmode))) {
+ printf("\tconfigured forwarding mode: (0x%02hhx)", cmode);
+
+ if (cmode & LLDP_EVB_CAPABILITY_FORWARD_REFLECTIVE_RELAY)
+ printf(" reflective relay");
+
+ if (cmode & LLDP_EVB_CAPABILITY_FORWARD_STANDARD)
+ printf(" standard 802.1Q");
+
+ printf("\n");
+ } else {
+ printf("Unable to decode cmode !\n");
+ }
+
+ if (!hexstr2bin(info+6, &ccap, sizeof(ccap))) {
+ printf("\tconfigured capabilities: (0x%02hhx)", ccap);
+
+ if ( ccap & LLDP_EVB_CAPABILITY_PROTOCOL_RTE)
+ printf(" RTE");
+
+ if ( ccap & LLDP_EVB_CAPABILITY_PROTOCOL_ECP)
+ printf(" ECP");
+
+ if ( ccap & LLDP_EVB_CAPABILITY_PROTOCOL_VDP)
+ printf(" VDP");
+
+ printf("\n");
+ } else {
+ printf("Unable to decode ccap !\n");
+ }
+
+ if (!hexstr2bin(info+8, (u8 *)&svsi, sizeof(svsi))) {
+ printf("\tno. of supported VSIs: %04i\n",svsi);
+ } else {
+ printf("Unable to decode svsi !\n");
+ }
+
+ if (!hexstr2bin(info+12, (u8 *)&cvsi, sizeof(cvsi))) {
+ printf("\tno. of configured VSIs: %04i\n",cvsi);
+ } else {
+ printf("Unable to decode cvsi !\n");
+ }
+
+ if (!hexstr2bin(info+16, &rte, sizeof(rte))) {
+ printf("\tRTE: %i\n",rte);
+ } else {
+ printf("Unable to decode cvsi !\n");
+ }
+
+ printf("\n");
+}
+
+/* return 1: if it printed the TLV
+ * 0: if it did not
+ */
+int evb_print_tlv(u32 tlvid, u16 len, char *info)
+{
+ struct type_name_info *tn = &evb_tlv_names[0];
+
+ while (tn->type != INVALID_TLVID) {
+ if (tlvid == tn->type) {
+ printf("%s\n", tn->name);
+ if (tn->print_info) {
+ printf("\t");
+ tn->print_info(len-4, info);
+ }
+ return 1;
+ }
+ tn++;
+ }
+
+ return 0;
+}
+
+u32 evb_lookup_tlv_name(char *tlvid_str)
+{
+ struct type_name_info *tn = &evb_tlv_names[0];
+
+ while (tn->type != INVALID_TLVID) {
+ if (!strcasecmp(tn->key, tlvid_str))
+ return tn->type;
+ tn++;
+ }
+ return INVALID_TLVID;
+}
+
diff --git a/lldp_evb_cmds.c b/lldp_evb_cmds.c
new file mode 100644
index 0000000..bda81c9
--- /dev/null
+++ b/lldp_evb_cmds.c
@@ -0,0 +1,519 @@
+/*******************************************************************************
+
+ implementation of EVB TLVs for LLDP
+ (c) Copyright IBM Corp. 2010
+
+ Author(s): Jens Osterkamp <jens@linux.vnet.ibm.com>
+
+ This program is free software; you can redistribute it and/or modify it
+ under the terms and conditions of the GNU General Public License,
+ version 2, as published by the Free Software Foundation.
+
+ This program is distributed in the hope it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ more details.
+
+ You should have received a copy of the GNU General Public License along with
+ this program; if not, write to the Free Software Foundation, Inc.,
+ 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+
+ The full GNU General Public License is included in this distribution in
+ the file called "COPYING".
+
+*******************************************************************************/
+
+#include "includes.h"
+#include "common.h"
+#include <stdio.h>
+#include <syslog.h>
+#include <sys/un.h>
+#include <sys/stat.h>
+#include <arpa/inet.h>
+#include <string.h>
+#include "lldpad.h"
+#include "ctrl_iface.h"
+#include "lldp.h"
+#include "lldp_evb.h"
+#include "lldp_mand_clif.h"
+#include "lldp_evb_clif.h"
+#include "lldp/ports.h"
+#include "libconfig.h"
+#include "config.h"
+#include "clif_msgs.h"
+#include "lldp/states.h"
+
+static int get_arg_tlvtxenable(struct cmd *, char *, char *, char *);
+static int set_arg_tlvtxenable(struct cmd *, char *, char *, char *);
+
+static int get_arg_fmode(struct cmd *, char *, char *, char *);
+static int set_arg_fmode(struct cmd *, char *, char *, char *);
+
+static int get_arg_rte(struct cmd *, char *, char *, char *);
+static int set_arg_rte(struct cmd *, char *, char *, char *);
+
+static int get_arg_vsis(struct cmd *, char *, char *, char *);
+static int set_arg_vsis(struct cmd *, char *, char *, char *);
+
+static int get_arg_capabilities(struct cmd *, char *, char *, char *);
+static int set_arg_capabilities(struct cmd *, char *, char *, char *);
+
+static struct arg_handlers arg_handlers[] = {
+ { ARG_EVB_FORWARDING_MODE, get_arg_fmode, set_arg_fmode },
+ { ARG_EVB_CAPABILITIES, get_arg_capabilities, set_arg_capabilities },
+ { ARG_EVB_VSIS, get_arg_vsis, set_arg_vsis },
+ { ARG_EVB_RTE, get_arg_rte, set_arg_rte },
+ { ARG_TLVTXENABLE, get_arg_tlvtxenable, set_arg_tlvtxenable },
+ { NULL }
+};
+
+static int get_arg_tlvtxenable(struct cmd *cmd, char *arg, char *argvalue,
+ char *obuf)
+{
+ int value;
+ char *s;
+ char arg_path[EVB_BUF_SIZE];
+
+ if (cmd->cmd != cmd_gettlv)
+ return cmd_invalid;
+
+ switch (cmd->tlvid) {
+ case (LLDP_MOD_EVB << 8) | LLDP_EVB_SUBTYPE:
+ snprintf(arg_path, sizeof(arg_path), "%s%08x.%s",
+ TLVID_PREFIX, cmd->tlvid, arg);
+
+ if (get_cfg(cmd->ifname, arg_path, (void *)&value,
+ CONFIG_TYPE_BOOL))
+ value = false;
+ break;
+ case INVALID_TLVID:
+ return cmd_invalid;
+ default:
+ return cmd_not_applicable;
+ }
+
+ if (value)
+ s = VAL_YES;
+ else
+ s = VAL_NO;
+
+ sprintf(obuf, "%02x%s%04x%s", (unsigned int) strlen(arg), arg,
+ (unsigned int) strlen(s), s);
+
+ return cmd_success;
+}
+
+static int set_arg_tlvtxenable(struct cmd *cmd, char *arg, char *argvalue,
+ char *obuf)
+{
+ int value;
+ char arg_path[EVB_BUF_SIZE];
+
+ if (cmd->cmd != cmd_settlv)
+ return cmd_invalid;
+
+ switch (cmd->tlvid) {
+ case (LLDP_MOD_EVB << 8) | LLDP_EVB_SUBTYPE:
+ break;
+ case INVALID_TLVID:
+ return cmd_invalid;
+ default:
+ return cmd_not_applicable;
+ }
+
+ if (!strcasecmp(argvalue, VAL_YES))
+ value = 1;
+ else if (!strcasecmp(argvalue, VAL_NO))
+ value = 0;
+ else
+ return cmd_invalid;
+
+ snprintf(arg_path, sizeof(arg_path), "%s%08x.%s",
+ TLVID_PREFIX, cmd->tlvid, arg);
+
+ if (set_cfg(cmd->ifname, arg_path, (void *)&value, CONFIG_TYPE_BOOL))
+ return cmd_failed;
+
+ somethingChangedLocal(cmd->ifname);
+
+ return cmd_success;
+}
+
+static int get_arg_fmode(struct cmd *cmd, char *arg, char *argvalue,
+ char *obuf)
+{
+ char *s;
+ struct evb_data *ed;
+
+ if (cmd->cmd != cmd_gettlv)
+ return cmd_invalid;
+
+ switch (cmd->tlvid) {
+ case (LLDP_MOD_EVB << 8) | LLDP_EVB_SUBTYPE:
+ break;
+ case INVALID_TLVID:
+ return cmd_invalid;
+ default:
+ return cmd_not_applicable;
+ }
+
+ ed = evb_data((char *) &cmd->ifname);
+
+ if (!ed)
+ return cmd_invalid;
+
+ if (ed->policy->smode & LLDP_EVB_CAPABILITY_FORWARD_REFLECTIVE_RELAY)
+ s = VAL_EVB_FMODE_REFLECTIVE_RELAY;
+ else
+ s = VAL_EVB_FMODE_BRIDGE;
+
+ sprintf(obuf, "%02x%s%04x%s", (unsigned int) strlen(arg), arg,
+ (unsigned int) strlen(s), s);
+
+ return cmd_success;
+}
+
+static int set_arg_fmode(struct cmd *cmd, char *arg, char *argvalue,
+ char *obuf)
+{
+ u8 smode;
+ char arg_path[EVB_BUF_SIZE];
+ struct evb_data *ed;
+
+ if (cmd->cmd != cmd_settlv)
+ return cmd_invalid;
+
+ switch (cmd->tlvid) {
+ case (LLDP_MOD_EVB << 8) | LLDP_EVB_SUBTYPE:
+ break;
+ case INVALID_TLVID:
+ return cmd_invalid;
+ default:
+ return cmd_not_applicable;
+ }
+
+ ed = evb_data((char *) &cmd->ifname);
+
+ if (!ed)
+ return cmd_invalid;
+
+ smode = 0;
+
+ if (!strcasecmp(argvalue, VAL_EVB_FMODE_BRIDGE)) {
+ smode = LLDP_EVB_CAPABILITY_FORWARD_STANDARD;
+ }
+
+ if (!strcasecmp(argvalue, VAL_EVB_FMODE_REFLECTIVE_RELAY)) {
+ smode = LLDP_EVB_CAPABILITY_FORWARD_REFLECTIVE_RELAY;
+ }
+
+ if (smode == 0) {
+ return cmd_invalid;
+ } else {
+ ed->policy->smode = smode;
+ }
+
+ snprintf(arg_path, sizeof(arg_path), "%s%08x.fmode",
+ TLVID_PREFIX, cmd->tlvid);
+
+ if (set_cfg(ed->ifname, arg_path, (void *) &argvalue, CONFIG_TYPE_STRING)) {
+ printf("%s:%s: saving EVB forwarding mode failed.\n",
+ __func__, ed->ifname);
+ return cmd_invalid;
+ }
+
+ somethingChangedLocal(cmd->ifname);
+
+ return cmd_success;
+}
+
+static int get_arg_capabilities(struct cmd *cmd, char *arg, char *argvalue,
+ char *obuf)
+{
+ int c;
+ char *s, *t;
+ struct evb_data *ed;
+
+ printf("%s(%i): arg %s, argvalue %s !\n", __func__, __LINE__, arg, argvalue);
+
+ s = t = malloc(EVB_BUF_SIZE);
+
+ if (!s)
+ goto out_free;
+
+ memset(s, 0, EVB_BUF_SIZE);
+
+ if (cmd->cmd != cmd_gettlv)
+ goto out_free;
+
+ switch (cmd->tlvid) {
+ case (LLDP_MOD_EVB << 8) | LLDP_EVB_SUBTYPE:
+ break;
+ case INVALID_TLVID:
+ goto out_free;
+ default:
+ free(t);
+ return cmd_not_applicable;
+ }
+
+ ed = evb_data((char *) &cmd->ifname);
+ if (!ed)
+ goto out_free;
+
+ if (ed->policy->scap & LLDP_EVB_CAPABILITY_PROTOCOL_RTE) {
+ c = sprintf(s, VAL_EVB_CAPA_RTE " ");
+ if (c <= 0)
+ goto out_free;
+ s += c;
+ }
+
+ if (ed->policy->scap & LLDP_EVB_CAPABILITY_PROTOCOL_ECP) {
+ c = sprintf(s, VAL_EVB_CAPA_ECP " ");
+ if (c <= 0)
+ goto out_free;
+ s += c;
+ }
+
+ if (ed->policy->scap & LLDP_EVB_CAPABILITY_PROTOCOL_VDP) {
+ c = sprintf(s, VAL_EVB_CAPA_VDP " ");
+ if (c <= 0)
+ goto out_free;
+ s += c;
+ }
+
+ sprintf(obuf, "%02x%s%04x%s", (unsigned int) strlen(arg), arg,
+ (unsigned int) strlen(t), t);
+
+ free(t);
+ return cmd_success;
+
+out_free:
+ free(t);
+ return cmd_invalid;
+}
+
+static int set_arg_capabilities(struct cmd *cmd, char *arg, char *argvalue,
+ char *obuf)
+{
+ u8 scap = 0;
+ char arg_path[EVB_BUF_SIZE];
+ struct evb_data *ed;
+
+ if (cmd->cmd != cmd_settlv)
+ return cmd_invalid;
+
+ switch (cmd->tlvid) {
+ case (LLDP_MOD_EVB << 8) | LLDP_EVB_SUBTYPE:
+ break;
+ case INVALID_TLVID:
+ return cmd_invalid;
+ default:
+ return cmd_not_applicable;
+ }
+
+ ed = evb_data((char *) &cmd->ifname);
+
+ if (!ed)
+ return cmd_invalid;
+
+ if (strcasestr(argvalue, VAL_EVB_CAPA_RTE))
+ scap |= LLDP_EVB_CAPABILITY_PROTOCOL_RTE;
+
+ if (strcasestr(argvalue, VAL_EVB_CAPA_ECP))
+ scap |= LLDP_EVB_CAPABILITY_PROTOCOL_ECP;
+
+ if (strcasestr(argvalue, VAL_EVB_CAPA_VDP))
+ scap |= LLDP_EVB_CAPABILITY_PROTOCOL_VDP;
+
+ ed->policy->scap = scap;
+
+ snprintf(arg_path, sizeof(arg_path), "%s%08x.capabilities",
+ TLVID_PREFIX, cmd->tlvid);
+
+ if (set_cfg(ed->ifname, arg_path, (void *) &argvalue, CONFIG_TYPE_STRING)) {
+ printf("%s:%s: saving EVB capabilities failed.\n",
+ __func__, ed->ifname);
+ return cmd_invalid;
+ }
+
+ somethingChangedLocal(cmd->ifname);
+
+ return cmd_success;
+}
+
+static int get_arg_rte(struct cmd *cmd, char *arg, char *argvalue,
+ char *obuf)
+{
+ char s[EVB_BUF_SIZE];
+ struct evb_data *ed;
+
+ if (cmd->cmd != cmd_gettlv)
+ return cmd_invalid;
+
+ switch (cmd->tlvid) {
+ case (LLDP_MOD_EVB << 8) | LLDP_EVB_SUBTYPE:
+ break;
+ case INVALID_TLVID:
+ return cmd_invalid;
+ default:
+ return cmd_not_applicable;
+ }
+
+ ed = evb_data((char *) &cmd->ifname);
+ if (!ed)
+ return cmd_invalid;
+
+ if (sprintf(s, "%i", ed->policy->rte) <= 0)
+ return cmd_invalid;
+
+ sprintf(obuf, "%02x%s%04x%s", (unsigned int) strlen(arg), arg,
+ (unsigned int) strlen(s), s);
+
+ return cmd_success;
+}
+
+static int set_arg_rte(struct cmd *cmd, char *arg, char *argvalue,
+ char *obuf)
+{
+ int value, err;
+ char arg_path[EVB_BUF_SIZE];
+ struct evb_data *ed = NULL;
+
+ if (cmd->cmd != cmd_settlv)
+ goto out_err;
+
+ switch (cmd->tlvid) {
+ case (LLDP_MOD_EVB << 8) | LLDP_EVB_SUBTYPE:
+ break;
+ case INVALID_TLVID:
+ goto out_err;
+ default:
+ return cmd_not_applicable;
+ }
+
+ ed = evb_data((char *) &cmd->ifname);
+
+ if (!ed)
+ return cmd_invalid;
+
+ value = atoi(argvalue);
+
+ if ((value < 0))
+ goto out_err;
+
+ ed->policy->rte = value;
+
+ err = snprintf(arg_path, sizeof(arg_path), "%s%08x.rte",
+ TLVID_PREFIX, cmd->tlvid);
+
+ if (err < 0)
+ goto out_err;
+
+ if (err < 0)
+ goto out_err;
+
+ if (set_cfg(ed->ifname, arg_path, (void *) &argvalue, CONFIG_TYPE_STRING))
+ goto out_err;
+
+ somethingChangedLocal(cmd->ifname);
+
+ return cmd_success;
+
+out_err:
+ printf("%s:%s: saving EVB rte failed.\n", __func__, ed->ifname);
+ return cmd_invalid;
+}
+
+static int get_arg_vsis(struct cmd *cmd, char *arg, char *argvalue,
+ char *obuf)
+{
+ char s[EVB_BUF_SIZE];
+ struct evb_data *ed;
+
+ if (cmd->cmd != cmd_gettlv)
+ return cmd_invalid;
+
+ switch (cmd->tlvid) {
+ case (LLDP_MOD_EVB << 8) | LLDP_EVB_SUBTYPE:
+ break;
+ case INVALID_TLVID:
+ return cmd_invalid;
+ default:
+ return cmd_not_applicable;
+ }
+
+ ed = evb_data((char *) &cmd->ifname);
+ if (!ed)
+ return cmd_invalid;
+
+ if (sprintf(s, "%04i", ed->policy->svsi) <= 0)
+ return cmd_invalid;
+
+ sprintf(obuf, "%02x%s%04x%s", (unsigned int) strlen(arg), arg,
+ (unsigned int) strlen(s), s);
+
+ return cmd_success;
+}
+
+static int set_arg_vsis(struct cmd *cmd, char *arg, char *argvalue,
+ char *obuf)
+{
+ int value, err;
+ char arg_path[EVB_BUF_SIZE];
+ char svalue[10];
+ char *sv;
+ struct evb_data *ed = NULL;
+
+ ed = evb_data((char *) &cmd->ifname);
+
+ if (!ed)
+ return cmd_invalid;
+
+ if (cmd->cmd != cmd_settlv)
+ goto out_err;
+
+ switch (cmd->tlvid) {
+ case (LLDP_MOD_EVB << 8) | LLDP_EVB_SUBTYPE:
+ break;
+ case INVALID_TLVID:
+ goto out_err;
+ default:
+ return cmd_not_applicable;
+ }
+
+ value = atoi(argvalue);
+
+ if ((value < 0) || (value > LLDP_EVB_DEFAULT_MAX_VSI))
+ goto out_err;
+
+ ed->policy->svsi = value;
+
+ err = snprintf(arg_path, sizeof(arg_path), "%s%08x.vsis",
+ TLVID_PREFIX, cmd->tlvid);
+
+ if (err < 0)
+ goto out_err;
+
+ err = snprintf(svalue, sizeof(svalue), "%i", value);
+
+ if (err < 0)
+ goto out_err;
+
+ sv = &svalue[0];
+
+ if (set_cfg(ed->ifname, arg_path, (void *) &sv, CONFIG_TYPE_STRING))
+ goto out_err;
+
+ somethingChangedLocal(cmd->ifname);
+
+ return cmd_success;
+
+out_err:
+ printf("%s:%s: saving EVB vsis failed.\n", __func__, ed->ifname);
+ return cmd_invalid;
+}
+
+struct arg_handlers *evb_get_arg_handlers()
+{
+ return &arg_handlers[0];
+}
diff --git a/lldpad.c b/lldpad.c
index a89b5a4..571da31 100644
--- a/lldpad.c
+++ b/lldpad.c
@@ -49,6 +49,7 @@
#include "lldp_dcbx.h"
#include "lldp_med.h"
#include "lldp_8023.h"
+#include "lldp_evb.h"
#include "config.h"
#include "lldpad_shm.h"
#include "clif.h"
@@ -63,6 +64,7 @@ struct lldp_module *(*register_tlv_table[])(void) = {
dcbx_register,
med_register,
ieee8023_register,
+ evb_register,
NULL,
};
diff --git a/lldptool.c b/lldptool.c
index 5cf2846..7e166fe 100644
--- a/lldptool.c
+++ b/lldptool.c
@@ -39,6 +39,7 @@
#include "lldp_med_clif.h"
#include "lldp_8023_clif.h"
#include "lldp_dcbx_clif.h"
+#include "lldp_evb_clif.h"
#include "lldptool.h"
#include "lldptool_cli.h"
#include "lldp_mod.h"
@@ -156,6 +157,7 @@ struct lldp_module *(*register_tlv_table[])(void) = {
ieee8023_cli_register,
med_cli_register,
dcbx_cli_register,
+ evb_cli_register,
NULL,
};
--
1.7.2.3
^ permalink raw reply related
* [PATCH 01/10] consolidation of MIN and MAX macros in common.h
From: Jens Osterkamp @ 2010-11-22 15:50 UTC (permalink / raw)
To: e1000-eedc, virtualization, evb; +Cc: chrisw, Jens Osterkamp
In-Reply-To: <1290441057-16729-1-git-send-email-jens@linux.vnet.ibm.com>
This patch consolidates a modified version of the already existing MIN
macro in lldpad to include/common.h and add a MAX macro.
Signed-off-by: Jens Osterkamp <jens@linux.vnet.ibm.com>
---
include/common.h | 14 ++++++++++++++
include/dcb_protocol.h | 4 ----
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/include/common.h b/include/common.h
index 01746ea..ab07dc2 100644
--- a/include/common.h
+++ b/include/common.h
@@ -387,4 +387,18 @@ typedef int socklen_t;
const char * wpa_ssid_txt(u8 *ssid, size_t ssid_len);
+#define MIN(x,y) \
+ ({ \
+ typeof (x) __x = (x); \
+ typeof (y) __y = (y); \
+ __x < __y ? __x : __y; \
+ })
+
+#define MAX(x,y) \
+ ({ \
+ typeof (x) __x = (x); \
+ typeof (y) __y = (y); \
+ __x > __y ? __x : __y; \
+ })
+
#endif /* COMMON_H */
diff --git a/include/dcb_protocol.h b/include/dcb_protocol.h
index 63e2d3c..d7127f5 100644
--- a/include/dcb_protocol.h
+++ b/include/dcb_protocol.h
@@ -58,10 +58,6 @@ typedef enum {
#define DUP_DCBX_TLV_LLINK 0x0020
#define TOO_MANY_NGHBRS 0x0040
-//#ifndef min /*todo: change to min()
- #define MIN(x, y) ((x) < (y) ? x : y)
-//#endif
-
#define INIT_DCB_OUI {0x00,0x1b,0x21}
int add_adapter(char *device_name);
--
1.7.2.3
^ permalink raw reply related
* implementation of IEEE 802.1Qbg in lldpad
From: Jens Osterkamp @ 2010-11-22 15:50 UTC (permalink / raw)
To: e1000-eedc, virtualization, evb; +Cc: chrisw
Hi,
this set of patches contains the initial implementation of the IEEE 802.1Qbg
standard: code for the exchange of EVB TLVs in LLDP frames to negotiate VSI
capabalities as well as VDP VSI TLVs between a host with virtual machines
and an adjacent switch.
It supports setting the parameters of the TLV exchange from the command
line using lldptool.
VDP profiles consisting of mode,mgrid,typeid,typeidversion,instanceid,mac,vlan
can be given to lldpad with lldptool or sent to lldpad via netlink messages from
the kernel or another program, e.g. libvirt.
VDP profiles are processed through the VDP/VSI and ECP state machines and
sent out in ECP frames.
ACK frames are received and processed through ECP and VDP/VSI state machines.
It implements a VDP bridge role for a port together with a lldptool command to
switch a port to the bridge role.
These patches apply against lldpad 0.9.38. Protocol debug messages are now logged
to syslog via LLDPAD_* macros and no longer to console. If you want to see them,
configure your syslog daemon to collect debug messages and start lldpad with -V 7.
Changes since last posting:
- implementation of VDP keepalive
- better handling of response/reason codes by the switch
- better handling of deassoc frame sent by switch
- improved (human readable) printout of profile
- bugfix to corrected wrong instance strlen
- cleanup of vdp state machine
- zero out unused part of ack frame
- simplify handling of pending profile changes
- simplify handling of response to libvirt
- bugfix to prevent duplicate sending of TLVs
For more information about lldpad take a look at
http://sourceforge.net/projects/e1000/files/DCB%20Tools/lldpad/
Please review and comment.
Thanks !
Jens
^ permalink raw reply
* Re: [RFC PATCH] virtio: (Partially) enable suspend/resume support
From: Matthew Garrett @ 2010-11-22 15:01 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Amit Shah, Anthony Liguori, Virtualization List
In-Reply-To: <20101122145454.GB30157@redhat.com>
On Mon, Nov 22, 2010 at 04:54:54PM +0200, Michael S. Tsirkin wrote:
> > What the Windows driver does is destroy all the virtqueues before
> > suspend and re-init all of them on restore. This works well, but in the
> > Linux case, we don't have a notifier that gets called after restore
> > succeeds and before user-space tasks are thawed,
>
> Let's add one?
May be difficult while retaining kABI, but it's certainly something that
can go upstream. If it's not possible within kABI constraints then we'll
need a workaround for 6.
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply
* Re: [RFC PATCH] virtio: (Partially) enable suspend/resume support
From: Michael S. Tsirkin @ 2010-11-22 14:54 UTC (permalink / raw)
To: Amit Shah; +Cc: Matthew Garrett, Anthony Liguori, Virtualization List
In-Reply-To: <20101122134537.GB6979@amit-x200.redhat.com>
On Mon, Nov 22, 2010 at 07:15:37PM +0530, Amit Shah wrote:
> (Adding Matthew Garrett and Vadim Rosenfeld)
>
> On (Fri) Nov 05 2010 [12:15:36], Michael S. Tsirkin wrote:
> > I still don't see it. why don't we reset on resume?
> > If there's a reset host must either discard or
> > flush out operations in progress.
>
> OK, let me list how virtio-serial works, then let's see how Windows and
> Linux differ in their suspend/resume implementations and then discuss
> solutions.
>
> virtio-serial, in its probe routine, exchanges some information over a
> control vq and per-port vqs -- the number of ports, the number of open
> ports, the names assigned to the ports, etc.
>
> When Linux starts up, it goes about doing the regular init, calling the
> probe routine. Later, when it detects there is a suspended image
> available, it restores that image.
>
> After the image is restored, the vring counts in qemu reflect the
> transfer that's taken place in the probe routine, whereas the vring
> counts in the guest kernel reflect the pre-suspend values.
>
> The Linux kernel's suspend/resume notifiers currently offer the
> following notifications:
> - Preparing to go to suspended state (tasks will be frozen now)
> - Preparing to restore the image saved at hibernate-time
> - Restore succeeded (called after user-space threads are thawed)
>
>
> What the Windows driver does is destroy all the virtqueues before
> suspend and re-init all of them on restore. This works well, but in the
> Linux case, we don't have a notifier that gets called after restore
> succeeds and before user-space tasks are thawed,
Let's add one?
> which means for an open
> virtio-serial port, a userspace app doing non-stop writes may find the
> communication channel broken because the underlying vq vanished.
>
> Windows has one problem with the balloon driver too -- inflate a
> balloon, hibernate. Start the machine, restore image. Windows thinks
> balloon is inflated. qemu thinks it's deflated. This can be solved by
> Windows providing a balloon size update to qemu but will most perhaps
> need a change to qemu.
>
> For network ports, I guess it'll be the same situation - getting the
> network up before userspace notices.
>
> Amit
^ permalink raw reply
* Re: [RFC PATCH] virtio: (Partially) enable suspend/resume support
From: Amit Shah @ 2010-11-22 13:45 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Matthew Garrett, Anthony Liguori, Virtualization List
In-Reply-To: <20101105101536.GA15514@redhat.com>
(Adding Matthew Garrett and Vadim Rosenfeld)
On (Fri) Nov 05 2010 [12:15:36], Michael S. Tsirkin wrote:
> I still don't see it. why don't we reset on resume?
> If there's a reset host must either discard or
> flush out operations in progress.
OK, let me list how virtio-serial works, then let's see how Windows and
Linux differ in their suspend/resume implementations and then discuss
solutions.
virtio-serial, in its probe routine, exchanges some information over a
control vq and per-port vqs -- the number of ports, the number of open
ports, the names assigned to the ports, etc.
When Linux starts up, it goes about doing the regular init, calling the
probe routine. Later, when it detects there is a suspended image
available, it restores that image.
After the image is restored, the vring counts in qemu reflect the
transfer that's taken place in the probe routine, whereas the vring
counts in the guest kernel reflect the pre-suspend values.
The Linux kernel's suspend/resume notifiers currently offer the
following notifications:
- Preparing to go to suspended state (tasks will be frozen now)
- Preparing to restore the image saved at hibernate-time
- Restore succeeded (called after user-space threads are thawed)
What the Windows driver does is destroy all the virtqueues before
suspend and re-init all of them on restore. This works well, but in the
Linux case, we don't have a notifier that gets called after restore
succeeds and before user-space tasks are thawed, which means for an open
virtio-serial port, a userspace app doing non-stop writes may find the
communication channel broken because the underlying vq vanished.
Windows has one problem with the balloon driver too -- inflate a
balloon, hibernate. Start the machine, restore image. Windows thinks
balloon is inflated. qemu thinks it's deflated. This can be solved by
Windows providing a balloon size update to qemu but will most perhaps
need a change to qemu.
For network ports, I guess it'll be the same situation - getting the
network up before userspace notices.
Amit
^ permalink raw reply
* Re: [Xen-devel] Re: [PATCH 09/14] xen/pvticketlock: Xen implementation for PV ticket locks
From: Jeremy Fitzhardinge @ 2010-11-17 17:41 UTC (permalink / raw)
To: Jan Beulich
Cc: Jeremy Fitzhardinge, Mathieu Desnoyers, Nick Piggin,
Peter Zijlstra, Linux Kernel Mailing List, Srivatsa Vaddagiri,
Eric Dumazet, Xen-devel, Avi Kivity, H. Peter Anvin,
xiyou.wangcong, Linux Virtualization
In-Reply-To: <4CE3BDCF0200007800022BF1@vpn.id2.novell.com>
On 11/17/2010 02:34 AM, Jan Beulich wrote:
>> Actually, on second thoughts, maybe it doesn't matter so much. The main
>> issue is making sure that the interrupt will make the VCPU drop out of
>> xen_poll_irq() - if it happens before xen_poll_irq(), it should leave
>> the event pending, which will cause the poll to return immediately. I
>> hope. Certainly disabling interrupts for some of the function will make
>> it easier to analyze with respect to interrupt nesting.
> That's not my main concern. Instead, what if you get interrupted
> anywhere here, the interrupt handler tries to acquire another
> spinlock and also has to go into the slow path? It'll overwrite part
> or all of the outer context's state.
That doesn't matter if the outer context doesn't end up blocking. If it
has already blocked then it will unblock as a result of the interrupt;
if it hasn't yet blocked, then the inner context will leave the event
pending and cause it to not block. Either way, it no longer uses or
needs that per-cpu state: it will return to the spin loop and (maybe)
get re-entered, setting it all up again.
I think there is a problem with the code as posted because it sets up
the percpu data before clearing the pending event, so it can end up
blocking with bad percpu data.
>> Another issue may be making sure the writes and reads of "w->want" and
>> "w->lock" are ordered properly to make sure that xen_unlock_kick() never
>> sees an inconsistent view of the (lock,want) tuple. The risk being that
>> xen_unlock_kick() sees a random, spurious (lock,want) pairing and sends
>> the kick event to the wrong VCPU, leaving the deserving one hung.
> Yes, proper operation sequence (and barriers) is certainly
> required here. If you allowed nesting, this may even become
> simpler (as you'd have a single write making visible the new
> "head" pointer, after having written all relevant fields of the
> new "head" structure).
Yes, simple nesting should be quite straightforward (ie allowing an
interrupt handler to take some other lock than the one the outer context
is waiting on).
J
^ permalink raw reply
* Re: [Xen-devel] Re: [PATCH 13/14] x86/ticketlock: add slowpath logic
From: Jeremy Fitzhardinge @ 2010-11-17 15:25 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Xen-devel, Mathieu Desnoyers, Nick Piggin,
Linux Kernel Mailing List, Jan Beulich, Srivatsa Vaddagiri,
Jeremy Fitzhardinge, Avi Kivity, H. Peter Anvin,
Américo Wang, Eric Dumazet, Linux Virtualization
In-Reply-To: <1289996503.2109.728.camel@laptop>
On 11/17/2010 04:21 AM, Peter Zijlstra wrote:
> On Tue, 2010-11-16 at 13:08 -0800, Jeremy Fitzhardinge wrote:
>> Maintain a flag in both LSBs of the ticket lock which indicates whether
>> anyone is in the lock slowpath and may need kicking when the current
>> holder unlocks. The flags are set when the first locker enters
>> the slowpath, and cleared when unlocking to an empty queue.
> So here you say you set both LSBs in order to keep head == tail working,
> but the code seems to suggest you only use the tail LSB.
>
> I think I see why using only one LSB is sufficient, but some consistency
> would be nice :-)
I tried that initially, but it turned out more messy. The problem is
that the flag can change while you're spinning on the lock, so you need
to mask it out every time you read tail before you can compare it to
head; if head has the flag set too, you just need to mask it out of
there as well:
ticket = xadd(lock, 2 << TICKET_SHIFT);
ticket.tail &= ~TICKET_SLOW_FLAGS;
while (ticket.head != ticket.tail) {
relax();
ticket.head = lock->head /* & ~TICKET_SLOW_FLAGS */;
}
IOW setting both doesn't help anything, and just requires an extra mask
in the spin loop (and anywhere else that uses 'head').
And hey, extra bit. Bound to be useful for something.
J
^ permalink raw reply
* Re: [PATCH 13/14] x86/ticketlock: add slowpath logic
From: Peter Zijlstra @ 2010-11-17 12:21 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Xen-devel, Mathieu Desnoyers, Nick Piggin, Srivatsa Vaddagiri,
Linux Kernel Mailing List, Jan Beulich, Eric Dumazet,
Jeremy Fitzhardinge, Avi Kivity, H. Peter Anvin,
Américo Wang, Linux Virtualization
In-Reply-To: <97ed99ae9160bdb6477284b333bd6708fb7a19cb.1289940821.git.jeremy.fitzhardinge@citrix.com>
On Tue, 2010-11-16 at 13:08 -0800, Jeremy Fitzhardinge wrote:
> Maintain a flag in both LSBs of the ticket lock which indicates whether
> anyone is in the lock slowpath and may need kicking when the current
> holder unlocks. The flags are set when the first locker enters
> the slowpath, and cleared when unlocking to an empty queue.
So here you say you set both LSBs in order to keep head == tail working,
but the code seems to suggest you only use the tail LSB.
I think I see why using only one LSB is sufficient, but some consistency
would be nice :-)
^ permalink raw reply
* Re: [Xen-devel] Re: [PATCH 09/14] xen/pvticketlock: Xen implementation for PV ticket locks
From: Jan Beulich @ 2010-11-17 10:34 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Jeremy Fitzhardinge, Mathieu Desnoyers, Nick Piggin,
Peter Zijlstra, Linux Kernel Mailing List, Srivatsa Vaddagiri,
Eric Dumazet, Xen-devel, Avi Kivity, H. Peter Anvin,
xiyou.wangcong, Linux Virtualization
In-Reply-To: <4CE3A714.9010509@goop.org>
>>> On 17.11.10 at 10:57, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> On 11/17/2010 12:52 AM, Jeremy Fitzhardinge wrote:
>> On 11/17/2010 12:11 AM, Jan Beulich wrote:
>>>>>> On 16.11.10 at 22:08, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>>>> +static void xen_lock_spinning(struct arch_spinlock *lock, unsigned want)
>>>> {
>>>> - struct xen_spinlock *xl = (struct xen_spinlock *)lock;
>>>> - struct xen_spinlock *prev;
>>>> int irq = __get_cpu_var(lock_kicker_irq);
>>>> - int ret;
>>>> + struct xen_lock_waiting *w = &__get_cpu_var(lock_waiting);
>>>> + int cpu = smp_processor_id();
>>>> u64 start;
>>>>
>>>> /* If kicker interrupts not initialized yet, just spin */
>>>> if (irq == -1)
>>>> - return 0;
>>>> + return;
>>>>
>>>> start = spin_time_start();
>>>>
>>>> - /* announce we're spinning */
>>>> - prev = spinning_lock(xl);
>>>> + w->want = want;
>>>> + w->lock = lock;
>>>> +
>>>> + /* This uses set_bit, which atomic and therefore a barrier */
>>>> + cpumask_set_cpu(cpu, &waiting_cpus);
>>> Since you don't allow nesting, don't you need to disable
>>> interrupts before you touch per-CPU state?
>> Yes, I think you're right - interrupts need to be disabled for the bulk
>> of this function.
>
> Actually, on second thoughts, maybe it doesn't matter so much. The main
> issue is making sure that the interrupt will make the VCPU drop out of
> xen_poll_irq() - if it happens before xen_poll_irq(), it should leave
> the event pending, which will cause the poll to return immediately. I
> hope. Certainly disabling interrupts for some of the function will make
> it easier to analyze with respect to interrupt nesting.
That's not my main concern. Instead, what if you get interrupted
anywhere here, the interrupt handler tries to acquire another
spinlock and also has to go into the slow path? It'll overwrite part
or all of the outer context's state.
> Another issue may be making sure the writes and reads of "w->want" and
> "w->lock" are ordered properly to make sure that xen_unlock_kick() never
> sees an inconsistent view of the (lock,want) tuple. The risk being that
> xen_unlock_kick() sees a random, spurious (lock,want) pairing and sends
> the kick event to the wrong VCPU, leaving the deserving one hung.
Yes, proper operation sequence (and barriers) is certainly
required here. If you allowed nesting, this may even become
simpler (as you'd have a single write making visible the new
"head" pointer, after having written all relevant fields of the
new "head" structure).
Jan
^ permalink raw reply
* Re: [Xen-devel] Re: [PATCH 09/14] xen/pvticketlock: Xen implementation for PV ticket locks
From: Jeremy Fitzhardinge @ 2010-11-17 9:57 UTC (permalink / raw)
To: Jan Beulich
Cc: Jeremy Fitzhardinge, Mathieu Desnoyers, Nick Piggin,
Peter Zijlstra, Srivatsa Vaddagiri, Linux Kernel Mailing List,
Xen-devel, Avi Kivity, H. Peter Anvin, xiyou.wangcong,
Eric Dumazet, Linux Virtualization
In-Reply-To: <4CE397E7.2010107@goop.org>
On 11/17/2010 12:52 AM, Jeremy Fitzhardinge wrote:
> On 11/17/2010 12:11 AM, Jan Beulich wrote:
>>>>> On 16.11.10 at 22:08, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>>> +static void xen_lock_spinning(struct arch_spinlock *lock, unsigned want)
>>> {
>>> - struct xen_spinlock *xl = (struct xen_spinlock *)lock;
>>> - struct xen_spinlock *prev;
>>> int irq = __get_cpu_var(lock_kicker_irq);
>>> - int ret;
>>> + struct xen_lock_waiting *w = &__get_cpu_var(lock_waiting);
>>> + int cpu = smp_processor_id();
>>> u64 start;
>>>
>>> /* If kicker interrupts not initialized yet, just spin */
>>> if (irq == -1)
>>> - return 0;
>>> + return;
>>>
>>> start = spin_time_start();
>>>
>>> - /* announce we're spinning */
>>> - prev = spinning_lock(xl);
>>> + w->want = want;
>>> + w->lock = lock;
>>> +
>>> + /* This uses set_bit, which atomic and therefore a barrier */
>>> + cpumask_set_cpu(cpu, &waiting_cpus);
>> Since you don't allow nesting, don't you need to disable
>> interrupts before you touch per-CPU state?
> Yes, I think you're right - interrupts need to be disabled for the bulk
> of this function.
Actually, on second thoughts, maybe it doesn't matter so much. The main
issue is making sure that the interrupt will make the VCPU drop out of
xen_poll_irq() - if it happens before xen_poll_irq(), it should leave
the event pending, which will cause the poll to return immediately. I
hope. Certainly disabling interrupts for some of the function will make
it easier to analyze with respect to interrupt nesting.
Another issue may be making sure the writes and reads of "w->want" and
"w->lock" are ordered properly to make sure that xen_unlock_kick() never
sees an inconsistent view of the (lock,want) tuple. The risk being that
xen_unlock_kick() sees a random, spurious (lock,want) pairing and sends
the kick event to the wrong VCPU, leaving the deserving one hung.
J
^ permalink raw reply
* Re: [PATCH 13/14] x86/ticketlock: add slowpath logic
From: Jan Beulich @ 2010-11-17 9:34 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Jeremy Fitzhardinge, Mathieu Desnoyers, Nick Piggin,
Peter Zijlstra, Linux Kernel Mailing List, Srivatsa Vaddagiri,
Eric Dumazet, Xen-devel, Avi Kivity, H. Peter Anvin,
xiyou.wangcong, Linux Virtualization
In-Reply-To: <4CE39B7A.1080706@goop.org>
>>> On 17.11.10 at 10:08, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> On 11/17/2010 12:56 AM, Jeremy Fitzhardinge wrote:
>> On 11/17/2010 12:52 AM, Jeremy Fitzhardinge wrote:
>>> But, yes, %z0 sounds interesting. Is it documented anywhere? I think
>>> I've tried to use it in the past and run into gcc bugs.
>> This one: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39590
>>
>> Should be OK in this case because there's no 64-bit values to be seen...
> Hm, it fails when __ticket_t is 16 bits:
>
> /home/jeremy/git/linux/arch/x86/include/asm/spinlock.h: Assembler messages:
> /home/jeremy/git/linux/arch/x86/include/asm/spinlock.h:73: Error: suffix
> or operands invalid for `or'
>
> lock; ors $1, 2(%rbx) #,
>
>
> So I don't think that's going to work out...
Indeed, it's only with 4.5 that non-float operands are properly
supported here. Sad.
Jan
^ permalink raw reply
* Re: [PATCH 13/14] x86/ticketlock: add slowpath logic
From: Avi Kivity @ 2010-11-17 9:10 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Jeremy Fitzhardinge, Mathieu Desnoyers, Nick Piggin,
Peter Zijlstra, Linux Kernel Mailing List, Srivatsa Vaddagiri,
Jan Beulich, Eric Dumazet, Xen-devel, H. Peter Anvin,
xiyou.wangcong, Linux Virtualization
In-Reply-To: <4CE39AD2.9080606@goop.org>
On 11/17/2010 11:05 AM, Jeremy Fitzhardinge wrote:
> On 11/17/2010 12:58 AM, Avi Kivity wrote:
> >> Actually in this case I'm pretty sure there's already a "set bit"
> >> function which will do the job. set_bit(), I guess, though it takes a
> >> bit number rather than a mask...
> >>
> >
> >
> > set_bit() operates on a long, while the intel manuals recommend
> > against operating on operands of different size, especially with
> > locked operations. I think newer processors have more relaxed
> > requirements, though.
>
> Despite its prototype, set_bit() is pretty specifically using "orb" for
> a the constant case, or bts otherwise (I don't know what size memory
> operation bts is considered to generate).
>
Perhaps that should be fixed.
bts will take its size from the argument, so it will be a btsq on
x86_64. AFAICT, the only visible difference between btsl and btsq is a
page fault if the last four bytes of the operand are in an unmapped page.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply
* Re: [PATCH 13/14] x86/ticketlock: add slowpath logic
From: Jeremy Fitzhardinge @ 2010-11-17 9:08 UTC (permalink / raw)
To: Jan Beulich
Cc: Jeremy Fitzhardinge, Mathieu Desnoyers, Nick Piggin,
Peter Zijlstra, Linux Kernel Mailing List, Srivatsa Vaddagiri,
Xen-devel, Avi Kivity, H. Peter Anvin, xiyou.wangcong,
Eric Dumazet, Linux Virtualization
In-Reply-To: <4CE398A0.3040105@goop.org>
On 11/17/2010 12:56 AM, Jeremy Fitzhardinge wrote:
> On 11/17/2010 12:52 AM, Jeremy Fitzhardinge wrote:
>> But, yes, %z0 sounds interesting. Is it documented anywhere? I think
>> I've tried to use it in the past and run into gcc bugs.
> This one: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39590
>
> Should be OK in this case because there's no 64-bit values to be seen...
Hm, it fails when __ticket_t is 16 bits:
/home/jeremy/git/linux/arch/x86/include/asm/spinlock.h: Assembler messages:
/home/jeremy/git/linux/arch/x86/include/asm/spinlock.h:73: Error: suffix
or operands invalid for `or'
lock; ors $1, 2(%rbx) #,
So I don't think that's going to work out...
J
^ permalink raw reply
* Re: [PATCH 13/14] x86/ticketlock: add slowpath logic
From: Jeremy Fitzhardinge @ 2010-11-17 9:05 UTC (permalink / raw)
To: Avi Kivity
Cc: Jeremy Fitzhardinge, Mathieu Desnoyers, Nick Piggin,
Peter Zijlstra, Linux Kernel Mailing List, Srivatsa Vaddagiri,
Jan Beulich, Eric Dumazet, Xen-devel, H. Peter Anvin,
xiyou.wangcong, Linux Virtualization
In-Reply-To: <4CE3992E.2070406@redhat.com>
On 11/17/2010 12:58 AM, Avi Kivity wrote:
>> Actually in this case I'm pretty sure there's already a "set bit"
>> function which will do the job. set_bit(), I guess, though it takes a
>> bit number rather than a mask...
>>
>
>
> set_bit() operates on a long, while the intel manuals recommend
> against operating on operands of different size, especially with
> locked operations. I think newer processors have more relaxed
> requirements, though.
Despite its prototype, set_bit() is pretty specifically using "orb" for
a the constant case, or bts otherwise (I don't know what size memory
operation bts is considered to generate).
J
^ permalink raw reply
* Re: [PATCH 13/14] x86/ticketlock: add slowpath logic
From: Avi Kivity @ 2010-11-17 8:58 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Jeremy Fitzhardinge, Mathieu Desnoyers, Nick Piggin,
Peter Zijlstra, Linux Kernel Mailing List, Srivatsa Vaddagiri,
Jan Beulich, Eric Dumazet, Xen-devel, H. Peter Anvin,
xiyou.wangcong, Linux Virtualization
In-Reply-To: <4CE397C8.9040901@goop.org>
On 11/17/2010 10:52 AM, Jeremy Fitzhardinge wrote:
> On 11/17/2010 12:31 AM, Jan Beulich wrote:
> >>>> On 16.11.10 at 22:08, Jeremy Fitzhardinge<jeremy@goop.org> wrote:
> >> +static inline void __ticket_enter_slowpath(struct arch_spinlock *lock)
> >> +{
> >> + if (sizeof(lock->tickets.tail) == sizeof(u8))
> >> + asm (LOCK_PREFIX "orb %1, %0"
> >> + : "+m" (lock->tickets.tail)
> >> + : "i" (TICKET_SLOWPATH_FLAG) : "memory");
> >> + else
> >> + asm (LOCK_PREFIX "orw %1, %0"
> >> + : "+m" (lock->tickets.tail)
> >> + : "i" (TICKET_SLOWPATH_FLAG) : "memory");
> >> +}
> > Came only now to mind: Here and elsewhere, did you try using
> > %z0 to have gcc produce the opcode suffix character, rather
> > than having these somewhat ugly if()-s?
>
> Actually in this case I'm pretty sure there's already a "set bit"
> function which will do the job. set_bit(), I guess, though it takes a
> bit number rather than a mask...
>
set_bit() operates on a long, while the intel manuals recommend against
operating on operands of different size, especially with locked
operations. I think newer processors have more relaxed requirements,
though.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply
* Re: [PATCH 00/14] PV ticket locks without expanding spinlock
From: Avi Kivity @ 2010-11-17 8:56 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Xen-devel, Mathieu Desnoyers, Nick Piggin, Peter Zijlstra,
Srivatsa Vaddagiri, Linux Kernel Mailing List, Jan Beulich,
Eric Dumazet, Jeremy Fitzhardinge, H. Peter Anvin,
Américo Wang, Linux Virtualization
In-Reply-To: <cover.1289940821.git.jeremy.fitzhardinge@citrix.com>
On 11/16/2010 11:08 PM, Jeremy Fitzhardinge wrote:
> From: Jeremy Fitzhardinge<jeremy.fitzhardinge@citrix.com>
>
> Hi all,
>
> This is a revised version of the pvticket lock series.
>
> The early part of the series is mostly unchanged: it converts the bulk
> of the ticket lock code into C and makes the "small" and "large"
> ticket code common. The only changes are the incorporation of various
> review comments.
>
> The latter part of the series converts from pv spinlocks to pv ticket
> locks (ie, using the ticket lock fastpath as-is, but adding pv ops for
> the ticketlock slowpaths).
>
> The significant difference here is that rather than adding a new
> ticket_t-sized element to arch_spinlock_t - effectively doubling the
> size - I steal the LSB of the tickets themselves to store a bit. This
> allows the structure to remain the same size, but at the cost of
> halving the max number of CPUs (127 for a 8-bit ticket, and a hard max
> of 32767 overall).
>
> The extra bit (well, two, but one is unused) in indicates whether the
> lock has gone into "slowpath state", which means one of its lockers
> has entered its slowpath and has blocked in the hypervisor. This
> means the current lock-holder needs to make sure it gets kicked out of
> the hypervisor on unlock.
>
> The spinlock remains in slowpath state until the last unlock happens
> (ie there are no more queued lockers).
>
> This code survives for a while with moderate testing, (make -j 100 on
> 8 VCPUs on a 4 PCPU system), but locks up after about 20 iterations,
> so there's still some race/deadlock in there (probably something
> misordered), but I think the basic approach is sound.
This is going to be very useful for kvm; I'd like to see the fixed
version gets merged.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply
* Re: [PATCH 13/14] x86/ticketlock: add slowpath logic
From: Jeremy Fitzhardinge @ 2010-11-17 8:56 UTC (permalink / raw)
To: Jan Beulich
Cc: Jeremy Fitzhardinge, Mathieu Desnoyers, Nick Piggin,
Peter Zijlstra, Srivatsa Vaddagiri, Linux Kernel Mailing List,
Xen-devel, Avi Kivity, H. Peter Anvin, xiyou.wangcong,
Eric Dumazet, Linux Virtualization
In-Reply-To: <4CE397C8.9040901@goop.org>
On 11/17/2010 12:52 AM, Jeremy Fitzhardinge wrote:
> But, yes, %z0 sounds interesting. Is it documented anywhere? I think
> I've tried to use it in the past and run into gcc bugs.
This one: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39590
Should be OK in this case because there's no 64-bit values to be seen...
J
^ permalink raw reply
* Re: [PATCH 09/14] xen/pvticketlock: Xen implementation for PV ticket locks
From: Jeremy Fitzhardinge @ 2010-11-17 8:52 UTC (permalink / raw)
To: Jan Beulich
Cc: Jeremy Fitzhardinge, Mathieu Desnoyers, Nick Piggin,
Peter Zijlstra, Linux Kernel Mailing List, Srivatsa Vaddagiri,
Eric Dumazet, Xen-devel, Avi Kivity, H. Peter Anvin,
xiyou.wangcong, Linux Virtualization
In-Reply-To: <4CE39C3C0200007800022AE2@vpn.id2.novell.com>
On 11/17/2010 12:11 AM, Jan Beulich wrote:
>>>> On 16.11.10 at 22:08, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>> +static void xen_lock_spinning(struct arch_spinlock *lock, unsigned want)
>> {
>> - struct xen_spinlock *xl = (struct xen_spinlock *)lock;
>> - struct xen_spinlock *prev;
>> int irq = __get_cpu_var(lock_kicker_irq);
>> - int ret;
>> + struct xen_lock_waiting *w = &__get_cpu_var(lock_waiting);
>> + int cpu = smp_processor_id();
>> u64 start;
>>
>> /* If kicker interrupts not initialized yet, just spin */
>> if (irq == -1)
>> - return 0;
>> + return;
>>
>> start = spin_time_start();
>>
>> - /* announce we're spinning */
>> - prev = spinning_lock(xl);
>> + w->want = want;
>> + w->lock = lock;
>> +
>> + /* This uses set_bit, which atomic and therefore a barrier */
>> + cpumask_set_cpu(cpu, &waiting_cpus);
> Since you don't allow nesting, don't you need to disable
> interrupts before you touch per-CPU state?
Yes, I think you're right - interrupts need to be disabled for the bulk
of this function.
J
^ permalink raw reply
* Re: [PATCH 13/14] x86/ticketlock: add slowpath logic
From: Jeremy Fitzhardinge @ 2010-11-17 8:52 UTC (permalink / raw)
To: Jan Beulich
Cc: Jeremy Fitzhardinge, Mathieu Desnoyers, Nick Piggin,
Peter Zijlstra, Linux Kernel Mailing List, Srivatsa Vaddagiri,
Eric Dumazet, Xen-devel, Avi Kivity, H. Peter Anvin,
xiyou.wangcong, Linux Virtualization
In-Reply-To: <4CE3A1060200007800022B04@vpn.id2.novell.com>
On 11/17/2010 12:31 AM, Jan Beulich wrote:
>>>> On 16.11.10 at 22:08, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>> +static inline void __ticket_enter_slowpath(struct arch_spinlock *lock)
>> +{
>> + if (sizeof(lock->tickets.tail) == sizeof(u8))
>> + asm (LOCK_PREFIX "orb %1, %0"
>> + : "+m" (lock->tickets.tail)
>> + : "i" (TICKET_SLOWPATH_FLAG) : "memory");
>> + else
>> + asm (LOCK_PREFIX "orw %1, %0"
>> + : "+m" (lock->tickets.tail)
>> + : "i" (TICKET_SLOWPATH_FLAG) : "memory");
>> +}
> Came only now to mind: Here and elsewhere, did you try using
> %z0 to have gcc produce the opcode suffix character, rather
> than having these somewhat ugly if()-s?
Actually in this case I'm pretty sure there's already a "set bit"
function which will do the job. set_bit(), I guess, though it takes a
bit number rather than a mask...
But, yes, %z0 sounds interesting. Is it documented anywhere? I think
I've tried to use it in the past and run into gcc bugs.
Thanks,
J
^ permalink raw reply
* Re: [PATCH 13/14] x86/ticketlock: add slowpath logic
From: Jan Beulich @ 2010-11-17 8:31 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Jeremy Fitzhardinge, Mathieu Desnoyers, Nick Piggin,
Peter Zijlstra, Linux Kernel Mailing List, Srivatsa Vaddagiri,
Eric Dumazet, Xen-devel, Avi Kivity, H. Peter Anvin,
xiyou.wangcong, Linux Virtualization
In-Reply-To: <97ed99ae9160bdb6477284b333bd6708fb7a19cb.1289940821.git.jeremy.fitzhardinge@citrix.com>
>>> On 16.11.10 at 22:08, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> +static inline void __ticket_enter_slowpath(struct arch_spinlock *lock)
> +{
> + if (sizeof(lock->tickets.tail) == sizeof(u8))
> + asm (LOCK_PREFIX "orb %1, %0"
> + : "+m" (lock->tickets.tail)
> + : "i" (TICKET_SLOWPATH_FLAG) : "memory");
> + else
> + asm (LOCK_PREFIX "orw %1, %0"
> + : "+m" (lock->tickets.tail)
> + : "i" (TICKET_SLOWPATH_FLAG) : "memory");
> +}
Came only now to mind: Here and elsewhere, did you try using
%z0 to have gcc produce the opcode suffix character, rather
than having these somewhat ugly if()-s?
Jan
^ permalink raw reply
* Re: [PATCH 09/14] xen/pvticketlock: Xen implementation for PV ticket locks
From: Jan Beulich @ 2010-11-17 8:11 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Jeremy Fitzhardinge, Mathieu Desnoyers, Nick Piggin,
Peter Zijlstra, Linux Kernel Mailing List, Srivatsa Vaddagiri,
Eric Dumazet, Xen-devel, Avi Kivity, H. Peter Anvin,
xiyou.wangcong, Linux Virtualization
In-Reply-To: <aa32da076143b8e13c23c1f589d7e6cbedb22907.1289940821.git.jeremy.fitzhardinge@citrix.com>
>>> On 16.11.10 at 22:08, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> +static void xen_lock_spinning(struct arch_spinlock *lock, unsigned want)
> {
> - struct xen_spinlock *xl = (struct xen_spinlock *)lock;
> - struct xen_spinlock *prev;
> int irq = __get_cpu_var(lock_kicker_irq);
> - int ret;
> + struct xen_lock_waiting *w = &__get_cpu_var(lock_waiting);
> + int cpu = smp_processor_id();
> u64 start;
>
> /* If kicker interrupts not initialized yet, just spin */
> if (irq == -1)
> - return 0;
> + return;
>
> start = spin_time_start();
>
> - /* announce we're spinning */
> - prev = spinning_lock(xl);
> + w->want = want;
> + w->lock = lock;
> +
> + /* This uses set_bit, which atomic and therefore a barrier */
> + cpumask_set_cpu(cpu, &waiting_cpus);
Since you don't allow nesting, don't you need to disable
interrupts before you touch per-CPU state?
Jan
^ permalink raw reply
* [PATCH 14/14] x86/ticketlocks: tidy up __ticket_unlock_kick()
From: Jeremy Fitzhardinge @ 2010-11-16 21:08 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Xen-devel, Mathieu Desnoyers, Nick Piggin, Srivatsa Vaddagiri,
Linux Kernel Mailing List, Jan Beulich, Eric Dumazet,
Jeremy Fitzhardinge, Avi Kivity, H. Peter Anvin,
Américo Wang, Linux Virtualization
In-Reply-To: <cover.1289940821.git.jeremy.fitzhardinge@citrix.com>
From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
__ticket_unlock_kick() is now only called from known slowpaths, so there's
no need for it to do any checking of its own.
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
arch/x86/include/asm/paravirt.h | 2 +-
arch/x86/include/asm/spinlock.h | 14 --------------
2 files changed, 1 insertions(+), 15 deletions(-)
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 6f275ca..7755b16 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -722,7 +722,7 @@ static inline void __ticket_lock_spinning(struct arch_spinlock *lock, unsigned t
PVOP_VCALLEE2(pv_lock_ops.lock_spinning, lock, ticket);
}
-static inline void ____ticket_unlock_kick(struct arch_spinlock *lock, unsigned ticket)
+static inline void __ticket_unlock_kick(struct arch_spinlock *lock, unsigned ticket)
{
PVOP_VCALL2(pv_lock_ops.unlock_kick, lock, ticket);
}
diff --git a/arch/x86/include/asm/spinlock.h b/arch/x86/include/asm/spinlock.h
index 8d1cb42..70675bc 100644
--- a/arch/x86/include/asm/spinlock.h
+++ b/arch/x86/include/asm/spinlock.h
@@ -90,10 +90,6 @@ static __always_inline void __ticket_lock_spinning(struct arch_spinlock *lock, u
{
}
-static __always_inline void ____ticket_unlock_kick(struct arch_spinlock *lock, unsigned ticket)
-{
-}
-
#endif /* CONFIG_PARAVIRT_SPINLOCKS */
/*
@@ -133,16 +129,6 @@ static __always_inline struct __raw_tickets __ticket_spin_claim(struct arch_spin
return tickets.tickets;
}
-/*
- * If a spinlock has someone waiting on it, then kick the appropriate
- * waiting cpu.
- */
-static __always_inline void __ticket_unlock_kick(struct arch_spinlock *lock, __ticket_t next)
-{
- if (unlikely(lock->tickets.tail != next))
- ____ticket_unlock_kick(lock, next);
-}
-
static __always_inline void arch_spin_lock(struct arch_spinlock *lock)
{
register struct __raw_tickets inc;
--
1.7.2.3
^ permalink raw reply related
* [PATCH 13/14] x86/ticketlock: add slowpath logic
From: Jeremy Fitzhardinge @ 2010-11-16 21:08 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Xen-devel, Mathieu Desnoyers, Nick Piggin, Srivatsa Vaddagiri,
Linux Kernel Mailing List, Jan Beulich, Eric Dumazet,
Jeremy Fitzhardinge, Avi Kivity, H. Peter Anvin,
Américo Wang, Linux Virtualization
In-Reply-To: <cover.1289940821.git.jeremy.fitzhardinge@citrix.com>
From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Maintain a flag in both LSBs of the ticket lock which indicates whether
anyone is in the lock slowpath and may need kicking when the current
holder unlocks. The flags are set when the first locker enters
the slowpath, and cleared when unlocking to an empty queue.
In the specific implementation of lock_spinning(), make sure to set
the slowpath flags on the lock just before blocking. We must do
this before the last-chance pickup test to prevent a deadlock
with the unlocker:
Unlocker Locker
test for lock pickup
-> fail
test slowpath + unlock
-> false
set slowpath flags
block
Whereas this works in any ordering:
Unlocker Locker
set slowpath flags
test for lock pickup
-> fail
block
test slowpath + unlock
-> true, kick
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
arch/x86/include/asm/spinlock.h | 53 ++++++++++++++++++++++++++++-----
arch/x86/include/asm/spinlock_types.h | 2 +
arch/x86/kernel/paravirt-spinlocks.c | 37 +++++++++++++++++++++++
arch/x86/xen/spinlock.c | 4 ++
4 files changed, 88 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/spinlock.h b/arch/x86/include/asm/spinlock.h
index 9e1c7ce..8d1cb42 100644
--- a/arch/x86/include/asm/spinlock.h
+++ b/arch/x86/include/asm/spinlock.h
@@ -53,7 +53,38 @@ static __always_inline void __ticket_unlock_release(struct arch_spinlock *lock)
/* How long a lock should spin before we consider blocking */
#define SPIN_THRESHOLD (1 << 11)
-#ifndef CONFIG_PARAVIRT_SPINLOCKS
+/* Only defined when CONFIG_PARAVIRT_SPINLOCKS defined, but may as
+ * well leave the prototype always visible. */
+extern void __ticket_unlock_release_slowpath(struct arch_spinlock *lock);
+
+#ifdef CONFIG_PARAVIRT_SPINLOCKS
+
+/*
+ * Return true if someone is in the slowpath on this lock. This
+ * should only be used by the current lock-holder.
+ */
+static inline bool __ticket_in_slowpath(struct arch_spinlock *lock)
+{
+ return !!(lock->tickets.tail & TICKET_SLOWPATH_FLAG);
+}
+
+static inline void __ticket_enter_slowpath(struct arch_spinlock *lock)
+{
+ if (sizeof(lock->tickets.tail) == sizeof(u8))
+ asm (LOCK_PREFIX "orb %1, %0"
+ : "+m" (lock->tickets.tail)
+ : "i" (TICKET_SLOWPATH_FLAG) : "memory");
+ else
+ asm (LOCK_PREFIX "orw %1, %0"
+ : "+m" (lock->tickets.tail)
+ : "i" (TICKET_SLOWPATH_FLAG) : "memory");
+}
+
+#else /* !CONFIG_PARAVIRT_SPINLOCKS */
+static inline bool __ticket_in_slowpath(struct arch_spinlock *lock)
+{
+ return false;
+}
static __always_inline void __ticket_lock_spinning(struct arch_spinlock *lock, unsigned ticket)
{
@@ -84,18 +115,22 @@ static __always_inline void ____ticket_unlock_kick(struct arch_spinlock *lock, u
*/
static __always_inline struct __raw_tickets __ticket_spin_claim(struct arch_spinlock *lock)
{
- register struct __raw_tickets tickets = { .tail = TICKET_LOCK_INC };
+ register struct arch_spinlock tickets = {
+ { .tickets.tail = TICKET_LOCK_INC }
+ };
if (sizeof(lock->tickets.head) == sizeof(u8))
asm volatile (LOCK_PREFIX "xaddw %w0, %1\n"
- : "+r" (tickets), "+m" (lock->tickets)
+ : "+r" (tickets.head_tail), "+m" (lock->tickets)
: : "memory", "cc");
else
asm volatile (LOCK_PREFIX "xaddl %0, %1\n"
- : "+r" (tickets), "+m" (lock->tickets)
+ : "+r" (tickets.head_tail), "+m" (lock->tickets)
: : "memory", "cc");
- return tickets;
+ tickets.tickets.tail &= ~TICKET_SLOWPATH_FLAG;
+
+ return tickets.tickets;
}
/*
@@ -144,9 +179,11 @@ static __always_inline int arch_spin_trylock(arch_spinlock_t *lock)
static __always_inline void arch_spin_unlock(arch_spinlock_t *lock)
{
- __ticket_t next = lock->tickets.head + TICKET_LOCK_INC;
- __ticket_unlock_release(lock);
- __ticket_unlock_kick(lock, next);
+ barrier(); /* prevent reordering out of locked region */
+ if (unlikely(__ticket_in_slowpath(lock)))
+ __ticket_unlock_release_slowpath(lock);
+ else
+ __ticket_unlock_release(lock);
barrier(); /* prevent reordering into locked region */
}
diff --git a/arch/x86/include/asm/spinlock_types.h b/arch/x86/include/asm/spinlock_types.h
index 0553c0b..7b383e2 100644
--- a/arch/x86/include/asm/spinlock_types.h
+++ b/arch/x86/include/asm/spinlock_types.h
@@ -9,8 +9,10 @@
#ifdef CONFIG_PARAVIRT_SPINLOCKS
#define __TICKET_LOCK_INC 2
+#define TICKET_SLOWPATH_FLAG ((__ticket_t)1)
#else
#define __TICKET_LOCK_INC 1
+#define TICKET_SLOWPATH_FLAG ((__ticket_t)0)
#endif
#if (CONFIG_NR_CPUS < (256 / __TICKET_LOCK_INC))
diff --git a/arch/x86/kernel/paravirt-spinlocks.c b/arch/x86/kernel/paravirt-spinlocks.c
index 4251c1d..21b6986 100644
--- a/arch/x86/kernel/paravirt-spinlocks.c
+++ b/arch/x86/kernel/paravirt-spinlocks.c
@@ -15,3 +15,40 @@ struct pv_lock_ops pv_lock_ops = {
};
EXPORT_SYMBOL(pv_lock_ops);
+
+/*
+ * If we're unlocking and we're leaving the lock uncontended (there's
+ * nobody else waiting for the lock), then we can clear the slowpath
+ * bits. However, we need to be careful about this because someone
+ * may just be entering as we leave, and enter the slowpath.
+ */
+void __ticket_unlock_release_slowpath(struct arch_spinlock *lock)
+{
+ struct arch_spinlock old, new;
+
+ BUILD_BUG_ON(((__ticket_t)NR_CPUS) != NR_CPUS);
+
+ old = ACCESS_ONCE(*lock);
+
+ new = old;
+ new.tickets.head += TICKET_LOCK_INC;
+
+ /* Clear the slowpath flag */
+ new.tickets.tail &= ~TICKET_SLOWPATH_FLAG;
+
+ /*
+ * If there's currently people waiting or someone snuck in
+ * since we read the lock above, then do a normal unlock and
+ * kick. If we managed to unlock with no queued waiters, then
+ * we can clear the slowpath flag.
+ */
+ if (new.tickets.head != new.tickets.tail ||
+ cmpxchg(&lock->head_tail,
+ old.head_tail, new.head_tail) != old.head_tail) {
+ /* still people waiting */
+ __ticket_unlock_release(lock);
+ }
+
+ __ticket_unlock_kick(lock, new.tickets.head);
+}
+EXPORT_SYMBOL(__ticket_unlock_release_slowpath);
diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index c31c5a3..91f2fd2 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -119,6 +119,10 @@ static void xen_lock_spinning(struct arch_spinlock *lock, unsigned want)
/* Only check lock once pending cleared */
barrier();
+ /* Mark entry to slowpath before doing the pickup test to make
+ sure we don't deadlock with an unlocker. */
+ __ticket_enter_slowpath(lock);
+
/* check again make sure it didn't become free while
we weren't looking */
if (ACCESS_ONCE(lock->tickets.head) == want) {
--
1.7.2.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox