netdev - Re: [RFC PATCH net-next 2/4] switchdev: add fwd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150614065601.GB2105@nanopsycho.orion>
Date:	Sun, 14 Jun 2015 08:56:02 +0200
From:	Jiri Pirko <jiri@...nulli.us>
To:	sfeldma@...il.com
Cc:	netdev@...r.kernel.org, simon.horman@...ronome.com,
	roopa@...ulusnetworks.com, ronen.arad@...el.com,
	john.r.fastabend@...el.com, andrew@...n.ch, f.fainelli@...il.com,
	linux@...ck-us.net, davidch@...adcom.com,
	stephen@...workplumber.org
Subject: Re: [RFC PATCH net-next 2/4] switchdev: add fwd_mark generator helper

Sat, Jun 13, 2015 at 08:04:28PM CEST, sfeldma@...il.com wrote:
>From: Scott Feldman <sfeldma@...il.com>
>
>skb->fwd_mark and dev->fwd_mark are 32-bit and should be unique for device
>and maybe even unique for a sub-set of ports within device, so add
>switchdev helper function to generate unique marks based on driver-supplied
>key.  Typically, the driver would use device switch ID for key, and maybe
>additional fields in key for grouped ports such as bridge ifindex.  The key
>can be of arbitrary length.
>
>The generator uses a global hash table to store fwd_marks hashed by key.
>
>Signed-off-by: Scott Feldman <sfeldma@...il.com>
>---
> include/net/switchdev.h   |    6 ++++
> net/switchdev/switchdev.c |   72 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 78 insertions(+)
>
>diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>index 437f8fe..6eaceee 100644
>--- a/include/net/switchdev.h
>+++ b/include/net/switchdev.h
>@@ -157,6 +157,7 @@ int switchdev_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
> int switchdev_port_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb,
> 			    struct net_device *dev,
> 			    struct net_device *filter_dev, int idx);
>+u32 switchdev_mark_get(void *key, size_t key_len);
> 
> #else
> 
>@@ -271,6 +272,11 @@ static inline int switchdev_port_fdb_dump(struct sk_buff *skb,
> 	return -EOPNOTSUPP;
> }
> 
>+static inline u32 switchdev_mark_get(void *key, size_t key_len)
>+{
>+	return 0;
>+}
>+
> #endif
> 
> #endif /* _LINUX_SWITCHDEV_H_ */
>diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>index a5d0f8e..9ca37b3 100644
>--- a/net/switchdev/switchdev.c
>+++ b/net/switchdev/switchdev.c
>@@ -16,6 +16,8 @@
> #include <linux/notifier.h>
> #include <linux/netdevice.h>
> #include <linux/if_bridge.h>
>+#include <linux/hashtable.h>
>+#include <linux/crc32.h>
> #include <net/ip_fib.h>
> #include <net/switchdev.h>
> 
>@@ -924,3 +926,73 @@ void switchdev_fib_ipv4_abort(struct fib_info *fi)
> 	fi->fib_net->ipv4.fib_offload_disabled = true;
> }
> EXPORT_SYMBOL_GPL(switchdev_fib_ipv4_abort);
>+
>+#define SWITCHDEV_MARK_HT_BITS	5
>+static DEFINE_HASHTABLE(switchdev_mark_ht, SWITCHDEV_MARK_HT_BITS);
>+static DEFINE_SPINLOCK(switchdev_mark_lock);
>+static u32 switchdev_mark_next = 1;
>+
>+/**
>+ *	switchdev_mark_get - Generate a unique mark for key
>+ *
>+ *	@key: key used to generate mark
>+ *	@key_len: length of key in bytes
>+ *
>+ *	Returns unqiue 32-bit mark for given key, or 0 if error.
                ^^^^^^
		typo.


>+ *	A small global hash table stores the marks for each key.
>+ *	The length of the key and key contents are arbitrary.
>+ *	The marks can be used, for example, to skb->fwd_mark a pkt
>+ *	to associate the skb with a key.
>+ */
>+u32 switchdev_mark_get(void *key, size_t key_len)
>+{
>+	struct switchdev_mark_ht_entry {
>+		struct hlist_node entry;
>+		void *key;
>+		size_t key_len;
>+		u32 key_crc32;
>+		u32 mark;
>+	} *entry;
>+	u32 key_crc32 = crc32(~0, key, key_len);
>+	u32 mark = 0;
>+	unsigned long flags;
>+
>+	spin_lock_irqsave(&switchdev_mark_lock, flags);

I fail to see why _irqsave variant is needed here.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html