2004-11-22 16:12:48

by Roland Dreier

[permalink] [raw]
Subject: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

Add support for sending queries to the SA (Subnet Administration). In
particular the PathRecord and MCMember (multicast group member) used
by the IP-over-InfiniBand driver are implemented.

Signed-off-by: Roland Dreier <[email protected]>


Index: linux-bk/drivers/infiniband/core/Makefile
===================================================================
--- linux-bk.orig/drivers/infiniband/core/Makefile 2004-11-21 21:25:53.101323036 -0800
+++ linux-bk/drivers/infiniband/core/Makefile 2004-11-21 21:25:53.879207651 -0800
@@ -2,7 +2,8 @@

obj-$(CONFIG_INFINIBAND) += \
ib_core.o \
- ib_mad.o
+ ib_mad.o \
+ ib_sa.o

ib_core-objs := \
packer.o \
@@ -17,3 +18,5 @@
mad.o \
smi.o \
agent.o
+
+ib_sa-objs := sa_query.o
Index: linux-bk/drivers/infiniband/core/sa_query.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-bk/drivers/infiniband/core/sa_query.c 2004-11-21 21:25:53.928200384 -0800
@@ -0,0 +1,815 @@
+/*
+ * This software is available to you under a choice of one of two
+ * licenses. You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available at
+ * <http://www.fsf.org/copyleft/gpl.html>, or the OpenIB.org BSD
+ * license, available in the LICENSE.TXT file accompanying this
+ * software. These details are also available at
+ * <http://openib.org/license.html>.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Copyright (c) 2004 Topspin Communications. All rights reserved.
+ *
+ * $Id$
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/random.h>
+#include <linux/spinlock.h>
+#include <linux/slab.h>
+#include <linux/pci.h>
+#include <linux/kref.h>
+#include <linux/idr.h>
+
+#include <ib_pack.h>
+#include <ib_sa.h>
+
+MODULE_AUTHOR("Roland Dreier");
+MODULE_DESCRIPTION("InfiniBand subnet administration query support");
+MODULE_LICENSE("Dual BSD/GPL");
+
+struct ib_sa_hdr {
+ u64 sm_key;
+ u16 attr_offset;
+ u16 reserved;
+ ib_sa_comp_mask comp_mask;
+} __attribute__ ((packed));
+
+struct ib_sa_mad {
+ struct ib_mad_hdr mad_hdr;
+ struct ib_rmpp_hdr rmpp_hdr;
+ struct ib_sa_hdr sa_hdr;
+ u8 data[200];
+} __attribute__ ((packed));
+
+struct ib_sa_sm_ah {
+ struct ib_ah *ah;
+ struct kref ref;
+};
+
+struct ib_sa_port {
+ struct ib_mad_agent *agent;
+ struct ib_mr *mr;
+ struct ib_sa_sm_ah *sm_ah;
+ struct work_struct update_task;
+ spinlock_t ah_lock;
+ u8 port_num;
+};
+
+struct ib_sa_device {
+ int start_port, end_port;
+ struct ib_event_handler event_handler;
+ struct ib_sa_port port[0];
+};
+
+struct ib_sa_query {
+ void (*callback)(struct ib_sa_query *, int, struct ib_sa_mad *);
+ void (*release)(struct ib_sa_query *);
+ struct ib_sa_port *port;
+ struct ib_sa_mad *mad;
+ struct ib_sa_sm_ah *sm_ah;
+ DECLARE_PCI_UNMAP_ADDR(mapping)
+ int id;
+};
+
+struct ib_sa_path_query {
+ void (*callback)(int, struct ib_sa_path_rec *, void *);
+ void *context;
+ struct ib_sa_query sa_query;
+};
+
+struct ib_sa_mcmember_query {
+ void (*callback)(int, struct ib_sa_mcmember_rec *, void *);
+ void *context;
+ struct ib_sa_query sa_query;
+};
+
+static void ib_sa_add_one(struct ib_device *device);
+static void ib_sa_remove_one(struct ib_device *device);
+
+static struct ib_client sa_client = {
+ .name = "sa",
+ .add = ib_sa_add_one,
+ .remove = ib_sa_remove_one
+};
+
+static spinlock_t idr_lock;
+DEFINE_IDR(query_idr);
+
+static spinlock_t tid_lock;
+static u32 tid;
+
+enum {
+ IB_SA_ATTR_CLASS_PORTINFO = 0x01,
+ IB_SA_ATTR_NOTICE = 0x02,
+ IB_SA_ATTR_INFORM_INFO = 0x03,
+ IB_SA_ATTR_NODE_REC = 0x11,
+ IB_SA_ATTR_PORT_INFO_REC = 0x12,
+ IB_SA_ATTR_SL2VL_REC = 0x13,
+ IB_SA_ATTR_SWITCH_REC = 0x14,
+ IB_SA_ATTR_LINEAR_FDB_REC = 0x15,
+ IB_SA_ATTR_RANDOM_FDB_REC = 0x16,
+ IB_SA_ATTR_MCAST_FDB_REC = 0x17,
+ IB_SA_ATTR_SM_INFO_REC = 0x18,
+ IB_SA_ATTR_LINK_REC = 0x20,
+ IB_SA_ATTR_GUID_INFO_REC = 0x30,
+ IB_SA_ATTR_SERVICE_REC = 0x31,
+ IB_SA_ATTR_PARTITION_REC = 0x33,
+ IB_SA_ATTR_RANGE_REC = 0x34,
+ IB_SA_ATTR_PATH_REC = 0x35,
+ IB_SA_ATTR_VL_ARB_REC = 0x36,
+ IB_SA_ATTR_MC_GROUP_REC = 0x37,
+ IB_SA_ATTR_MC_MEMBER_REC = 0x38,
+ IB_SA_ATTR_TRACE_REC = 0x39,
+ IB_SA_ATTR_MULTI_PATH_REC = 0x3a,
+ IB_SA_ATTR_SERVICE_ASSOC_REC = 0x3b
+};
+
+#define PATH_REC_FIELD(field) \
+ .struct_offset_bytes = offsetof(struct ib_sa_path_rec, field), \
+ .struct_size_bytes = sizeof ((struct ib_sa_path_rec *) 0)->field, \
+ .field_name = "sa_path_rec:" #field
+
+static const struct ib_field path_rec_table[] = {
+ { RESERVED,
+ .offset_words = 0,
+ .offset_bits = 0,
+ .size_bits = 32 },
+ { RESERVED,
+ .offset_words = 1,
+ .offset_bits = 0,
+ .size_bits = 32 },
+ { PATH_REC_FIELD(dgid),
+ .offset_words = 2,
+ .offset_bits = 0,
+ .size_bits = 128 },
+ { PATH_REC_FIELD(sgid),
+ .offset_words = 6,
+ .offset_bits = 0,
+ .size_bits = 128 },
+ { PATH_REC_FIELD(dlid),
+ .offset_words = 10,
+ .offset_bits = 0,
+ .size_bits = 16 },
+ { PATH_REC_FIELD(slid),
+ .offset_words = 10,
+ .offset_bits = 16,
+ .size_bits = 16 },
+ { PATH_REC_FIELD(raw_traffic),
+ .offset_words = 11,
+ .offset_bits = 0,
+ .size_bits = 1 },
+ { RESERVED,
+ .offset_words = 11,
+ .offset_bits = 1,
+ .size_bits = 3 },
+ { PATH_REC_FIELD(flow_label),
+ .offset_words = 11,
+ .offset_bits = 4,
+ .size_bits = 20 },
+ { PATH_REC_FIELD(hop_limit),
+ .offset_words = 11,
+ .offset_bits = 24,
+ .size_bits = 8 },
+ { PATH_REC_FIELD(traffic_class),
+ .offset_words = 12,
+ .offset_bits = 0,
+ .size_bits = 8 },
+ { PATH_REC_FIELD(reversible),
+ .offset_words = 12,
+ .offset_bits = 8,
+ .size_bits = 1 },
+ { PATH_REC_FIELD(numb_path),
+ .offset_words = 12,
+ .offset_bits = 9,
+ .size_bits = 7 },
+ { PATH_REC_FIELD(pkey),
+ .offset_words = 12,
+ .offset_bits = 16,
+ .size_bits = 16 },
+ { RESERVED,
+ .offset_words = 13,
+ .offset_bits = 0,
+ .size_bits = 12 },
+ { PATH_REC_FIELD(sl),
+ .offset_words = 13,
+ .offset_bits = 12,
+ .size_bits = 4 },
+ { PATH_REC_FIELD(mtu_selector),
+ .offset_words = 13,
+ .offset_bits = 16,
+ .size_bits = 2 },
+ { PATH_REC_FIELD(mtu),
+ .offset_words = 13,
+ .offset_bits = 18,
+ .size_bits = 6 },
+ { PATH_REC_FIELD(rate_selector),
+ .offset_words = 13,
+ .offset_bits = 24,
+ .size_bits = 2 },
+ { PATH_REC_FIELD(rate),
+ .offset_words = 13,
+ .offset_bits = 26,
+ .size_bits = 6 },
+ { PATH_REC_FIELD(packet_life_time_selector),
+ .offset_words = 14,
+ .offset_bits = 0,
+ .size_bits = 2 },
+ { PATH_REC_FIELD(packet_life_time),
+ .offset_words = 14,
+ .offset_bits = 2,
+ .size_bits = 6 },
+ { PATH_REC_FIELD(preference),
+ .offset_words = 14,
+ .offset_bits = 8,
+ .size_bits = 8 },
+ { RESERVED,
+ .offset_words = 14,
+ .offset_bits = 16,
+ .size_bits = 48 },
+};
+
+#define MCMEMBER_REC_FIELD(field) \
+ .struct_offset_bytes = offsetof(struct ib_sa_mcmember_rec, field), \
+ .struct_size_bytes = sizeof ((struct ib_sa_mcmember_rec *) 0)->field, \
+ .field_name = "sa_mcmember_rec:" #field
+
+static const struct ib_field mcmember_rec_table[] = {
+ { MCMEMBER_REC_FIELD(mgid),
+ .offset_words = 0,
+ .offset_bits = 0,
+ .size_bits = 128 },
+ { MCMEMBER_REC_FIELD(port_gid),
+ .offset_words = 4,
+ .offset_bits = 0,
+ .size_bits = 128 },
+ { MCMEMBER_REC_FIELD(qkey),
+ .offset_words = 8,
+ .offset_bits = 0,
+ .size_bits = 32 },
+ { MCMEMBER_REC_FIELD(mlid),
+ .offset_words = 9,
+ .offset_bits = 0,
+ .size_bits = 16 },
+ { MCMEMBER_REC_FIELD(mtu_selector),
+ .offset_words = 9,
+ .offset_bits = 16,
+ .size_bits = 2 },
+ { MCMEMBER_REC_FIELD(mtu),
+ .offset_words = 9,
+ .offset_bits = 18,
+ .size_bits = 6 },
+ { MCMEMBER_REC_FIELD(traffic_class),
+ .offset_words = 9,
+ .offset_bits = 24,
+ .size_bits = 8 },
+ { MCMEMBER_REC_FIELD(pkey),
+ .offset_words = 10,
+ .offset_bits = 0,
+ .size_bits = 16 },
+ { MCMEMBER_REC_FIELD(rate_selector),
+ .offset_words = 10,
+ .offset_bits = 16,
+ .size_bits = 2 },
+ { MCMEMBER_REC_FIELD(rate),
+ .offset_words = 10,
+ .offset_bits = 18,
+ .size_bits = 6 },
+ { MCMEMBER_REC_FIELD(packet_life_time_selector),
+ .offset_words = 10,
+ .offset_bits = 24,
+ .size_bits = 2 },
+ { MCMEMBER_REC_FIELD(packet_life_time),
+ .offset_words = 10,
+ .offset_bits = 26,
+ .size_bits = 6 },
+ { MCMEMBER_REC_FIELD(sl),
+ .offset_words = 11,
+ .offset_bits = 0,
+ .size_bits = 4 },
+ { MCMEMBER_REC_FIELD(flow_label),
+ .offset_words = 11,
+ .offset_bits = 4,
+ .size_bits = 20 },
+ { MCMEMBER_REC_FIELD(hop_limit),
+ .offset_words = 11,
+ .offset_bits = 24,
+ .size_bits = 8 },
+ { MCMEMBER_REC_FIELD(scope),
+ .offset_words = 12,
+ .offset_bits = 0,
+ .size_bits = 4 },
+ { MCMEMBER_REC_FIELD(join_state),
+ .offset_words = 12,
+ .offset_bits = 4,
+ .size_bits = 4 },
+ { MCMEMBER_REC_FIELD(proxy_join),
+ .offset_words = 12,
+ .offset_bits = 8,
+ .size_bits = 1 },
+ { RESERVED,
+ .offset_words = 12,
+ .offset_bits = 9,
+ .size_bits = 23 },
+};
+
+static void free_sm_ah(struct kref *kref)
+{
+ struct ib_sa_sm_ah *sm_ah = container_of(kref, struct ib_sa_sm_ah, ref);
+
+ ib_destroy_ah(sm_ah->ah);
+ kfree(sm_ah);
+}
+
+static void update_sm_ah(void *port_ptr)
+{
+ struct ib_sa_port *port = port_ptr;
+ struct ib_sa_sm_ah *new_ah, *old_ah;
+ struct ib_port_attr port_attr;
+ struct ib_ah_attr ah_attr;
+
+ if (ib_query_port(port->agent->device, port->port_num, &port_attr)) {
+ printk(KERN_WARNING "Couldn't query port\n");
+ return;
+ }
+
+ new_ah = kmalloc(sizeof *new_ah, GFP_KERNEL);
+ if (!new_ah) {
+ printk(KERN_WARNING "Couldn't allocate new SM AH\n");
+ return;
+ }
+
+ kref_init(&new_ah->ref);
+
+ memset(&ah_attr, 0, sizeof ah_attr);
+ ah_attr.dlid = port_attr.sm_lid;
+ ah_attr.sl = port_attr.sm_sl;
+ ah_attr.port_num = port->port_num;
+
+ new_ah->ah = ib_create_ah(port->agent->qp->pd, &ah_attr);
+ if (IS_ERR(new_ah->ah)) {
+ printk(KERN_WARNING "Couldn't create new SM AH\n");
+ kfree(new_ah);
+ return;
+ }
+
+ spin_lock_irq(&port->ah_lock);
+ old_ah = port->sm_ah;
+ port->sm_ah = new_ah;
+ spin_unlock_irq(&port->ah_lock);
+
+ if (old_ah)
+ kref_put(&old_ah->ref, free_sm_ah);
+}
+
+static void ib_sa_event(struct ib_event_handler *handler, struct ib_event *event)
+{
+ if (event->event == IB_EVENT_PORT_ERR ||
+ event->event == IB_EVENT_PORT_ACTIVE ||
+ event->event == IB_EVENT_LID_CHANGE ||
+ event->event == IB_EVENT_PKEY_CHANGE ||
+ event->event == IB_EVENT_SM_CHANGE) {
+ struct ib_sa_device *sa_dev =
+ ib_get_client_data(event->device, &sa_client);
+
+ schedule_work(&sa_dev->port[event->element.port_num -
+ sa_dev->start_port].update_task);
+ }
+}
+
+void ib_sa_cancel_query(int id, struct ib_sa_query *query)
+{
+ unsigned long flags;
+ struct ib_mad_agent *agent;
+
+ spin_lock_irqsave(&idr_lock, flags);
+ if (idr_find(&query_idr, id) != query) {
+ spin_unlock_irqrestore(&idr_lock, flags);
+ return;
+ }
+ agent = query->port->agent;
+ spin_unlock_irqrestore(&idr_lock, flags);
+
+ ib_cancel_mad(agent, id);
+}
+EXPORT_SYMBOL(ib_sa_cancel_query);
+
+static void init_mad(struct ib_sa_mad *mad, struct ib_mad_agent *agent)
+{
+ unsigned long flags;
+
+ memset(mad, 0, sizeof *mad);
+
+ mad->mad_hdr.base_version = IB_MGMT_BASE_VERSION;
+ mad->mad_hdr.mgmt_class = IB_MGMT_CLASS_SUBN_ADM;
+ mad->mad_hdr.class_version = IB_SA_CLASS_VERSION;
+
+ spin_lock_irqsave(&tid_lock, flags);
+ mad->mad_hdr.tid =
+ cpu_to_be64(((u64) agent->hi_tid) << 32 | tid++);
+ spin_unlock_irqrestore(&tid_lock, flags);
+}
+
+static int send_mad(struct ib_sa_query *query, int timeout_ms)
+{
+ struct ib_sa_port *port = query->port;
+ unsigned long flags;
+ int ret;
+ struct ib_sge gather_list;
+ struct ib_send_wr *bad_wr, wr = {
+ .opcode = IB_WR_SEND,
+ .sg_list = &gather_list,
+ .num_sge = 1,
+ .send_flags = IB_SEND_SIGNALED,
+ .wr = {
+ .ud = {
+ .mad_hdr = &query->mad->mad_hdr,
+ .remote_qpn = 1,
+ .remote_qkey = IB_QP1_QKEY,
+ .timeout_ms = timeout_ms
+ }
+ }
+ };
+
+retry:
+ if (!idr_pre_get(&query_idr, GFP_ATOMIC))
+ return -ENOMEM;
+ spin_lock_irqsave(&idr_lock, flags);
+ ret = idr_get_new(&query_idr, query, &query->id);
+ spin_unlock_irqrestore(&idr_lock, flags);
+ if (ret == -EAGAIN)
+ goto retry;
+ if (ret)
+ return ret;
+
+ wr.wr_id = query->id;
+
+ spin_lock_irqsave(&port->ah_lock, flags);
+ kref_get(&port->sm_ah->ref);
+ query->sm_ah = port->sm_ah;
+ wr.wr.ud.ah = port->sm_ah->ah;
+ spin_unlock_irqrestore(&port->ah_lock, flags);
+
+ gather_list.addr = pci_map_single(port->agent->device->dma_device,
+ query->mad,
+ sizeof (struct ib_sa_mad),
+ PCI_DMA_TODEVICE);
+ gather_list.length = sizeof (struct ib_sa_mad);
+ gather_list.lkey = port->mr->lkey;
+ pci_unmap_addr_set(query, mapping, gather_list.addr);
+
+ ret = ib_post_send_mad(port->agent, &wr, &bad_wr);
+ if (ret) {
+ pci_unmap_single(port->agent->device->dma_device,
+ pci_unmap_addr(query, mapping),
+ sizeof (struct ib_sa_mad),
+ PCI_DMA_TODEVICE);
+ kref_put(&query->sm_ah->ref, free_sm_ah);
+ spin_lock_irqsave(&idr_lock, flags);
+ idr_remove(&query_idr, query->id);
+ spin_unlock_irqrestore(&idr_lock, flags);
+ }
+
+ return ret;
+}
+
+static void ib_sa_path_rec_callback(struct ib_sa_query *sa_query,
+ int status,
+ struct ib_sa_mad *mad)
+{
+ struct ib_sa_path_query *query =
+ container_of(sa_query, struct ib_sa_path_query, sa_query);
+
+ if (mad) {
+ struct ib_sa_path_rec rec;
+
+ ib_unpack(path_rec_table, ARRAY_SIZE(path_rec_table),
+ mad->data, &rec);
+ query->callback(status, &rec, query->context);
+ } else
+ query->callback(status, NULL, query->context);
+}
+
+static void ib_sa_path_rec_release(struct ib_sa_query *sa_query)
+{
+ kfree(sa_query->mad);
+ kfree(container_of(sa_query, struct ib_sa_path_query, sa_query));
+}
+
+int ib_sa_path_rec_get(struct ib_device *device, u8 port_num,
+ struct ib_sa_path_rec *rec,
+ ib_sa_comp_mask comp_mask,
+ int timeout_ms, int gfp_mask,
+ void (*callback)(int status,
+ struct ib_sa_path_rec *resp,
+ void *context),
+ void *context,
+ struct ib_sa_query **sa_query)
+{
+ struct ib_sa_path_query *query;
+ struct ib_sa_device *sa_dev = ib_get_client_data(device, &sa_client);
+ struct ib_sa_port *port = &sa_dev->port[port_num - sa_dev->start_port];
+ struct ib_mad_agent *agent = port->agent;
+ int ret;
+
+ query = kmalloc(sizeof *query, gfp_mask);
+ if (!query)
+ return -ENOMEM;
+ query->sa_query.mad = kmalloc(sizeof *query->sa_query.mad, gfp_mask);
+ if (!query->sa_query.mad) {
+ kfree(query);
+ return -ENOMEM;
+ }
+
+ query->callback = callback;
+ query->context = context;
+
+ init_mad(query->sa_query.mad, agent);
+
+ query->sa_query.callback = ib_sa_path_rec_callback;
+ query->sa_query.release = ib_sa_path_rec_release;
+ query->sa_query.port = port;
+ query->sa_query.mad->mad_hdr.method = IB_MGMT_METHOD_GET;
+ query->sa_query.mad->mad_hdr.attr_id = cpu_to_be16(IB_SA_ATTR_PATH_REC);
+ query->sa_query.mad->sa_hdr.comp_mask = comp_mask;
+
+ ib_pack(path_rec_table, ARRAY_SIZE(path_rec_table),
+ rec, query->sa_query.mad->data);
+
+ *sa_query = &query->sa_query;
+ ret = send_mad(&query->sa_query, timeout_ms);
+ if (ret) {
+ *sa_query = NULL;
+ kfree(query->sa_query.mad);
+ kfree(query);
+ }
+
+ return ret ? ret : query->sa_query.id;
+}
+EXPORT_SYMBOL(ib_sa_path_rec_get);
+
+static void ib_sa_mcmember_rec_callback(struct ib_sa_query *sa_query,
+ int status,
+ struct ib_sa_mad *mad)
+{
+ struct ib_sa_mcmember_query *query =
+ container_of(sa_query, struct ib_sa_mcmember_query, sa_query);
+
+ if (mad) {
+ struct ib_sa_mcmember_rec rec;
+
+ ib_unpack(mcmember_rec_table, ARRAY_SIZE(mcmember_rec_table),
+ mad->data, &rec);
+ query->callback(status, &rec, query->context);
+ } else
+ query->callback(status, NULL, query->context);
+}
+
+static void ib_sa_mcmember_rec_release(struct ib_sa_query *sa_query)
+{
+ kfree(sa_query->mad);
+ kfree(container_of(sa_query, struct ib_sa_mcmember_query, sa_query));
+}
+
+int ib_sa_mcmember_rec_query(struct ib_device *device, u8 port_num,
+ u8 method,
+ struct ib_sa_mcmember_rec *rec,
+ ib_sa_comp_mask comp_mask,
+ int timeout_ms, int gfp_mask,
+ void (*callback)(int status,
+ struct ib_sa_mcmember_rec *resp,
+ void *context),
+ void *context,
+ struct ib_sa_query **sa_query)
+{
+ struct ib_sa_mcmember_query *query;
+ struct ib_sa_device *sa_dev = ib_get_client_data(device, &sa_client);
+ struct ib_sa_port *port = &sa_dev->port[port_num - sa_dev->start_port];
+ struct ib_mad_agent *agent = port->agent;
+ int ret;
+
+ query = kmalloc(sizeof *query, gfp_mask);
+ if (!query)
+ return -ENOMEM;
+ query->sa_query.mad = kmalloc(sizeof *query->sa_query.mad, gfp_mask);
+ if (!query->sa_query.mad) {
+ kfree(query);
+ return -ENOMEM;
+ }
+
+ query->callback = callback;
+ query->context = context;
+
+ init_mad(query->sa_query.mad, agent);
+
+ query->sa_query.callback = ib_sa_mcmember_rec_callback;
+ query->sa_query.release = ib_sa_mcmember_rec_release;
+ query->sa_query.port = port;
+ query->sa_query.mad->mad_hdr.method = method;
+ query->sa_query.mad->mad_hdr.attr_id = cpu_to_be16(IB_SA_ATTR_MC_MEMBER_REC);
+ query->sa_query.mad->sa_hdr.comp_mask = comp_mask;
+
+ ib_pack(mcmember_rec_table, ARRAY_SIZE(mcmember_rec_table),
+ rec, query->sa_query.mad->data);
+
+ *sa_query = &query->sa_query;
+ ret = send_mad(&query->sa_query, timeout_ms);
+ if (ret) {
+ *sa_query = NULL;
+ kfree(query->sa_query.mad);
+ kfree(query);
+ }
+
+ return ret ? ret : query->sa_query.id;
+}
+EXPORT_SYMBOL(ib_sa_mcmember_rec_query);
+
+static void send_handler(struct ib_mad_agent *agent,
+ struct ib_mad_send_wc *mad_send_wc)
+{
+ struct ib_sa_query *query;
+ unsigned long flags;
+
+ spin_lock_irqsave(&idr_lock, flags);
+ query = idr_find(&query_idr, mad_send_wc->wr_id);
+ spin_unlock_irqrestore(&idr_lock, flags);
+
+ if (!query)
+ return;
+
+ switch (mad_send_wc->status) {
+ case IB_WC_SUCCESS:
+ /* No callback -- already got recv */
+ break;
+ case IB_WC_RESP_TIMEOUT_ERR:
+ query->callback(query, -ETIMEDOUT, NULL);
+ break;
+ case IB_WC_WR_FLUSH_ERR:
+ query->callback(query, -EINTR, NULL);
+ break;
+ default:
+ query->callback(query, -EIO, NULL);
+ break;
+ }
+
+ pci_unmap_single(agent->device->dma_device,
+ pci_unmap_addr(query, mapping),
+ sizeof (struct ib_sa_mad),
+ PCI_DMA_TODEVICE);
+ kref_put(&query->sm_ah->ref, free_sm_ah);
+
+ query->release(query);
+
+ spin_lock_irqsave(&idr_lock, flags);
+ idr_remove(&query_idr, mad_send_wc->wr_id);
+ spin_unlock_irqrestore(&idr_lock, flags);
+}
+
+static void recv_handler(struct ib_mad_agent *mad_agent,
+ struct ib_mad_recv_wc *mad_recv_wc)
+{
+ struct ib_sa_query *query;
+ unsigned long flags;
+
+ spin_lock_irqsave(&idr_lock, flags);
+ query = idr_find(&query_idr, mad_recv_wc->wc->wr_id);
+ spin_unlock_irqrestore(&idr_lock, flags);
+
+ if (query) {
+ if (mad_recv_wc->wc->status == IB_WC_SUCCESS)
+ query->callback(query,
+ mad_recv_wc->recv_buf->mad->mad_hdr.status ?
+ -EINVAL : 0,
+ (struct ib_sa_mad *) mad_recv_wc->recv_buf->mad);
+ else
+ query->callback(query, -EIO, NULL);
+ }
+
+ ib_free_recv_mad(mad_recv_wc);
+}
+
+static void ib_sa_add_one(struct ib_device *device)
+{
+ struct ib_sa_device *sa_dev;
+ int s, e, i;
+
+ if (device->node_type == IB_NODE_SWITCH)
+ s = e = 0;
+ else {
+ s = 1;
+ e = device->phys_port_cnt;
+ }
+
+ sa_dev = kmalloc(sizeof *sa_dev +
+ (e - s + 1) * sizeof (struct ib_sa_port),
+ GFP_KERNEL);
+ if (!sa_dev)
+ return;
+
+ sa_dev->start_port = s;
+ sa_dev->end_port = e;
+
+ for (i = 0; i <= e - s; ++i) {
+ sa_dev->port[i].mr = NULL;
+ sa_dev->port[i].sm_ah = NULL;
+ sa_dev->port[i].port_num = i + s;
+ spin_lock_init(&sa_dev->port[i].ah_lock);
+
+ sa_dev->port[i].agent =
+ ib_register_mad_agent(device, i + s, IB_QPT_GSI,
+ NULL, 0, send_handler,
+ recv_handler, sa_dev);
+ if (IS_ERR(sa_dev->port[i].agent))
+ goto err;
+
+ sa_dev->port[i].mr = ib_get_dma_mr(sa_dev->port[i].agent->qp->pd,
+ IB_ACCESS_LOCAL_WRITE);
+ if (IS_ERR(sa_dev->port[i].mr)) {
+ ib_unregister_mad_agent(sa_dev->port[i].agent);
+ goto err;
+ }
+
+ INIT_WORK(&sa_dev->port[i].update_task,
+ update_sm_ah, &sa_dev->port[i]);
+ }
+
+ /*
+ * We register our event handler after everything is set up,
+ * and then update our cached info after the event handler is
+ * registered to avoid any problems if a port changes state
+ * during our initialization.
+ */
+
+ INIT_IB_EVENT_HANDLER(&sa_dev->event_handler, device, ib_sa_event);
+ if (ib_register_event_handler(&sa_dev->event_handler))
+ goto err;
+
+ for (i = 0; i <= e - s; ++i)
+ update_sm_ah(&sa_dev->port[i]);
+
+ ib_set_client_data(device, &sa_client, sa_dev);
+
+ return;
+
+err:
+ while (--i >= 0) {
+ ib_dereg_mr(sa_dev->port[i].mr);
+ ib_unregister_mad_agent(sa_dev->port[i].agent);
+ }
+
+ kfree(sa_dev);
+
+ return;
+}
+
+static void ib_sa_remove_one(struct ib_device *device)
+{
+ struct ib_sa_device *sa_dev = ib_get_client_data(device, &sa_client);
+ int i;
+
+ if (!sa_dev)
+ return;
+
+ ib_unregister_event_handler(&sa_dev->event_handler);
+
+ for (i = 0; i <= sa_dev->end_port - sa_dev->start_port; ++i) {
+ ib_unregister_mad_agent(sa_dev->port[i].agent);
+ kref_put(&sa_dev->port[i].sm_ah->ref, free_sm_ah);
+ }
+
+ kfree(sa_dev);
+}
+
+static int __init ib_sa_init(void)
+{
+ int ret;
+
+ spin_lock_init(&idr_lock);
+ spin_lock_init(&tid_lock);
+
+ get_random_bytes(&tid, sizeof tid);
+
+ ret = ib_register_client(&sa_client);
+ if (ret)
+ printk(KERN_ERR "Couldn't register ib_sa client\n");
+
+ return ret;
+}
+
+static void __exit ib_sa_cleanup(void)
+{
+ ib_unregister_client(&sa_client);
+}
+
+module_init(ib_sa_init);
+module_exit(ib_sa_cleanup);
Index: linux-bk/drivers/infiniband/include/ib_sa.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-bk/drivers/infiniband/include/ib_sa.h 2004-11-21 21:25:53.970194155 -0800
@@ -0,0 +1,221 @@
+/*
+ * This software is available to you under a choice of one of two
+ * licenses. You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available at
+ * <http://www.fsf.org/copyleft/gpl.html>, or the OpenIB.org BSD
+ * license, available in the LICENSE.TXT file accompanying this
+ * software. These details are also available at
+ * <http://openib.org/license.html>.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Copyright (c) 2004 Topspin Communications. All rights reserved.
+ *
+ * $Id$
+ */
+
+#ifndef IB_SA_H
+#define IB_SA_H
+
+#include <linux/compiler.h>
+
+#include <ib_verbs.h>
+#include <ib_mad.h>
+
+enum {
+ IB_SA_CLASS_VERSION = 2, /* IB spec version 1.1/1.2 */
+
+ IB_SA_METHOD_DELETE = 0x15
+};
+
+enum ib_sa_selector {
+ IB_SA_GTE = 0,
+ IB_SA_LTE = 1,
+ IB_SA_EQ = 2,
+ /*
+ * The meaning of "best" depends on the attribute: for
+ * example, for MTU best will return the largest available
+ * MTU, while for packet life time, best will return the
+ * smallest available life time.
+ */
+ IB_SA_BEST = 3
+};
+
+typedef u64 __bitwise ib_sa_comp_mask;
+
+#define IB_SA_COMP_MASK(n) ((__force ib_sa_comp_mask) cpu_to_be64(1ull << n))
+
+/*
+ * Structures for SA records are named "struct ib_sa_xxx_rec." No
+ * attempt is made to pack structures to match the physical layout of
+ * SA records in SA MADs; all packing and unpacking is handled by the
+ * SA query code.
+ *
+ * For a record with structure ib_sa_xxx_rec, the naming convention
+ * for the component mask value for field yyy is IB_SA_XXX_REC_YYY (we
+ * never use different abbreviations or otherwise change the spelling
+ * of xxx/yyy between ib_sa_xxx_rec.yyy and IB_SA_XXX_REC_YYY).
+ *
+ * Reserved rows are indicated with comments to help maintainability.
+ */
+
+/* reserved: 0 */
+/* reserved: 1 */
+#define IB_SA_PATH_REC_DGID IB_SA_COMP_MASK( 2)
+#define IB_SA_PATH_REC_SGID IB_SA_COMP_MASK( 3)
+#define IB_SA_PATH_REC_DLID IB_SA_COMP_MASK( 4)
+#define IB_SA_PATH_REC_SLID IB_SA_COMP_MASK( 5)
+#define IB_SA_PATH_REC_RAW_TRAFFIC IB_SA_COMP_MASK( 6)
+/* reserved: 7 */
+#define IB_SA_PATH_REC_FLOW_LABEL IB_SA_COMP_MASK( 8)
+#define IB_SA_PATH_REC_HOP_LIMIT IB_SA_COMP_MASK( 9)
+#define IB_SA_PATH_REC_TRAFFIC_CLASS IB_SA_COMP_MASK(10)
+#define IB_SA_PATH_REC_REVERSIBLE IB_SA_COMP_MASK(11)
+#define IB_SA_PATH_REC_NUMB_PATH IB_SA_COMP_MASK(12)
+#define IB_SA_PATH_REC_PKEY IB_SA_COMP_MASK(13)
+/* reserved: 14 */
+#define IB_SA_PATH_REC_SL IB_SA_COMP_MASK(15)
+#define IB_SA_PATH_REC_MTU_SELECTOR IB_SA_COMP_MASK(16)
+#define IB_SA_PATH_REC_MTU IB_SA_COMP_MASK(17)
+#define IB_SA_PATH_REC_RATE_SELECTOR IB_SA_COMP_MASK(18)
+#define IB_SA_PATH_REC_RATE IB_SA_COMP_MASK(19)
+#define IB_SA_PATH_REC_PACKET_LIFE_TIME_SELECTOR IB_SA_COMP_MASK(20)
+#define IB_SA_PATH_REC_PACKET_LIFE_TIME IB_SA_COMP_MASK(21)
+#define IB_SA_PATH_REC_PREFERENCE IB_SA_COMP_MASK(22)
+
+struct ib_sa_path_rec {
+ /* reserved */
+ /* reserved */
+ union ib_gid dgid;
+ union ib_gid sgid;
+ u16 dlid;
+ u16 slid;
+ int raw_traffic;
+ /* reserved */
+ u32 flow_label;
+ u8 hop_limit;
+ u8 traffic_class;
+ int reversible;
+ u8 numb_path;
+ u16 pkey;
+ /* reserved */
+ u8 sl;
+ u8 mtu_selector;
+ enum ib_mtu mtu;
+ u8 rate_selector;
+ u8 rate;
+ u8 packet_life_time_selector;
+ u8 packet_life_time;
+ u8 preference;
+};
+
+#define IB_SA_MCMEMBER_REC_MGID IB_SA_COMP_MASK( 0)
+#define IB_SA_MCMEMBER_REC_PORT_GID IB_SA_COMP_MASK( 1)
+#define IB_SA_MCMEMBER_REC_QKEY IB_SA_COMP_MASK( 2)
+#define IB_SA_MCMEMBER_REC_MLID IB_SA_COMP_MASK( 3)
+#define IB_SA_MCMEMBER_REC_MTU_SELECTOR IB_SA_COMP_MASK( 4)
+#define IB_SA_MCMEMBER_REC_MTU IB_SA_COMP_MASK( 5)
+#define IB_SA_MCMEMBER_REC_TRAFFIC_CLASS IB_SA_COMP_MASK( 6)
+#define IB_SA_MCMEMBER_REC_PKEY IB_SA_COMP_MASK( 7)
+#define IB_SA_MCMEMBER_REC_RATE_SELECTOR IB_SA_COMP_MASK( 8)
+#define IB_SA_MCMEMBER_REC_RATE IB_SA_COMP_MASK( 9)
+#define IB_SA_MCMEMBER_REC_PACKET_LIFE_TIME_SELECTOR IB_SA_COMP_MASK(10)
+#define IB_SA_MCMEMBER_REC_PACKET_LIFE_TIME IB_SA_COMP_MASK(11)
+#define IB_SA_MCMEMBER_REC_SL IB_SA_COMP_MASK(12)
+#define IB_SA_MCMEMBER_REC_FLOW_LABEL IB_SA_COMP_MASK(13)
+#define IB_SA_MCMEMBER_REC_HOP_LIMIT IB_SA_COMP_MASK(14)
+#define IB_SA_MCMEMBER_REC_SCOPE IB_SA_COMP_MASK(15)
+#define IB_SA_MCMEMBER_REC_JOIN_STATE IB_SA_COMP_MASK(16)
+#define IB_SA_MCMEMBER_REC_PROXY_JOIN IB_SA_COMP_MASK(17)
+
+struct ib_sa_mcmember_rec {
+ union ib_gid mgid;
+ union ib_gid port_gid;
+ u32 qkey;
+ u16 mlid;
+ u8 mtu_selector;
+ enum ib_mtu mtu;
+ u8 traffic_class;
+ u16 pkey;
+ u8 rate_selector;
+ u8 rate;
+ u8 packet_life_time_selector;
+ u8 packet_life_time;
+ u8 sl;
+ u32 flow_label;
+ u8 hop_limit;
+ u8 scope;
+ u8 join_state;
+ int proxy_join;
+};
+
+struct ib_sa_query;
+
+void ib_sa_cancel_query(int id, struct ib_sa_query *query);
+
+int ib_sa_path_rec_get(struct ib_device *device, u8 port_num,
+ struct ib_sa_path_rec *rec,
+ ib_sa_comp_mask comp_mask,
+ int timeout_ms, int gfp_mask,
+ void (*callback)(int status,
+ struct ib_sa_path_rec *resp,
+ void *context),
+ void *context,
+ struct ib_sa_query **query);
+
+int ib_sa_mcmember_rec_query(struct ib_device *device, u8 port_num,
+ u8 method,
+ struct ib_sa_mcmember_rec *rec,
+ ib_sa_comp_mask comp_mask,
+ int timeout_ms, int gfp_mask,
+ void (*callback)(int status,
+ struct ib_sa_mcmember_rec *resp,
+ void *context),
+ void *context,
+ struct ib_sa_query **query);
+
+static inline int
+ib_sa_mcmember_rec_set(struct ib_device *device, u8 port_num,
+ struct ib_sa_mcmember_rec *rec,
+ ib_sa_comp_mask comp_mask,
+ int timeout_ms, int gfp_mask,
+ void (*callback)(int status,
+ struct ib_sa_mcmember_rec *resp,
+ void *context),
+ void *context,
+ struct ib_sa_query **query)
+{
+ return ib_sa_mcmember_rec_query(device, port_num,
+ IB_MGMT_METHOD_SET,
+ rec, comp_mask,
+ timeout_ms, gfp_mask, callback,
+ context, query);
+}
+
+static inline int
+ib_sa_mcmember_rec_delete(struct ib_device *device, u8 port_num,
+ struct ib_sa_mcmember_rec *rec,
+ ib_sa_comp_mask comp_mask,
+ int timeout_ms, int gfp_mask,
+ void (*callback)(int status,
+ struct ib_sa_mcmember_rec *resp,
+ void *context),
+ void *context,
+ struct ib_sa_query **query)
+{
+ return ib_sa_mcmember_rec_query(device, port_num,
+ IB_SA_METHOD_DELETE,
+ rec, comp_mask,
+ timeout_ms, gfp_mask, callback,
+ context, query);
+}
+
+
+#endif /* IB_SA_H */


