From: "Nicolas de Pesloüan" <nicolas.2p.debian@gmail.com>
To: Jiri Pirko <jpirko@redhat.com>
Cc: netdev@vger.kernel.org, davem@davemloft.net,
shemminger@linux-foundation.org, kaber@trash.net,
fubar@us.ibm.com, eric.dumazet@gmail.com, andy@greyhouse.net
Subject: Re: [patch net-next-2.6 3/8] net: get rid of multiple bond-related netdevice->priv_flags
Date: Sat, 05 Mar 2011 15:14:23 +0100 [thread overview]
Message-ID: <4D72453F.10909@gmail.com> (raw)
In-Reply-To: <1299320969-7951-4-git-send-email-jpirko@redhat.com>
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@redhat.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 */
next prev parent reply other threads:[~2011-03-05 14:14 UTC|newest]
Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-03-05 10:29 [patch net-next-2.6 0/8] mostly bonding rx path changes Jiri Pirko
2011-03-05 10:29 ` [patch net-next-2.6 1/8] af_packet: use skb->skb_iif instead of orig_dev->ifindex Jiri Pirko
2011-03-05 14:03 ` Nicolas de Pesloüan
2011-03-05 10:29 ` [patch net-next-2.6 2/8] bonding: register slave pointer for rx_handler Jiri Pirko
2011-03-05 14:06 ` Nicolas de Pesloüan
2011-03-05 14:27 ` Jiri Pirko
2011-03-05 14:38 ` Nicolas de Pesloüan
2011-03-05 10:29 ` [patch net-next-2.6 3/8] net: get rid of multiple bond-related netdevice->priv_flags Jiri Pirko
2011-03-05 14:14 ` Nicolas de Pesloüan [this message]
2011-03-05 14:37 ` Ben Hutchings
2011-03-05 14:46 ` Nicolas de Pesloüan
2011-03-05 10:29 ` [patch net-next-2.6 4/8] bonding: wrap slave state work Jiri Pirko
2011-03-05 15:21 ` Nicolas de Pesloüan
2011-03-07 9:58 ` Jiri Pirko
2011-03-07 19:55 ` Nicolas de Pesloüan
2011-03-08 7:18 ` Jiri Pirko
2011-03-08 21:23 ` Nicolas de Pesloüan
2011-03-05 10:29 ` [patch net-next-2.6 5/8] bonding: get rid of IFF_SLAVE_INACTIVE netdev->priv_flag Jiri Pirko
2011-03-05 14:18 ` Nicolas de Pesloüan
2011-03-05 10:29 ` [patch net-next-2.6 6/8] bonding: move processing of recv handlers into handle_frame() Jiri Pirko
2011-03-05 14:33 ` Nicolas de Pesloüan
2011-03-05 14:43 ` Jiri Pirko
2011-03-05 14:50 ` Nicolas de Pesloüan
2011-03-06 12:24 ` Nicolas de Pesloüan
2011-03-06 13:34 ` Jiri Pirko
2011-03-06 14:25 ` Nicolas de Pesloüan
2011-03-06 16:32 ` Michał Mirosław
2011-03-06 17:37 ` Nicolas de Pesloüan
2011-03-07 12:51 ` [patch net-next-2.6] net: reinject arps into bonding slave instead of master Jiri Pirko
2011-03-07 14:32 ` Andy Gospodarek
2011-03-07 20:12 ` Nicolas de Pesloüan
2011-03-07 21:19 ` Jiri Pirko
2011-03-07 21:30 ` Nicolas de Pesloüan
2011-03-07 22:43 ` Andy Gospodarek
2011-03-07 23:09 ` Nicolas de Pesloüan
2011-03-08 2:43 ` Andy Gospodarek
2011-03-08 21:34 ` Nicolas de Pesloüan
2011-03-08 7:13 ` Jiri Pirko
2011-03-08 13:42 ` Andy Gospodarek
2011-03-08 21:44 ` Nicolas de Pesloüan
2011-03-09 7:45 ` Jiri Pirko
2011-03-09 14:49 ` Nicolas de Pesloüan
2011-03-09 15:09 ` Jiri Pirko
2011-03-09 15:28 ` Nicolas de Pesloüan
2011-03-09 17:11 ` Jiri Pirko
2011-03-09 22:18 ` Nicolas de Pesloüan
2011-03-10 6:48 ` Jiri Pirko
2011-03-10 20:44 ` Nicolas de Pesloüan
2011-03-10 20:52 ` Jiri Pirko
2011-03-10 21:05 ` Jiri Pirko
2011-03-09 20:51 ` Jiri Pirko
2011-03-09 13:33 ` Neil Horman
2011-03-05 10:29 ` [patch net-next-2.6 7/8] net: introduce rx_handler results and logic around that Jiri Pirko
2011-03-05 12:48 ` Ben Hutchings
2011-03-05 14:52 ` Nicolas de Pesloüan
2011-03-05 14:54 ` Jiri Pirko
2011-03-05 15:06 ` Nicolas de Pesloüan
2011-03-05 15:13 ` [patch net-next-2.6] net: comment rx_handler results Jiri Pirko
2011-03-05 15:27 ` Nicolas de Pesloüan
2011-03-05 15:37 ` Jiri Pirko
2011-03-05 15:50 ` Nicolas de Pesloüan
2011-03-06 20:00 ` [PATCH net-next-2.6] net: enhance the documentation for rx_handler Nicolas de Pesloüan
2011-03-07 9:54 ` Jiri Pirko
2011-03-07 16:36 ` Stephen Hemminger
2011-03-07 20:01 ` [PATCH net-next-2.6 V2] " Nicolas de Pesloüan
2011-03-05 15:31 ` [patch net-next-2.6 7/8 v2] net: introduce rx_handler results and logic around that Jiri Pirko
2011-03-05 10:29 ` [patch net-next-2.6 8/8] net: get rid of orig_dev parameter of packet handlers Jiri Pirko
2011-03-05 15:05 ` Nicolas de Pesloüan
2011-03-05 15:15 ` Jiri Pirko
2011-03-05 15:32 ` [patch net-next-2.6 8/8 v2] " Jiri Pirko
2011-03-05 16:56 ` Nicolas de Pesloüan
2011-03-05 22:07 ` Jiri Pirko
2011-03-05 22:18 ` Nicolas de Pesloüan
-- strict thread matches above, loose matches on Subject: below --
2011-03-05 8:29 [patch net-next-2.6 0/8] mostly bonding rx path changes Jiri Pirko
2011-03-05 8:29 ` [patch net-next-2.6 3/8] net: get rid of multiple bond-related netdevice->priv_flags Jiri Pirko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4D72453F.10909@gmail.com \
--to=nicolas.2p.debian@gmail.com \
--cc=andy@greyhouse.net \
--cc=davem@davemloft.net \
--cc=eric.dumazet@gmail.com \
--cc=fubar@us.ibm.com \
--cc=jpirko@redhat.com \
--cc=kaber@trash.net \
--cc=netdev@vger.kernel.org \
--cc=shemminger@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).