| 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: <4D72453F.10909@gmail.com>
Date: Sat, 05 Mar 2011 15:14:23 +0100
From: Nicolas de Pesloüan
<nicolas.2p.debian@...il.com>
To: Jiri Pirko <jpirko@...hat.com>
CC: netdev@...r.kernel.org, davem@...emloft.net,
shemminger@...ux-foundation.org, kaber@...sh.net, fubar@...ibm.com,
eric.dumazet@...il.com, andy@...yhouse.net
Subject: Re: [patch net-next-2.6 3/8] net: get rid of multiple bond-related
netdevice->priv_flags
Le 05/03/2011 11:29, Jiri Pirko a écrit :
> Now when bond-related code is moved from net/core/dev.c into bonding
> code, multiple priv_flags are not needed anymore. So toss them out.
Agreed. See at the end for a single comment.
>
> Signed-off-by: Jiri Pirko<jpirko@...hat.com>
> ---
> drivers/net/bonding/bond_main.c | 33 ++++++++++++++-------------------
> drivers/net/bonding/bond_sysfs.c | 8 --------
> drivers/net/bonding/bonding.h | 24 +-----------------------
> include/linux/if.h | 22 +++++++++-------------
> 4 files changed, 24 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 1c19368..7923184 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1471,20 +1471,20 @@ static void bond_setup_by_slave(struct net_device *bond_dev,
> * ARP on active-backup slaves with arp_validate enabled.
> */
> static bool bond_should_deliver_exact_match(struct sk_buff *skb,
> - struct net_device *slave_dev,
> - struct net_device *bond_dev)
> + struct slave *slave,
> + struct bonding *bond)
> {
> - if (slave_dev->priv_flags& IFF_SLAVE_INACTIVE) {
> - if (slave_dev->priv_flags& IFF_SLAVE_NEEDARP&&
> + if (slave->dev->priv_flags& IFF_SLAVE_INACTIVE) {
> + if (slave_do_arp_validate(bond, slave)&&
> skb->protocol == __cpu_to_be16(ETH_P_ARP))
> return false;
>
> - if (bond_dev->priv_flags& IFF_MASTER_ALB&&
> + if (bond->params.mode == BOND_MODE_ALB&&
> skb->pkt_type != PACKET_BROADCAST&&
> skb->pkt_type != PACKET_MULTICAST)
> return false;
>
> - if (bond_dev->priv_flags& IFF_MASTER_8023AD&&
> + if (bond->params.mode == BOND_MODE_8023AD&&
> skb->protocol == __cpu_to_be16(ETH_P_SLOW))
> return false;
>
> @@ -1497,6 +1497,7 @@ static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
> {
> struct slave *slave;
> struct net_device *bond_dev;
> + struct bonding *bond;
>
> skb = skb_share_check(skb, GFP_ATOMIC);
> if (unlikely(!skb))
> @@ -1507,17 +1508,19 @@ static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
> if (unlikely(!bond_dev))
> return skb;
>
> - if (bond_dev->priv_flags& IFF_MASTER_ARPMON)
> + bond = netdev_priv(bond_dev);
> +
> + if (bond->params.arp_interval)
> slave->dev->last_rx = jiffies;
>
> - if (bond_should_deliver_exact_match(skb, slave->dev, bond_dev)) {
> + if (bond_should_deliver_exact_match(skb, slave, bond)) {
> skb->deliver_no_wcard = 1;
> return skb;
> }
>
> skb->dev = bond_dev;
>
> - if (bond_dev->priv_flags& IFF_MASTER_ALB&&
> + if (bond->params.mode == BOND_MODE_ALB&&
> bond_dev->priv_flags& IFF_BRIDGE_PORT&&
> skb->pkt_type == PACKET_HOST) {
> u16 *dest = (u16 *) eth_hdr(skb)->h_dest;
> @@ -2128,9 +2131,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
>
> dev_set_mtu(slave_dev, slave->original_mtu);
>
> - slave_dev->priv_flags&= ~(IFF_MASTER_8023AD | IFF_MASTER_ALB |
> - IFF_SLAVE_INACTIVE | IFF_BONDING |
> - IFF_SLAVE_NEEDARP);
> + slave_dev->priv_flags&= ~(IFF_SLAVE_INACTIVE | IFF_BONDING);
>
> kfree(slave);
>
> @@ -2241,8 +2242,7 @@ static int bond_release_all(struct net_device *bond_dev)
> dev_set_mac_address(slave_dev,&addr);
> }
>
> - slave_dev->priv_flags&= ~(IFF_MASTER_8023AD | IFF_MASTER_ALB |
> - IFF_SLAVE_INACTIVE);
> + slave_dev->priv_flags&= ~IFF_SLAVE_INACTIVE;
>
> kfree(slave);
>
> @@ -4718,11 +4718,9 @@ void bond_set_mode_ops(struct bonding *bond, int mode)
> case BOND_MODE_BROADCAST:
> break;
> case BOND_MODE_8023AD:
> - bond_set_master_3ad_flags(bond);
> bond_set_xmit_hash_policy(bond);
> break;
> case BOND_MODE_ALB:
> - bond_set_master_alb_flags(bond);
> /* FALLTHRU */
> case BOND_MODE_TLB:
> break;
> @@ -4813,9 +4811,6 @@ static void bond_setup(struct net_device *bond_dev)
> bond_dev->priv_flags |= IFF_BONDING;
> bond_dev->priv_flags&= ~IFF_XMIT_DST_RELEASE;
>
> - if (bond->params.arp_interval)
> - bond_dev->priv_flags |= IFF_MASTER_ARPMON;
> -
> /* At first, we block adding VLANs. That's the only way to
> * prevent problems that occur when adding VLANs over an
> * empty bond. The block will be removed once non-challenged
> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
> index 72bb0f6..05e0ae5 100644
> --- a/drivers/net/bonding/bond_sysfs.c
> +++ b/drivers/net/bonding/bond_sysfs.c
> @@ -322,11 +322,6 @@ static ssize_t bonding_store_mode(struct device *d,
> ret = -EINVAL;
> goto out;
> }
> - if (bond->params.mode == BOND_MODE_8023AD)
> - bond_unset_master_3ad_flags(bond);
> -
> - if (bond->params.mode == BOND_MODE_ALB)
> - bond_unset_master_alb_flags(bond);
>
> bond->params.mode = new_value;
> bond_set_mode_ops(bond, bond->params.mode);
> @@ -527,8 +522,6 @@ static ssize_t bonding_store_arp_interval(struct device *d,
> pr_info("%s: Setting ARP monitoring interval to %d.\n",
> bond->dev->name, new_value);
> bond->params.arp_interval = new_value;
> - if (bond->params.arp_interval)
> - bond->dev->priv_flags |= IFF_MASTER_ARPMON;
> if (bond->params.miimon) {
> pr_info("%s: ARP monitoring cannot be used with MII monitoring. %s Disabling MII monitoring.\n",
> bond->dev->name, bond->dev->name);
> @@ -1004,7 +997,6 @@ static ssize_t bonding_store_miimon(struct device *d,
> pr_info("%s: MII monitoring cannot be used with ARP monitoring. Disabling ARP monitoring...\n",
> bond->dev->name);
> bond->params.arp_interval = 0;
> - bond->dev->priv_flags&= ~IFF_MASTER_ARPMON;
> if (bond->params.arp_validate) {
> bond_unregister_arp(bond);
> bond->params.arp_validate =
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index 1aac5cd..ddee62f 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -354,34 +354,12 @@ static inline void bond_set_slave_inactive_flags(struct slave *slave)
> slave->state = BOND_STATE_BACKUP;
> if (!bond->params.all_slaves_active)
> slave->dev->priv_flags |= IFF_SLAVE_INACTIVE;
> - if (slave_do_arp_validate(bond, slave))
> - slave->dev->priv_flags |= IFF_SLAVE_NEEDARP;
> }
>
> static inline void bond_set_slave_active_flags(struct slave *slave)
> {
> slave->state = BOND_STATE_ACTIVE;
> - slave->dev->priv_flags&= ~(IFF_SLAVE_INACTIVE | IFF_SLAVE_NEEDARP);
> -}
> -
> -static inline void bond_set_master_3ad_flags(struct bonding *bond)
> -{
> - bond->dev->priv_flags |= IFF_MASTER_8023AD;
> -}
> -
> -static inline void bond_unset_master_3ad_flags(struct bonding *bond)
> -{
> - bond->dev->priv_flags&= ~IFF_MASTER_8023AD;
> -}
> -
> -static inline void bond_set_master_alb_flags(struct bonding *bond)
> -{
> - bond->dev->priv_flags |= IFF_MASTER_ALB;
> -}
> -
> -static inline void bond_unset_master_alb_flags(struct bonding *bond)
> -{
> - bond->dev->priv_flags&= ~IFF_MASTER_ALB;
> + slave->dev->priv_flags&= ~IFF_SLAVE_INACTIVE;
> }
>
> struct vlan_entry *bond_next_vlan(struct bonding *bond, struct vlan_entry *curr);
> diff --git a/include/linux/if.h b/include/linux/if.h
> index 3bc63e6..2fdd47a 100644
> --- a/include/linux/if.h
> +++ b/include/linux/if.h
> @@ -60,21 +60,17 @@
> #define IFF_802_1Q_VLAN 0x1 /* 802.1Q VLAN device. */
> #define IFF_EBRIDGE 0x2 /* Ethernet bridging device. */
> #define IFF_SLAVE_INACTIVE 0x4 /* bonding slave not the curr. active */
> -#define IFF_MASTER_8023AD 0x8 /* bonding master, 802.3ad. */
> -#define IFF_MASTER_ALB 0x10 /* bonding master, balance-alb. */
> -#define IFF_BONDING 0x20 /* bonding master or slave */
> -#define IFF_SLAVE_NEEDARP 0x40 /* need ARPs for validation */
> -#define IFF_ISATAP 0x80 /* ISATAP interface (RFC4214) */
> -#define IFF_MASTER_ARPMON 0x100 /* bonding master, ARP mon in use */
> -#define IFF_WAN_HDLC 0x200 /* WAN HDLC device */
> -#define IFF_XMIT_DST_RELEASE 0x400 /* dev_hard_start_xmit() is allowed to
> +#define IFF_BONDING 0x8 /* bonding master or slave */
> +#define IFF_ISATAP 0x10 /* ISATAP interface (RFC4214) */
> +#define IFF_WAN_HDLC 0x20 /* WAN HDLC device */
> +#define IFF_XMIT_DST_RELEASE 0x40 /* dev_hard_start_xmit() is allowed to
> * release skb->dst
> */
Why did you changed those values? Aren't those exposed to userland? Just removing the unused one
sounds good to me.
> -#define IFF_DONT_BRIDGE 0x800 /* disallow bridging this ether dev */
> -#define IFF_DISABLE_NETPOLL 0x1000 /* disable netpoll at run-time */
> -#define IFF_MACVLAN_PORT 0x2000 /* device used as macvlan port */
> -#define IFF_BRIDGE_PORT 0x4000 /* device used as bridge port */
> -#define IFF_OVS_DATAPATH 0x8000 /* device used as Open vSwitch
> +#define IFF_DONT_BRIDGE 0x80 /* disallow bridging this ether dev */
> +#define IFF_DISABLE_NETPOLL 0x100 /* disable netpoll at run-time */
> +#define IFF_MACVLAN_PORT 0x200 /* device used as macvlan port */
> +#define IFF_BRIDGE_PORT 0x400 /* device used as bridge port */
> +#define IFF_OVS_DATAPATH 0x800 /* device used as Open vSwitch
> * datapath port */
>
> #define IF_GET_IFACE 0x0001 /* for querying only */
--
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