2004-11-22 19:45:30

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

Nitpicking.

Sam

> --- linux-bk.orig/drivers/infiniband/core/Makefile 2004-11-21 21:25:53.101323036 -0800
> +++ linux-bk/drivers/infiniband/core/Makefile 2004-11-21 21:25:53.879207651 -0800
> @@ -2,7 +2,8 @@
>
> obj-$(CONFIG_INFINIBAND) += \
> ib_core.o \
> - ib_mad.o
> + ib_mad.o \
> + ib_sa.o
It's more readable to keep .o files on one line.


>
> ib_core-objs := \
> packer.o \

For new stuff please use ib_core-y :=

> @@ -17,3 +18,5 @@
> mad.o \
> smi.o \
> agent.o
> +
> +ib_sa-objs := sa_query.o
ib_sa-y := please.

> +#include <linux/idr.h>
> +
> +#include <ib_pack.h>
> +#include <ib_sa.h>
If they are in same dir as .c file use:
#include "ib_pack.h"
#include "ib_sa.h"

> Index: linux-bk/drivers/infiniband/include/ib_sa.h

.h files for a subsystem like this ought to be placed in include/infiniband
if they will be used by files in other directories than drivers/infiniband


2004-11-22 22:30:28

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

On Mon, Nov 22, 2004 at 07:13:48AM -0800, Roland Dreier wrote:
>
> Index: linux-bk/drivers/infiniband/core/Makefile
> ===================================================================

