| 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
| ||
|
Date: Thu, 30 Aug 2012 17:26:09 +0300
From: "Michael S. Tsirkin" <mst@...hat.com>
To: Vlad Yasevich <vyasevic@...hat.com>
Cc: netdev@...r.kernel.org
Subject: Re: [RFC PATCH bridge 5/5] bridge: Add sysfs interface to display
VLANS
On Thu, Aug 30, 2012 at 10:05:16AM -0400, Vlad Yasevich wrote:
> On 08/30/2012 08:27 AM, Michael S. Tsirkin wrote:
> >On Thu, Aug 23, 2012 at 03:29:55PM -0400, Vlad Yasevich wrote:
> >>Add a binary sysfs file that will dump out vlans currently configured on the
> >>port.
> >>
> >>Signed-off-by: Vlad Yasevich <vyasevic@...hat.com>
> >
> >So what's the format here? I could not tell.
> >List of vlans? Why binary? Why not make it text in that case?
> >This would also allow reporting "all" if filtering
> >is disabled and "untagged" for untagged packets.
>
> I decided to do binary because text may result in more then page
> worth of data.
Well yes, you need 3 bytes to print a vid and 1 extra for a space. But
in binary, 2 bytes per VID and there are 4K valid VIDs so binary needs
more than 1 page too?
If using binary I would go for bit per valid bit
this solves it nicely.
We can have a separate field that shows whether untagged
packets are filtered.
> The display tool will know how to display things
> properly.
>
> -vlad
I think you know that one of the points of sysfs
is to allow use without tools.
Tools are happier using other interfaces such as
ioctl or netlink.
> >
> >>---
> >> include/linux/if_bridge.h | 1 +
> >> net/bridge/br_if.c | 34 ++++++++++++++++++++++++++++++++++
> >> net/bridge/br_private.h | 2 ++
> >> net/bridge/br_sysfs_if.c | 28 ++++++++++++++++++++++++++++
> >> 4 files changed, 65 insertions(+), 0 deletions(-)
> >>
> >>diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> >>index ab750dd..d0f869b 100644
> >>--- a/include/linux/if_bridge.h
> >>+++ b/include/linux/if_bridge.h
> >>@@ -20,6 +20,7 @@
> >> #define SYSFS_BRIDGE_PORT_SUBDIR "brif"
> >> #define SYSFS_BRIDGE_PORT_ATTR "brport"
> >> #define SYSFS_BRIDGE_PORT_LINK "bridge"
> >>+#define SYSFS_BRIDGE_PORT_VLANS "vlans"
> >>
> >> #define BRCTL_VERSION 1
> >>
> >>diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> >>index 90c1038..3963748 100644
> >>--- a/net/bridge/br_if.c
> >>+++ b/net/bridge/br_if.c
> >>@@ -510,6 +510,40 @@ int br_del_port_vlan(struct net_bridge_port *p, unsigned long vlan)
> >> return 0;
> >> }
> >>
> >>+size_t br_port_fill_vlans(struct net_bridge_port *p, char* buf,
> >>+ unsigned long max, unsigned long skip)
> >>+{
> >>+ unsigned long *map;
> >>+ unsigned short *vid = (unsigned short *)buf;
> >>+ unsigned short i;
> >>+ int num = 0;
> >>+
> >>+ if (skip > (VLAN_N_VID+1))
> >>+ return -EINVAL;
> >>+
> >>+ memset(buf, 0, max * sizeof(unsigned short));
> >
> >Isn't max is in bytes? why is this safe?
> >
> >>+
> >>+ rcu_read_lock();
> >>+ map = rcu_dereference(p->vlan_map);
> >>+ if (!map)
> >>+ goto out;
> >>+
> >>+ for (i = skip + 1; i < VLAN_N_VID + 1; i++) {
> >
> >Isn't skip in bytes too? Why do you compare it to i which is
> >in dwords?
> >
> >>+ if (test_bit(i, map)) {
> >>+ if (num > max)
> >>+ goto out;
> >>+
> >>+ *vid = i-1;
> >>+ vid++;
> >>+ num++;
> >>+ }
> >>+ }
> >>+out:
> >>+ rcu_read_unlock();
> >>+
> >>+ return num*sizeof(unsigned short);
> >>+}
> >>+
> >> void __net_exit br_net_exit(struct net *net)
> >> {
> >> struct net_device *dev;
> >>diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> >>index 5639c1c..cf95cd7 100644
> >>--- a/net/bridge/br_private.h
> >>+++ b/net/bridge/br_private.h
> >>@@ -404,6 +404,8 @@ extern netdev_features_t br_features_recompute(struct net_bridge *br,
> >> netdev_features_t features);
> >> extern int br_set_port_vlan(struct net_bridge_port *p, unsigned long vid);
> >> extern int br_del_port_vlan(struct net_bridge_port *p, unsigned long vid);
> >>+extern size_t br_port_fill_vlans(struct net_bridge_port *p, char *buf,
> >>+ unsigned long max, unsigned long skip);
> >>
> >> /* br_input.c */
> >> extern int br_handle_frame_finish(struct sk_buff *skb);
> >>diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
> >>index 13b36bd..a81e2ef 100644
> >>--- a/net/bridge/br_sysfs_if.c
> >>+++ b/net/bridge/br_sysfs_if.c
> >>@@ -234,6 +234,29 @@ const struct sysfs_ops brport_sysfs_ops = {
> >> };
> >>
> >> /*
> >>+ * Export the vlan table for a given port as a binary file.
> >>+ * The records are unsgined shorts.
> >>+ *
> >>+ * Returns the number of bytes read.
> >>+ */
> >>+static ssize_t brport_vlans_read(struct file *filp, struct kobject *kobj,
> >>+ struct bin_attribute *bin_attr,
> >>+ char *buf, loff_t off, size_t count)
> >>+{
> >>+ struct net_bridge_port *p = to_brport(kobj);
> >>+
> >>+ return br_port_fill_vlans(p, buf,
> >>+ count/sizeof(unsigned short),
> >>+ off/sizeof(unsigned short));
> >>+}
> >>+
> >>+static struct bin_attribute port_vlans = {
> >>+ .attr = { .name = SYSFS_BRIDGE_PORT_VLANS,
> >>+ .mode = S_IRUGO, },
> >>+ .read = brport_vlans_read,
> >>+};
> >>+
> >>+/*
> >> * Add sysfs entries to ethernet device added to a bridge.
> >> * Creates a brport subdirectory with bridge attributes.
> >> * Puts symlink in bridge's brif subdirectory
> >>@@ -255,6 +278,11 @@ int br_sysfs_addif(struct net_bridge_port *p)
> >> return err;
> >> }
> >>
> >>+ err = sysfs_create_bin_file(&p->kobj, &port_vlans);
> >>+ if (err) {
> >>+ return err;
> >>+ }
> >>+
> >> strlcpy(p->sysfs_name, p->dev->name, IFNAMSIZ);
> >> return sysfs_create_link(br->ifobj, &p->kobj, p->sysfs_name);
> >> }
> >>--
> >>1.7.7.6
> >>
> >>--
> >>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
--
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
Powered by blists - more mailing lists