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: Fri, 12 Oct 2012 14:39:35 -0700 From: Joe Perches <joe@...ches.com> To: Arvid Brodin <Arvid.Brodin@...n.com> Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "shemminger@...tta.com" <shemminger@...tta.com>, "jboticario@...il.com" <jboticario@...il.com>, "balferreira@...glemail.com" <balferreira@...glemail.com> Subject: Re: [RFC v4 1/1] net/hsr: Support for the HSR protocol (IEC:2010 / HSR v0) On Fri, 2012-10-12 at 17:11 +0000, Arvid Brodin wrote: > This would add support for the High-availability Seamless Redundancy network > protocol, version 0 (2010). > > This RFC is NOT meant for mainline inclusion at the moment since we're trying > to figure out how to handle a probable incompatibility with the newer > HSR v1 (2012) standard. just trivial comments, ignore as you choose. > diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c [] > +void hsr_set_operstate(struct net_device *hsr_dev, struct net_device *slave1, > + struct net_device *slave2) > +{ > + if (!is_admin_up(hsr_dev)) { > + __hsr_set_operstate(hsr_dev, IF_OPER_DOWN); > + return; > + } > + > + if (is_operstate_up(slave1) || is_operstate_up(slave2)) > + __hsr_set_operstate(hsr_dev, IF_OPER_UP); > + else > + __hsr_set_operstate(hsr_dev, IF_OPER_LOWERLAYERDOWN); > +} sometimes it's better to have a single call like { int type; if (!is_admin_up(hsr_dev)) type = IF_OPER_DOWN; else if (is_operstate_up(slave1) || is_operstate_up(slave2)) type = IF_OPER_UP; else type = IF_OPER_LOWERLAYERDOWN; __hsr_set_operstate(hsr_dev, type); } [] > +static int hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev) > +{ > + struct hsr_priv *hsr_priv; > + struct hsr_ethhdr *hsr_ethhdr; > + struct sk_buff *skb2; > + int res1, res2; > + > + hsr_priv = netdev_priv(dev); > + hsr_ethhdr = (struct hsr_ethhdr *) skb->data; > + > + if ((ntohs(skb->protocol) != ETH_P_HSR) || > + (ntohs(hsr_ethhdr->ethhdr.h_proto) != ETH_P_HSR)) { These ntohs calls are done at runtime. The conversion could be done at compile time instead using if (skb->protocol != ntohs(ETH_P_HSR) || hsr_ethhdr->ethhdr.h_proto != ntohs(ETH_P_HSR)) { [] > +static void restore_slaves(struct net_device *hsr_dev) > +{ [] > + netdev_info(hsr_dev, "HSR: Cannot restore slave " > + "promiscuity (%s, %d)\n", > + hsr_priv->slave[i]->name, res); It's nicer to coalesce formats for easier grep netdev_info(hsr_dev, "HSR: Cannot restore slave promiscuity (%s, %d)\n", hsr_priv->slave[i]->name, res); or maybe use special wrappers for something like: int hsr_info(hsr, slave, fmt, ...) { } [] > diff --git a/net/hsr/hsr_framereg.c b/net/hsr/hsr_framereg.c > +static struct node_entry *find_node_by_AddrB(struct list_head *node_db, > + unsigned char addr[ETH_ALEN]) > +{ > + struct node_entry *node; > + > + list_for_each_entry_rcu(node, node_db, mac_list) > + if (!compare_ether_addr(node->MacAddressB, addr)) > + return node; please use braces when the list/for has an if -- 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