Please hack your submit script to not add these headers, when importing
to bk they end up showing up in the change log comments :(

> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-bk/drivers/infiniband/core/sa_query.c 2004-11-21 21:25:53.928200384 -0800
> @@ -0,0 +1,815 @@
> +/*
> + * This software is available to you under a choice of one of two
> + * licenses. You may choose to be licensed under the terms of the GNU
> + * General Public License (GPL) Version 2, available at
> + * <http://www.fsf.org/copyleft/gpl.html>, or the OpenIB.org BSD
> + * license, available in the LICENSE.TXT file accompanying this
> + * software. These details are also available at
> + * <http://openib.org/license.html>.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + *
> + * Copyright (c) 2004 Topspin Communications. All rights reserved.

No email address of who to bug with issues?

> + *
> + * $Id$

Not needed :)

> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/err.h>
> +#include <linux/random.h>
> +#include <linux/spinlock.h>
> +#include <linux/slab.h>
> +#include <linux/pci.h>
> +#include <linux/kref.h>
> +#include <linux/idr.h>
> +
> +#include <ib_pack.h>
> +#include <ib_sa.h>
> +
> +MODULE_AUTHOR("Roland Dreier");
> +MODULE_DESCRIPTION("InfiniBand subnet administration query support");
> +MODULE_LICENSE("Dual BSD/GPL");
> +
> +struct ib_sa_hdr {
> + u64 sm_key;
> + u16 attr_offset;
> + u16 reserved;
> + ib_sa_comp_mask comp_mask;
> +} __attribute__ ((packed));

Why is this packed?

> +struct ib_sa_mad {
> + struct ib_mad_hdr mad_hdr;
> + struct ib_rmpp_hdr rmpp_hdr;
> + struct ib_sa_hdr sa_hdr;
> + u8 data[200];
> +} __attribute__ ((packed));

Same here?

> +
> +struct ib_sa_sm_ah {
> + struct ib_ah *ah;
> + struct kref ref;
> +};
> +
> +struct ib_sa_port {
> + struct ib_mad_agent *agent;
> + struct ib_mr *mr;
> + struct ib_sa_sm_ah *sm_ah;
> + struct work_struct update_task;
> + spinlock_t ah_lock;
> + u8 port_num;
> +};
> +
> +struct ib_sa_device {
> + int start_port, end_port;
> + struct ib_event_handler event_handler;
> + struct ib_sa_port port[0];
> +};
> +
> +struct ib_sa_query {
> + void (*callback)(struct ib_sa_query *, int, struct ib_sa_mad *);
> + void (*release)(struct ib_sa_query *);
> + struct ib_sa_port *port;
> + struct ib_sa_mad *mad;
> + struct ib_sa_sm_ah *sm_ah;
> + DECLARE_PCI_UNMAP_ADDR(mapping)
> + int id;
> +};
> +
> +struct ib_sa_path_query {
> + void (*callback)(int, struct ib_sa_path_rec *, void *);
> + void *context;
> + struct ib_sa_query sa_query;
> +};
> +
> +struct ib_sa_mcmember_query {
> + void (*callback)(int, struct ib_sa_mcmember_rec *, void *);
> + void *context;
> + struct ib_sa_query sa_query;
> +};
> +
> +static void ib_sa_add_one(struct ib_device *device);
> +static void ib_sa_remove_one(struct ib_device *device);
> +
> +static struct ib_client sa_client = {
> + .name = "sa",
> + .add = ib_sa_add_one,
> + .remove = ib_sa_remove_one
> +};
> +
> +static spinlock_t idr_lock;
> +DEFINE_IDR(query_idr);

Should this be global or static?

> +
> +static spinlock_t tid_lock;
> +static u32 tid;
> +
> +enum {
> + IB_SA_ATTR_CLASS_PORTINFO = 0x01,
> + IB_SA_ATTR_NOTICE = 0x02,
> + IB_SA_ATTR_INFORM_INFO = 0x03,
> + IB_SA_ATTR_NODE_REC = 0x11,
> + IB_SA_ATTR_PORT_INFO_REC = 0x12,
> + IB_SA_ATTR_SL2VL_REC = 0x13,
> + IB_SA_ATTR_SWITCH_REC = 0x14,
> + IB_SA_ATTR_LINEAR_FDB_REC = 0x15,
> + IB_SA_ATTR_RANDOM_FDB_REC = 0x16,
> + IB_SA_ATTR_MCAST_FDB_REC = 0x17,
> + IB_SA_ATTR_SM_INFO_REC = 0x18,
> + IB_SA_ATTR_LINK_REC = 0x20,
> + IB_SA_ATTR_GUID_INFO_REC = 0x30,
> + IB_SA_ATTR_SERVICE_REC = 0x31,
> + IB_SA_ATTR_PARTITION_REC = 0x33,
> + IB_SA_ATTR_RANGE_REC = 0x34,
> + IB_SA_ATTR_PATH_REC = 0x35,
> + IB_SA_ATTR_VL_ARB_REC = 0x36,
> + IB_SA_ATTR_MC_GROUP_REC = 0x37,
> + IB_SA_ATTR_MC_MEMBER_REC = 0x38,
> + IB_SA_ATTR_TRACE_REC = 0x39,
> + IB_SA_ATTR_MULTI_PATH_REC = 0x3a,
> + IB_SA_ATTR_SERVICE_ASSOC_REC = 0x3b
> +};

Oops, tabs vs. spaces.

Care to use the __bitwise field here so that you can have sparse check
to see that you are actually using the proper enum values in all places?
See the kobject_action code for an example of this.

> +
> +#define PATH_REC_FIELD(field) \
> + .struct_offset_bytes = offsetof(struct ib_sa_path_rec, field), \
> + .struct_size_bytes = sizeof ((struct ib_sa_path_rec *) 0)->field, \
> + .field_name = "sa_path_rec:" #field
> +
> +static const struct ib_field path_rec_table[] = {
> + { RESERVED,
> + .offset_words = 0,
> + .offset_bits = 0,
> + .size_bits = 32 },

What is "RESERVED"? I must be missing a previous patch somewhere, I
currently don't see all of the series yet.

thanks,

greg k-h

2004-11-22 22:45:18

by Fab Tillier

[permalink] [raw]
Subject: RE: [openib-general] Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA(Subnet Administration) query support

> From: Greg KH [mailto:[email protected]]
> Sent: Monday, November 22, 2004 2:25 PM
>
> > +struct ib_sa_hdr {
> > + u64 sm_key;
> > + u16 attr_offset;
> > + u16 reserved;
> > + ib_sa_comp_mask comp_mask;
> > +} __attribute__ ((packed));
>
> Why is this packed?
>
> > +struct ib_sa_mad {
> > + struct ib_mad_hdr mad_hdr;
> > + struct ib_rmpp_hdr rmpp_hdr;
> > + struct ib_sa_hdr sa_hdr;
> > + u8 data[200];
> > +} __attribute__ ((packed));
>
> Same here?

These describe on-the-wire IB structures, and their definition matches the
IB spec (Version 1.1, Volume 1)

struct ib_mad_hdr matches "Standard MAD Header", Figure 144
struct ib_rmpp_hdr matches "RMPP MAD Header", Figure 168
struct ib_sa_hdr and struct ib_sa_mad match "SA Header", Figure 193

Hope that answers your question - let us know if it doesn't.

Cheers,

- Fab

2004-11-22 23:45:14

by Roland Dreier

[permalink] [raw]
Subject: Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

Greg> Please hack your submit script to not add these headers,
Greg> when importing to bk they end up showing up in the change
Greg> log comments :(

OK, will do.

Greg> No email address of who to bug with issues?

There's a patch to MAINTAINERS...

Greg> Why is this packed?
Greg> Same here?

Both of these structures unfortunately have 64 bit fields only aligned
to 32 bits (and are sent on the wire so we can't fiddle with the
layout). So without the "packed" they won't come out right on 64-bit archs.

Greg> Should this be global or static?

static, fixed.

Greg> Oops, tabs vs. spaces.

fixed.

Greg> Care to use the __bitwise field here so that you can have
Greg> sparse check to see that you are actually using the proper
Greg> enum values in all places? See the kobject_action code for
Greg> an example of this.

Sure, that's a good idea. I'll look for other places we can do this too.

Greg> What is "RESERVED"? I must be missing a previous patch
Greg> somewhere, I currently don't see all of the series yet.

It's in part 1/12: http://article.gmane.org/gmane.linux.kernel/257531
unfortunately some people marked it as spam and it didn't get
everywhere.

- Roland

2004-11-22 21:41:00

by Roland Dreier

[permalink] [raw]
Subject: Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

Sam> Nitpicking.

Great, thanks for the help :) I'll fix these up before our next
version of the patches are posted.

Sam> It's more readable to keep .o files on one line.

OK, I will reformat our Makefiles. (I used the old style because it's
easier to add/remove source files, but I think you're right that it's
better to optimize for readability rather than the rare event of
adding/removing sources)

Sam> For new stuff please use ib_core-y :=

OK, no problem (until a few days ago I didn't even know -y was
equivalent to -obj, let alone preferred).

Sam> .h files for a subsystem like this ought to be placed in
Sam> include/infiniband if they will be used by files in other
Sam> directories than drivers/infiniband

Right now all the code is in drivers/infiniband. However Christoph
suggested moving the .h files to include/infiniband as well. I have
no problem moving the includes (and as you point out this eliminates
having to add a -I to our CFLAGS), but on the other hand do we want to
add a new toplevel include directory for what is still admittedly a
minor subsystem?

Thanks,
Roland

2004-11-23 06:42:29

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

On Mon, Nov 22, 2004 at 03:34:23PM -0800, Roland Dreier wrote:
> Greg> No email address of who to bug with issues?
>
> There's a patch to MAINTAINERS...

Yeah, but a name in each file is much nicer.

> Greg> What is "RESERVED"? I must be missing a previous patch
> Greg> somewhere, I currently don't see all of the series yet.
>
> It's in part 1/12: http://article.gmane.org/gmane.linux.kernel/257531
> unfortunately some people marked it as spam and it didn't get
> everywhere.

Thanks for pointing this out.

One comment, the file drivers/infiniband/core/cache.c has a license that
is illegal due to the contents of the file. Please change the license
of the file to GPL only.

Oh, and how about kernel-doc comments for all functions that are
EXPORT_SYMBOL() marked? And for your core big structures?

thanks,

greg k-h

2004-11-23 06:50:29

by Roland Dreier

[permalink] [raw]
Subject: Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

Greg> Yeah, but a name in each file is much nicer.

Very little of the kernel seems to follow this rule right now.

Greg> One comment, the file drivers/infiniband/core/cache.c has a
Greg> license that is illegal due to the contents of the file.
Greg> Please change the license of the file to GPL only.

?? Can you explain this? What makes that file special?

Greg> Oh, and how about kernel-doc comments for all functions that
Greg> are EXPORT_SYMBOL() marked? And for your core big
Greg> structures?

I guess we'll start working on it...

- Roland

2004-11-23 07:30:06

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

On Mon, Nov 22, 2004 at 10:47:29PM -0800, Roland Dreier wrote:
> Greg> Yeah, but a name in each file is much nicer.
>
> Very little of the kernel seems to follow this rule right now.

I agree, but it's good to add this for new files.

> Greg> One comment, the file drivers/infiniband/core/cache.c has a
> Greg> license that is illegal due to the contents of the file.
> Greg> Please change the license of the file to GPL only.
>
> ?? Can you explain this? What makes that file special?

You are using a specific data structure that is only licensed to be used
in GPL code. By using it in code that has a non-GPL license (like the
dual license you have) you are violating the license of that code, and
open yourself up to lawsuits by the holder of that code.

There, can I be vague enough? :)

To be straightforward, either drop the RCU code completely, or change
the license of your code.

Hm, because of the fact that you are linking in GPL only code into this
code (because of the .h files you are using) how could you ever expect
to use a BSD-like license for this collected work?

Aren't licenses fun...

thanks,

greg k-h

2004-11-23 16:04:31

by Randy.Dunlap

[permalink] [raw]
Subject: Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

Greg KH wrote:
> On Mon, Nov 22, 2004 at 03:34:23PM -0800, Roland Dreier wrote:
>
>> Greg> No email address of who to bug with issues?
>>
>>There's a patch to MAINTAINERS...
>
>
> Yeah, but a name in each file is much nicer.

I disagree. I'd rather be able to look in MAINTAINERS | CREDITS
for all such references.


--
~Randy

2004-11-23 17:55:16

by Johannes Erdfelt

[permalink] [raw]
Subject: Re: [openib-general] Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

On Mon, Nov 22, 2004, Greg KH <[email protected]> wrote:
> On Mon, Nov 22, 2004 at 10:47:29PM -0800, Roland Dreier wrote:
> > Greg> One comment, the file drivers/infiniband/core/cache.c has a
> > Greg> license that is illegal due to the contents of the file.
> > Greg> Please change the license of the file to GPL only.
> >
> > ?? Can you explain this? What makes that file special?
>
> You are using a specific data structure that is only licensed to be used
> in GPL code. By using it in code that has a non-GPL license (like the
> dual license you have) you are violating the license of that code, and
> open yourself up to lawsuits by the holder of that code.

I don't understand this. You seem to be assuming that everyone who
compiles this will be compiling it with the GPL version of the RCU code.
It seems to me that only the resulting object file must be licensed
under the GPL because of the fact it uses other GPL-only code (the RCU
code in the kernel)

As a standalone piece of code, wouldn't it be freely licensed however
the author wishes?

> There, can I be vague enough? :)

Maybe it's the fact that you're being vague that is leading to my
confusion.

> To be straightforward, either drop the RCU code completely, or change
> the license of your code.

Or compile against non-GPL RCU code, right?

> Hm, because of the fact that you are linking in GPL only code into this
> code (because of the .h files you are using) how could you ever expect
> to use a BSD-like license for this collected work?
>
> Aren't licenses fun...

I don't mean to be nitpicking here, but I'm trying to figure out why you
interpreted the GPL like you did. I'd be venturing to guess that pretty
much everyone will be compiling against the GPL-only RCU code.

Just because the possibility exists that code can be compiled against
GPL-only code, doesn't necessarily mean that it too requires a GPL-only
license.

It seems like pretty much any code would fall into that category.

JE

2004-11-23 18:48:28

by Greg KH

[permalink] [raw]
Subject: Re: [openib-general] Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

On Tue, Nov 23, 2004 at 09:52:46AM -0800, Johannes Erdfelt wrote:
> On Mon, Nov 22, 2004, Greg KH <[email protected]> wrote:
> > To be straightforward, either drop the RCU code completely, or change
> > the license of your code.
>
> Or compile against non-GPL RCU code, right?

No. RCU is covered by a patent that only allows for it to be
implemented in GPL licensed code. If you want to use RCU in non-GPL
code, you need to sign a license agreement with the holder of the RCU
patent.

This was all covered a few years ago when the RCU code first went into
the kernel tree. See the lkml archives for details.

So the very usage of RCU in the code is the issue here, not the fact
that it is being linked against a GPL licensed header file.

This isn't a GPL interpretation here, but a patent license agreement
issue. Sorry for not being clearer the first time around, I thought
everyone was aware of this issue by now.

thanks,

greg k-h

2004-11-23 19:34:53

by linux-os

[permalink] [raw]
Subject: Re: [openib-general] Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

On Tue, 23 Nov 2004, Greg KH wrote:

> On Tue, Nov 23, 2004 at 09:52:46AM -0800, Johannes Erdfelt wrote:
>> On Mon, Nov 22, 2004, Greg KH <[email protected]> wrote:
>>> To be straightforward, either drop the RCU code completely, or change
>>> the license of your code.
>>
>> Or compile against non-GPL RCU code, right?
>
> No. RCU is covered by a patent that only allows for it to be
> implemented in GPL licensed code. If you want to use RCU in non-GPL
> code, you need to sign a license agreement with the holder of the RCU
> patent.
>
> This was all covered a few years ago when the RCU code first went into
> the kernel tree. See the lkml archives for details.
>
> So the very usage of RCU in the code is the issue here, not the fact
> that it is being linked against a GPL licensed header file.
>
> This isn't a GPL interpretation here, but a patent license agreement
> issue. Sorry for not being clearer the first time around, I thought
> everyone was aware of this issue by now.
>
> thanks,

Not! RCU was implemented by MIT in 1975 under a grant from NASA.
A subsequent patent by Sequent is likely invalid because of prior
art. Further, IBM purchased Sequent, not SCO.

Remember, regardless of how many times a lie is repeated, it
never becomes true.

Cheers,
Dick Johnson
Penguin : Linux version 2.6.9 on an i686 machine (5537.79 BogoMips).
Notice : All mail here is now cached for review by John Ashcroft.
98.36% of all statistics are fiction.

2004-11-23 19:47:00

by Greg KH

[permalink] [raw]
Subject: Re: [openib-general] Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

On Tue, Nov 23, 2004 at 02:30:52PM -0500, linux-os wrote:
>
> Not! RCU was implemented by MIT in 1975 under a grant from NASA.

Hm, the implementors of RCU might take objection to this. For more info
about RCU, please see:
http://lse.sourceforge.net/locking/rcupdate.html

thanks,

greg k-h

2004-11-23 19:18:22

by Johannes Erdfelt

[permalink] [raw]
Subject: Re: [openib-general] Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

On Tue, Nov 23, 2004, Greg KH <[email protected]> wrote:
> On Tue, Nov 23, 2004 at 09:52:46AM -0800, Johannes Erdfelt wrote:
> > On Mon, Nov 22, 2004, Greg KH <[email protected]> wrote:
> > > To be straightforward, either drop the RCU code completely, or change
> > > the license of your code.
> >
> > Or compile against non-GPL RCU code, right?
>
> No. RCU is covered by a patent that only allows for it to be
> implemented in GPL licensed code. If you want to use RCU in non-GPL
> code, you need to sign a license agreement with the holder of the RCU
> patent.
>
> This was all covered a few years ago when the RCU code first went into
> the kernel tree. See the lkml archives for details.
>
> So the very usage of RCU in the code is the issue here, not the fact
> that it is being linked against a GPL licensed header file.
>
> This isn't a GPL interpretation here, but a patent license agreement
> issue. Sorry for not being clearer the first time around, I thought
> everyone was aware of this issue by now.

Ahh, I missed that and it wasn't mentioned anywhere in the header file
atleast.

Thanks for clearing that up.

JE

2004-11-23 21:14:50

by Greg KH

[permalink] [raw]
Subject: Re: [openib-general] Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

On Tue, Nov 23, 2004 at 10:56:06AM -0800, Roland Dreier wrote:
> Greg> No. RCU is covered by a patent that only allows for it to
> Greg> be implemented in GPL licensed code. If you want to use RCU
> Greg> in non-GPL code, you need to sign a license agreement with
> Greg> the holder of the RCU patent.
>
> Surely IBM can implement RCU in non-GPLed AIX code or license the
> patent to whoever they like, with whatever terms they like?

As holders of the patent, they can. If you wish to do this, please
contact IBM's IP Licensing group :)

thanks,

greg k-h

2004-11-23 21:14:50

by Roland Dreier

[permalink] [raw]
Subject: Re: [openib-general] Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

Greg> No. RCU is covered by a patent that only allows for it to
Greg> be implemented in GPL licensed code. If you want to use RCU
Greg> in non-GPL code, you need to sign a license agreement with
Greg> the holder of the RCU patent.

Surely IBM can implement RCU in non-GPLed AIX code or license the
patent to whoever they like, with whatever terms they like?

- Roland

2004-11-24 19:59:50

by Greg KH

[permalink] [raw]
Subject: Re: [openib-general] Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

On Wed, Nov 24, 2004 at 11:23:37AM -0800, Roland Dreier wrote:
> Greg> No. RCU is covered by a patent that only allows for it to
> Greg> be implemented in GPL licensed code. If you want to use RCU
> Greg> in non-GPL code, you need to sign a license agreement with
> Greg> the holder of the RCU patent.
>
> Not to stir up the flames any further (and this is really a moot point
> since I got rid of our use of RCU)...
>
> But given that I could implement the same API as in rcupdate.h as
> below without using IBM's patented technique, how does merely using
> this API cause code to require a patent license?

Consult your IP lawyer for issues like this, I am not one, and do not
have an answer for this.

thanks,

greg k-h

2004-11-25 02:13:12

by Roland Dreier

[permalink] [raw]
Subject: Re: [openib-general] Re: [PATCH][RFC/v1][4/12] Add InfiniBand SA (Subnet Administration) query support

Greg> No. RCU is covered by a patent that only allows for it to
Greg> be implemented in GPL licensed code. If you want to use RCU
Greg> in non-GPL code, you need to sign a license agreement with
Greg> the holder of the RCU patent.

Not to stir up the flames any further (and this is really a moot point
since I got rid of our use of RCU)...

But given that I could implement the same API as in rcupdate.h as
below without using IBM's patented technique, how does merely using
this API cause code to require a patent license?

Thanks,
Roland

/*
* Copyright (c) 2004 Topspin Communications. All rights reserved.
*
* Usage of the works is permitted provided that this instrument is
* retained with the works, so that any entity that uses the works is
* notified of this instrument.
*
* DISCLAIMER: THE WORKS ARE WITHOUT WARRANTY.
*/

#ifndef MYRCU_H
#define MYRCU_H

struct rcu_head { };

#define rcu_read_lock lock_kernel()
#define rcu_read_unlock unlock_kernel()

#define rcu_dereference(p) (p)
#define rcu_assign_pointer(p,v) do { \
lock_kernel(); \
(p) = (v); \
unlock_kernel(); \
} while (0)

static inline void call_rcu(struct rcu_head *head,
void (*func)(struct rcu_head *head))
{
lock_kernel();
func(head);
unlock_kernel();
}

#endif /* MYRCU_H */