| 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
| ||
|
Message-ID: <52F8FEF1.60407@redhat.com>
Date: Mon, 10 Feb 2014 11:31:45 -0500
From: Vlad Yasevich <vyasevic@...hat.com>
To: Jamal Hadi Salim <jhs@...atatu.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: Stephen Hemminger <stephen@...workplumber.org>,
Scott Feldman <sfeldma@...ulusnetworks.com>,
John Fastabend <john.r.fastabend@...el.com>
Subject: Re: RFC: bridge get fdb by bridge device
On 02/09/2014 10:06 AM, Jamal Hadi Salim wrote:
>
> This patch allows something equivalent to
> "brctl showmacs <bridge device>" with iproute2
> syntax "bridge link br <bridge device>"
> Filtering by bridge is done in the kernel.
> The current setup doesnt scale when you have many bridges each
> with large fdbs (preliminary fix with the kernel patch).
>
> iproute2 allows filtering by bridge port, example:
> "bridge link br br1234 dev port1234"
> but the filtering is done in user space.
> In a future patch i would like to do the port filtering
> in the kernel. As well, adding a MAC filter in the kernel
> makes sense.
>
> Kernel patch is against net-next.
>
> cheers,
> jamal
>
> bridge-fdb-filter1
>
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 393b1bc..507ea4e 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -2423,26 +2423,50 @@ static int rtnl_fdb_dump(struct sk_buff *skb,
struct netlink_callback *cb)
> {
> int idx = 0;
> struct net *net = sock_net(skb->sk);
> + const struct net_device_ops *ops;
> struct net_device *dev;
> + struct ndmsg *ndm;
>
> - rcu_read_lock();
> - for_each_netdev_rcu(net, dev) {
> - if (dev->priv_flags & IFF_BRIDGE_PORT) {
> - struct net_device *br_dev;
> - const struct net_device_ops *ops;
> -
> - br_dev = netdev_master_upper_dev_get(dev);
> - ops = br_dev->netdev_ops;
> - if (ops->ndo_fdb_dump)
> - idx = ops->ndo_fdb_dump(skb, cb, dev, idx);
> + ndm = nlmsg_data(cb->nlh);
> + if (ndm->ndm_ifindex) {
We get really lucky here that ndm_ifindex and ifi_index happen to map to
the same location.
> + dev = __dev_get_by_index(net, ndm->ndm_ifindex);
> + if (dev == NULL) {
> + pr_info("PF_BRIDGE: RTM_GETNEIGH with unknown ifindex\n");
> + return -ENODEV;
> + }
> +
> + if (!(dev->priv_flags & IFF_EBRIDGE)) {
> + pr_info("PF_BRIDGE: RTM_GETNEIGH %s not a bridge device\n",
> + dev->name);
> + return -EINVAL;
> }
> + ops = dev->netdev_ops;
> + if (ops->ndo_fdb_dump) {
> + idx = ops->ndo_fdb_dump(skb, cb, dev, idx);
> + } else {
> + pr_info("PF_BRIDGE: RTM_GETNEIGH %s no dumper\n",
> + dev->name);
> + return -EINVAL;
> + }
I agree with both of Johns commens fro the above code.
I think you can use ndo_dflt_fdb_dump() here and remove the first check
for IFF_EBRIDGE.
The only odd thing is that it would permit syntax like:
# bridge fbd show br eth0
or
# bridge fdb show br macvlan0
but I think that's ok.
> + } else {
> + rcu_read_lock();
> + for_each_netdev_rcu(net, dev) {
> + if (dev->priv_flags & IFF_BRIDGE_PORT) {
> + struct net_device *br_dev;
> + br_dev = netdev_master_upper_dev_get(dev);
> + ops = br_dev->netdev_ops;
> + if (ops->ndo_fdb_dump)
> + idx = ops->ndo_fdb_dump(skb, cb, dev, idx);
> + }
>
> - if (dev->netdev_ops->ndo_fdb_dump)
> - idx = dev->netdev_ops->ndo_fdb_dump(skb, cb, dev, idx);
> - else
> - idx = ndo_dflt_fdb_dump(skb, cb, dev, idx);
> + if (dev->netdev_ops->ndo_fdb_dump)
> + idx = dev->netdev_ops->ndo_fdb_dump(skb, cb, dev,
> + idx);
> + else
> + idx = ndo_dflt_fdb_dump(skb, cb, dev, idx);
> + }
> + rcu_read_unlock();
> }
> - rcu_read_unlock();
>
> cb->args[0] = idx;
> return skb->len;
>
>
> iprt-fdb-brfilter1
>
>
> diff --git a/bridge/fdb.c b/bridge/fdb.c
> index e2e53f1..f3073d6 100644
> --- a/bridge/fdb.c
> +++ b/bridge/fdb.c
> @@ -33,7 +33,7 @@ static void usage(void)
> fprintf(stderr, "Usage: bridge fdb { add | append | del | replace }
ADDR dev DEV {self|master} [ temp ]\n"
> " [router] [ dst IPADDR] [ vlan VID ]\n"
> " [ port PORT] [ vni VNI ] [via DEV]\n");
> - fprintf(stderr, " bridge fdb {show} [ dev DEV ]\n");
> + fprintf(stderr, " bridge fdb {show} [ br BRDEV ] [ dev DEV ]\n");
'port' option is now allowed in the show operation
-vlad
> exit(-1);
> }
>
> @@ -152,18 +152,35 @@ int print_fdb(const struct sockaddr_nl *who,
struct nlmsghdr *n, void *arg)
>
> static int fdb_show(int argc, char **argv)
> {
> + struct ndmsg ndm = { };
> char *filter_dev = NULL;
> + char *br = NULL;
> +
> + ndm.ndm_family = PF_BRIDGE;
> + ndm.ndm_state = NUD_NOARP;
>
> while (argc > 0) {
> - if (strcmp(*argv, "dev") == 0) {
> + if ((strcmp(*argv, "port") == 0) || strcmp(*argv, "dev") == 0) {
> NEXT_ARG();
> - if (filter_dev)
> - duparg("dev", *argv);
> filter_dev = *argv;
> + } else if (strcmp(*argv, "br") == 0) {
> + NEXT_ARG();
> + br = *argv;
> + } else {
> + if (matches(*argv, "help") == 0)
> + usage();
> }
> argc--; argv++;
> }
>
> + if (br) {
> + ndm.ndm_ifindex = ll_name_to_index(br);
> + if (ndm.ndm_ifindex == 0) {
> + fprintf(stderr, "Cannot find bridge device \"%s\"\n", br);
> + return -1;
> + }
> + }
> +
> if (filter_dev) {
> filter_index = if_nametoindex(filter_dev);
> if (filter_index == 0) {
> @@ -171,13 +188,15 @@ static int fdb_show(int argc, char **argv)
> filter_dev);
> return -1;
> }
> +
> }
>
> - if (rtnl_wilddump_request(&rth, PF_BRIDGE, RTM_GETNEIGH) < 0) {
> + if (rtnl_dump_request(&rth, RTM_GETNEIGH, &ndm, sizeof(struct
ndmsg)) < 0) {
> perror("Cannot send dump request");
> exit(1);
> }
>
> +
> if (rtnl_dump_filter(&rth, print_fdb, stdout) < 0) {
> fprintf(stderr, "Dump terminated\n");
> exit(1);
>
--
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