Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH next 5/6] bonding: Allow userspace to set macaddr on bonding-dev
From: Jay Vosburgh @ 2015-02-07  1:54 UTC (permalink / raw)
  To: Mahesh Bandewar
  Cc: Andy Gospodarek, Veaceslav Falico, Nikolay Aleksandrov,
	David Miller, netdev, Eric Dumazet, Maciej Żenczykowski
In-Reply-To: <CAF2d9jgV5adAjydN18Ct7BVJJcc=FRc+5icNMS=pRQMRX=83+w@mail.gmail.com>

Mahesh Bandewar <maheshb@google.com> wrote:

>On Fri, Feb 6, 2015 at 5:20 PM, Jay Vosburgh <jay.vosburgh@canonical.com> wrote:
>> Mahesh Bandewar <maheshb@google.com> wrote:
>>
>>>This patch allows user-space to set the mac-address on the bonding device.
>>>This mac-address can not be NULL or a Multicast. If the mac-address is set
>>>from user-space; kernel will honor it and will not overwrite it. In the
>>>absense (value from user space); the logic will default to using the
>>>masters' mac as the mac address for the bonding device.
>>>
>>>It can be set using example code below -
>>>
>>>   # modprobe bonding mode=4
>>>   # sys_mac_addr=$(printf '%02x:%02x:%02x:%02x:%02x:%02x' \
>>>                    $(( (RANDOM & 0xFE) | 0x02 )) \
>>>                    $(( RANDOM & 0xFF )) \
>>>                    $(( RANDOM & 0xFF )) \
>>>                    $(( RANDOM & 0xFF )) \
>>>                    $(( RANDOM & 0xFF )) \
>>>                    $(( RANDOM & 0xFF )))
>>>   # echo $sys_mac_addr > /sys/class/net/bond0/bonding/ad_actor_system_mac_address
>>>   # echo +eth1 > /sys/class/net/bond0/bonding/slaves
>>>   ...
>>>   # ip link set bond0 up
>>
>>         How is this patch functionally different from setting the
>> bonding master's MAC address to a particular value prior to adding any
>> slaves?
>>
>
>Maciej is correct but I think I was bit ambiguous about it in the
>commit message which might have made you think this way. I'll reword
>the commit message.

	Thanks; presumably there is some administrative reason for this.

	Also, for patches 4, 5 and 6, I believe current practice is to
provide a netlink / iproute2 facility for options.

	-J


>>         -J
>>
>>>Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>>>---
>>> drivers/net/bonding/bond_3ad.c     |  7 ++++++-
>>> drivers/net/bonding/bond_main.c    |  1 +
>>> drivers/net/bonding/bond_options.c | 29 +++++++++++++++++++++++++++++
>>> drivers/net/bonding/bond_procfs.c  |  6 ++++++
>>> drivers/net/bonding/bond_sysfs.c   | 16 ++++++++++++++++
>>> include/net/bond_options.h         |  1 +
>>> include/net/bonding.h              |  1 +
>>> 7 files changed, 60 insertions(+), 1 deletion(-)
>>>
>>>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>>>index 1177f96194dd..373d3db3809f 100644
>>>--- a/drivers/net/bonding/bond_3ad.c
>>>+++ b/drivers/net/bonding/bond_3ad.c
>>>@@ -1914,7 +1914,12 @@ void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution)
>>>
>>>               BOND_AD_INFO(bond).system.sys_priority =
>>>                       bond->params.ad_actor_sysprio;
>>>-              BOND_AD_INFO(bond).system.sys_mac_addr = *((struct mac_addr *)bond->dev->dev_addr);
>>>+              if (is_zero_ether_addr(bond->params.ad_actor_sys_macaddr))
>>>+                      BOND_AD_INFO(bond).system.sys_mac_addr =
>>>+                          *((struct mac_addr *)bond->dev->dev_addr);
>>>+              else
>>>+                      BOND_AD_INFO(bond).system.sys_mac_addr =
>>>+                          *((struct mac_addr *)bond->params.ad_actor_sys_macaddr);
>>>
>>>               /* initialize how many times this module is called in one
>>>                * second (should be about every 100ms)
>>>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>>index 561b2bde5aeb..1a6735ef2ea7 100644
>>>--- a/drivers/net/bonding/bond_main.c
>>>+++ b/drivers/net/bonding/bond_main.c
>>>@@ -4440,6 +4440,7 @@ static int bond_check_params(struct bond_params *params)
>>>       params->packets_per_slave = packets_per_slave;
>>>       params->tlb_dynamic_lb = 1; /* Default value */
>>>       params->ad_actor_sysprio = ad_actor_sysprio;
>>>+      eth_zero_addr(params->ad_actor_sys_macaddr);
>>>       if (packets_per_slave > 0) {
>>>               params->reciprocal_packets_per_slave =
>>>                       reciprocal_value(packets_per_slave);
>>>diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>>>index d8f6760143ae..330d48b6a1e6 100644
>>>--- a/drivers/net/bonding/bond_options.c
>>>+++ b/drivers/net/bonding/bond_options.c
>>>@@ -72,6 +72,8 @@ static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
>>>                                 const struct bond_opt_value *newval);
>>> static int bond_option_ad_actor_sysprio_set(struct bonding *bond,
>>>                                 const struct bond_opt_value *newval);
>>>+static int bond_option_ad_actor_sys_macaddr_set(struct bonding *bond,
>>>+                                const struct bond_opt_value *newval);
>>>
>>>
>>> static const struct bond_opt_value bond_mode_tbl[] = {
>>>@@ -396,6 +398,13 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
>>>               .values = bond_ad_actor_sysprio_tbl,
>>>               .set = bond_option_ad_actor_sysprio_set,
>>>       },
>>>+      [BOND_OPT_AD_ACTOR_SYS_MACADDR] = {
>>>+              .id = BOND_OPT_AD_ACTOR_SYS_MACADDR,
>>>+              .name = "ad_actor_system_mac_address",
>>>+              .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
>>>+              .flags = BOND_OPTFLAG_RAWVAL | BOND_OPTFLAG_IFDOWN,
>>>+              .set = bond_option_ad_actor_sys_macaddr_set,
>>>+      },
>>> };
>>>
>>> /* Searches for an option by name */
>>>@@ -1376,3 +1385,23 @@ static int bond_option_ad_actor_sysprio_set(struct bonding *bond,
>>>       bond->params.ad_actor_sysprio = newval->value;
>>>       return 0;
>>> }
>>>+
>>>+static int bond_option_ad_actor_sys_macaddr_set(struct bonding *bond,
>>>+                                          const struct bond_opt_value *newval)
>>>+{
>>>+      u8 macaddr[ETH_ALEN];
>>>+      int i;
>>>+
>>>+      i = sscanf(newval->string, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
>>>+                 &macaddr[0], &macaddr[1], &macaddr[2],
>>>+                 &macaddr[3], &macaddr[4], &macaddr[5]);
>>>+
>>>+      if (i != ETH_ALEN || !is_valid_ether_addr(macaddr)) {
>>>+              netdev_err(bond->dev, "Invalid MAC address.\n");
>>>+              return -EINVAL;
>>>+      }
>>>+
>>>+      ether_addr_copy(bond->params.ad_actor_sys_macaddr, macaddr);
>>>+
>>>+      return 0;
>>>+}
>>>diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
>>>index 9e33c48886ef..81452ced852f 100644
>>>--- a/drivers/net/bonding/bond_procfs.c
>>>+++ b/drivers/net/bonding/bond_procfs.c
>>>@@ -136,6 +136,8 @@ static void bond_info_show_master(struct seq_file *seq)
>>>                          optval->string);
>>>               seq_printf(seq, "System priority: %d\n",
>>>                               BOND_AD_INFO(bond).system.sys_priority);
>>>+              seq_printf(seq, "System MAC address: %pM\n",
>>>+                              &BOND_AD_INFO(bond).system.sys_mac_addr);
>>>
>>>               if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
>>>                       seq_printf(seq, "bond %s has no active aggregator\n",
>>>@@ -198,6 +200,8 @@ static void bond_info_show_slave(struct seq_file *seq,
>>>                       seq_puts(seq, "details actor lacp pdu:\n");
>>>                       seq_printf(seq, "    system priority: %d\n",
>>>                                  port->actor_system_priority);
>>>+                      seq_printf(seq, "    system mac address: %pM\n",
>>>+                                 &port->actor_system);
>>>                       seq_printf(seq, "    port key: %d\n",
>>>                                  port->actor_oper_port_key);
>>>                       seq_printf(seq, "    port priority: %d\n",
>>>@@ -210,6 +214,8 @@ static void bond_info_show_slave(struct seq_file *seq,
>>>                       seq_puts(seq, "details partner lacp pdu:\n");
>>>                       seq_printf(seq, "    system priority: %d\n",
>>>                                  port->partner_oper.system_priority);
>>>+                      seq_printf(seq, "    system mac address: %pM\n",
>>>+                                 &port->partner_oper.system);
>>>                       seq_printf(seq, "    oper key: %d\n",
>>>                                  port->partner_oper.key);
>>>                       seq_printf(seq, "    port priority: %d\n",
>>>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>>>index 4350aa06f867..91713d0b6685 100644
>>>--- a/drivers/net/bonding/bond_sysfs.c
>>>+++ b/drivers/net/bonding/bond_sysfs.c
>>>@@ -706,6 +706,21 @@ static ssize_t bonding_show_ad_actor_sysprio(struct device *d,
>>> static DEVICE_ATTR(ad_actor_system_priority, S_IRUGO | S_IWUSR,
>>>                  bonding_show_ad_actor_sysprio, bonding_sysfs_store_option);
>>>
>>>+static ssize_t bonding_show_ad_actor_sys_macaddr(struct device *d,
>>>+                                               struct device_attribute *attr,
>>>+                                               char *buf)
>>>+{
>>>+      struct bonding *bond = to_bond(d);
>>>+
>>>+      if (BOND_MODE(bond) == BOND_MODE_8023AD)
>>>+              return sprintf(buf, "%pM\n", bond->params.ad_actor_sys_macaddr);
>>>+
>>>+      return 0;
>>>+}
>>>+static DEVICE_ATTR(ad_actor_system_mac_address, S_IRUGO | S_IWUSR,
>>>+                 bonding_show_ad_actor_sys_macaddr,
>>>+                 bonding_sysfs_store_option);
>>>+
>>> static struct attribute *per_bond_attrs[] = {
>>>       &dev_attr_slaves.attr,
>>>       &dev_attr_mode.attr,
>>>@@ -740,6 +755,7 @@ static struct attribute *per_bond_attrs[] = {
>>>       &dev_attr_packets_per_slave.attr,
>>>       &dev_attr_tlb_dynamic_lb.attr,
>>>       &dev_attr_ad_actor_system_priority.attr,
>>>+      &dev_attr_ad_actor_system_mac_address.attr,
>>>       NULL,
>>> };
>>>
>>>diff --git a/include/net/bond_options.h b/include/net/bond_options.h
>>>index c2af1db37354..993ef73cd050 100644
>>>--- a/include/net/bond_options.h
>>>+++ b/include/net/bond_options.h
>>>@@ -64,6 +64,7 @@ enum {
>>>       BOND_OPT_SLAVES,
>>>       BOND_OPT_TLB_DYNAMIC_LB,
>>>       BOND_OPT_AD_ACTOR_SYSPRIO,
>>>+      BOND_OPT_AD_ACTOR_SYS_MACADDR,
>>>       BOND_OPT_LAST
>>> };
>>>
>>>diff --git a/include/net/bonding.h b/include/net/bonding.h
>>>index 7a5c79fcf866..cd2092e6bc71 100644
>>>--- a/include/net/bonding.h
>>>+++ b/include/net/bonding.h
>>>@@ -144,6 +144,7 @@ struct bond_params {
>>>       int tlb_dynamic_lb;
>>>       struct reciprocal_value reciprocal_packets_per_slave;
>>>       u16 ad_actor_sysprio;
>>>+      u8 ad_actor_sys_macaddr[ETH_ALEN];
>>> };
>>>
>>> struct bond_parm_tbl {
>>>--
>>>2.2.0.rc0.207.ga3a616c
>>>

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

^ permalink raw reply

* Re: [PATCH] af_packet: don't pass empty blocks for PACKET_V3
From: Willem de Bruijn @ 2015-02-07  1:45 UTC (permalink / raw)
  To: Alexander Drozdov
  Cc: David S. Miller, Daniel Borkmann, Eric Dumazet, Al Viro,
	Michael S. Tsirkin, Network Development, linux-kernel, Guy Harris,
	Dan Collins
In-Reply-To: <54D4651D.6060300@gmail.com>

On Thu, Feb 5, 2015 at 10:54 PM, Alexander Drozdov <al.drozdov@gmail.com> wrote:
> On 05.02.2015 23:01:38 +0300 Willem de Bruijn wrote:
>>
>> On Wed, Feb 4, 2015 at 9:58 PM, Alexander Drozdov <al.drozdov@gmail.com>
>> wrote:
>>>
>>> Don't close an empty block on timeout. Its meaningless to
>>> pass it to the user. Moreover, passing empty blocks wastes
>>> CPU & buffer space increasing probability of packets
>>> dropping on small timeouts.
>>>
>>> Side effect of this patch is indefinite user-space wait
>>> in poll on idle links. But, I believe its better to set
>>> timeout for poll(2) when needed than to get empty blocks
>>> every millisecond when not needed.
>>
>> This change would break existing applications that have come
>> to depend on the periodic signal.
>>
>> I don't disagree with the argument that the data ready signal
>> should be sent only when a block is full or a timer expires and
>> at least some data is waiting, but that is moot at this point.
>
> I missed something. As pointed by Guy Harris <guy@alum.mit.edu>,
> before the previous patch periodic signal was not delivered. The previous
> patch
> (da413eec729dae5dc by Dan Collins <dan@dcollins.co.nz>) is for 3.19 kernel
> only. Should we care about existing 3.19-only applications?

It does sound reasonable to expect processes to handle infinite sleep
on no data if that is the historical behavior of the interface.

>>
>>> Signed-off-by: Alexander Drozdov <al.drozdov@gmail.com>
>>> ---
>>>   net/packet/af_packet.c | 10 +++++++++-
>>>   1 file changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
>>> index 9cfe2e1..9a2f70a 100644
>>> --- a/net/packet/af_packet.c
>>> +++ b/net/packet/af_packet.c
>>> @@ -698,6 +698,10 @@ static void prb_retire_rx_blk_timer_expired(unsigned
>>> long data)
>>>
>>>          if (pkc->last_kactive_blk_num == pkc->kactive_blk_num) {
>>>                  if (!frozen) {
>>> +                       if (!BLOCK_NUM_PKTS(pbd)) {
>>> +                               /* An empty block. Just refresh the
>>> timer. */
>>> +                               goto refresh_timer;
>>> +                       }
>>>                          prb_retire_current_block(pkc, po,
>>> TP_STATUS_BLK_TMO);
>>>                          if (!prb_dispatch_next_block(pkc, po))
>>>                                  goto refresh_timer;
>>> @@ -798,7 +802,11 @@ static void prb_close_block(struct tpacket_kbdq_core
>>> *pkc1,
>>>                  h1->ts_last_pkt.ts_sec = last_pkt->tp_sec;
>>>                  h1->ts_last_pkt.ts_nsec = last_pkt->tp_nsec;
>>>          } else {
>>> -               /* Ok, we tmo'd - so get the current time */
>>> +               /* Ok, we tmo'd - so get the current time.
>>> +                *
>>> +                * It shouldn't really happen as we don't close empty
>>> +                * blocks. See prb_retire_rx_blk_timer_expired().
>>> +                */
>>>                  struct timespec ts;
>>>                  getnstimeofday(&ts);
>>>                  h1->ts_last_pkt.ts_sec = ts.tv_sec;
>>> --
>>> 1.9.1
>>>
>

^ permalink raw reply

* Re: [PATCH next 5/6] bonding: Allow userspace to set macaddr on bonding-dev
From: Mahesh Bandewar @ 2015-02-07  1:35 UTC (permalink / raw)
  To: Jay Vosburgh
  Cc: Andy Gospodarek, Veaceslav Falico, Nikolay Aleksandrov,
	David Miller, netdev, Eric Dumazet, Maciej Żenczykowski
In-Reply-To: <4192.1423272023@famine>

On Fri, Feb 6, 2015 at 5:20 PM, Jay Vosburgh <jay.vosburgh@canonical.com> wrote:
> Mahesh Bandewar <maheshb@google.com> wrote:
>
>>This patch allows user-space to set the mac-address on the bonding device.
>>This mac-address can not be NULL or a Multicast. If the mac-address is set
>>from user-space; kernel will honor it and will not overwrite it. In the
>>absense (value from user space); the logic will default to using the
>>masters' mac as the mac address for the bonding device.
>>
>>It can be set using example code below -
>>
>>   # modprobe bonding mode=4
>>   # sys_mac_addr=$(printf '%02x:%02x:%02x:%02x:%02x:%02x' \
>>                    $(( (RANDOM & 0xFE) | 0x02 )) \
>>                    $(( RANDOM & 0xFF )) \
>>                    $(( RANDOM & 0xFF )) \
>>                    $(( RANDOM & 0xFF )) \
>>                    $(( RANDOM & 0xFF )) \
>>                    $(( RANDOM & 0xFF )))
>>   # echo $sys_mac_addr > /sys/class/net/bond0/bonding/ad_actor_system_mac_address
>>   # echo +eth1 > /sys/class/net/bond0/bonding/slaves
>>   ...
>>   # ip link set bond0 up
>
>         How is this patch functionally different from setting the
> bonding master's MAC address to a particular value prior to adding any
> slaves?
>

Maciej is correct but I think I was bit ambiguous about it in the
commit message which might have made you think this way. I'll reword
the commit message.

>         -J
>
>>Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>>---
>> drivers/net/bonding/bond_3ad.c     |  7 ++++++-
>> drivers/net/bonding/bond_main.c    |  1 +
>> drivers/net/bonding/bond_options.c | 29 +++++++++++++++++++++++++++++
>> drivers/net/bonding/bond_procfs.c  |  6 ++++++
>> drivers/net/bonding/bond_sysfs.c   | 16 ++++++++++++++++
>> include/net/bond_options.h         |  1 +
>> include/net/bonding.h              |  1 +
>> 7 files changed, 60 insertions(+), 1 deletion(-)
>>
>>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>>index 1177f96194dd..373d3db3809f 100644
>>--- a/drivers/net/bonding/bond_3ad.c
>>+++ b/drivers/net/bonding/bond_3ad.c
>>@@ -1914,7 +1914,12 @@ void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution)
>>
>>               BOND_AD_INFO(bond).system.sys_priority =
>>                       bond->params.ad_actor_sysprio;
>>-              BOND_AD_INFO(bond).system.sys_mac_addr = *((struct mac_addr *)bond->dev->dev_addr);
>>+              if (is_zero_ether_addr(bond->params.ad_actor_sys_macaddr))
>>+                      BOND_AD_INFO(bond).system.sys_mac_addr =
>>+                          *((struct mac_addr *)bond->dev->dev_addr);
>>+              else
>>+                      BOND_AD_INFO(bond).system.sys_mac_addr =
>>+                          *((struct mac_addr *)bond->params.ad_actor_sys_macaddr);
>>
>>               /* initialize how many times this module is called in one
>>                * second (should be about every 100ms)
>>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>index 561b2bde5aeb..1a6735ef2ea7 100644
>>--- a/drivers/net/bonding/bond_main.c
>>+++ b/drivers/net/bonding/bond_main.c
>>@@ -4440,6 +4440,7 @@ static int bond_check_params(struct bond_params *params)
>>       params->packets_per_slave = packets_per_slave;
>>       params->tlb_dynamic_lb = 1; /* Default value */
>>       params->ad_actor_sysprio = ad_actor_sysprio;
>>+      eth_zero_addr(params->ad_actor_sys_macaddr);
>>       if (packets_per_slave > 0) {
>>               params->reciprocal_packets_per_slave =
>>                       reciprocal_value(packets_per_slave);
>>diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>>index d8f6760143ae..330d48b6a1e6 100644
>>--- a/drivers/net/bonding/bond_options.c
>>+++ b/drivers/net/bonding/bond_options.c
>>@@ -72,6 +72,8 @@ static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
>>                                 const struct bond_opt_value *newval);
>> static int bond_option_ad_actor_sysprio_set(struct bonding *bond,
>>                                 const struct bond_opt_value *newval);
>>+static int bond_option_ad_actor_sys_macaddr_set(struct bonding *bond,
>>+                                const struct bond_opt_value *newval);
>>
>>
>> static const struct bond_opt_value bond_mode_tbl[] = {
>>@@ -396,6 +398,13 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
>>               .values = bond_ad_actor_sysprio_tbl,
>>               .set = bond_option_ad_actor_sysprio_set,
>>       },
>>+      [BOND_OPT_AD_ACTOR_SYS_MACADDR] = {
>>+              .id = BOND_OPT_AD_ACTOR_SYS_MACADDR,
>>+              .name = "ad_actor_system_mac_address",
>>+              .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
>>+              .flags = BOND_OPTFLAG_RAWVAL | BOND_OPTFLAG_IFDOWN,
>>+              .set = bond_option_ad_actor_sys_macaddr_set,
>>+      },
>> };
>>
>> /* Searches for an option by name */
>>@@ -1376,3 +1385,23 @@ static int bond_option_ad_actor_sysprio_set(struct bonding *bond,
>>       bond->params.ad_actor_sysprio = newval->value;
>>       return 0;
>> }
>>+
>>+static int bond_option_ad_actor_sys_macaddr_set(struct bonding *bond,
>>+                                          const struct bond_opt_value *newval)
>>+{
>>+      u8 macaddr[ETH_ALEN];
>>+      int i;
>>+
>>+      i = sscanf(newval->string, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
>>+                 &macaddr[0], &macaddr[1], &macaddr[2],
>>+                 &macaddr[3], &macaddr[4], &macaddr[5]);
>>+
>>+      if (i != ETH_ALEN || !is_valid_ether_addr(macaddr)) {
>>+              netdev_err(bond->dev, "Invalid MAC address.\n");
>>+              return -EINVAL;
>>+      }
>>+
>>+      ether_addr_copy(bond->params.ad_actor_sys_macaddr, macaddr);
>>+
>>+      return 0;
>>+}
>>diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
>>index 9e33c48886ef..81452ced852f 100644
>>--- a/drivers/net/bonding/bond_procfs.c
>>+++ b/drivers/net/bonding/bond_procfs.c
>>@@ -136,6 +136,8 @@ static void bond_info_show_master(struct seq_file *seq)
>>                          optval->string);
>>               seq_printf(seq, "System priority: %d\n",
>>                               BOND_AD_INFO(bond).system.sys_priority);
>>+              seq_printf(seq, "System MAC address: %pM\n",
>>+                              &BOND_AD_INFO(bond).system.sys_mac_addr);
>>
>>               if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
>>                       seq_printf(seq, "bond %s has no active aggregator\n",
>>@@ -198,6 +200,8 @@ static void bond_info_show_slave(struct seq_file *seq,
>>                       seq_puts(seq, "details actor lacp pdu:\n");
>>                       seq_printf(seq, "    system priority: %d\n",
>>                                  port->actor_system_priority);
>>+                      seq_printf(seq, "    system mac address: %pM\n",
>>+                                 &port->actor_system);
>>                       seq_printf(seq, "    port key: %d\n",
>>                                  port->actor_oper_port_key);
>>                       seq_printf(seq, "    port priority: %d\n",
>>@@ -210,6 +214,8 @@ static void bond_info_show_slave(struct seq_file *seq,
>>                       seq_puts(seq, "details partner lacp pdu:\n");
>>                       seq_printf(seq, "    system priority: %d\n",
>>                                  port->partner_oper.system_priority);
>>+                      seq_printf(seq, "    system mac address: %pM\n",
>>+                                 &port->partner_oper.system);
>>                       seq_printf(seq, "    oper key: %d\n",
>>                                  port->partner_oper.key);
>>                       seq_printf(seq, "    port priority: %d\n",
>>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>>index 4350aa06f867..91713d0b6685 100644
>>--- a/drivers/net/bonding/bond_sysfs.c
>>+++ b/drivers/net/bonding/bond_sysfs.c
>>@@ -706,6 +706,21 @@ static ssize_t bonding_show_ad_actor_sysprio(struct device *d,
>> static DEVICE_ATTR(ad_actor_system_priority, S_IRUGO | S_IWUSR,
>>                  bonding_show_ad_actor_sysprio, bonding_sysfs_store_option);
>>
>>+static ssize_t bonding_show_ad_actor_sys_macaddr(struct device *d,
>>+                                               struct device_attribute *attr,
>>+                                               char *buf)
>>+{
>>+      struct bonding *bond = to_bond(d);
>>+
>>+      if (BOND_MODE(bond) == BOND_MODE_8023AD)
>>+              return sprintf(buf, "%pM\n", bond->params.ad_actor_sys_macaddr);
>>+
>>+      return 0;
>>+}
>>+static DEVICE_ATTR(ad_actor_system_mac_address, S_IRUGO | S_IWUSR,
>>+                 bonding_show_ad_actor_sys_macaddr,
>>+                 bonding_sysfs_store_option);
>>+
>> static struct attribute *per_bond_attrs[] = {
>>       &dev_attr_slaves.attr,
>>       &dev_attr_mode.attr,
>>@@ -740,6 +755,7 @@ static struct attribute *per_bond_attrs[] = {
>>       &dev_attr_packets_per_slave.attr,
>>       &dev_attr_tlb_dynamic_lb.attr,
>>       &dev_attr_ad_actor_system_priority.attr,
>>+      &dev_attr_ad_actor_system_mac_address.attr,
>>       NULL,
>> };
>>
>>diff --git a/include/net/bond_options.h b/include/net/bond_options.h
>>index c2af1db37354..993ef73cd050 100644
>>--- a/include/net/bond_options.h
>>+++ b/include/net/bond_options.h
>>@@ -64,6 +64,7 @@ enum {
>>       BOND_OPT_SLAVES,
>>       BOND_OPT_TLB_DYNAMIC_LB,
>>       BOND_OPT_AD_ACTOR_SYSPRIO,
>>+      BOND_OPT_AD_ACTOR_SYS_MACADDR,
>>       BOND_OPT_LAST
>> };
>>
>>diff --git a/include/net/bonding.h b/include/net/bonding.h
>>index 7a5c79fcf866..cd2092e6bc71 100644
>>--- a/include/net/bonding.h
>>+++ b/include/net/bonding.h
>>@@ -144,6 +144,7 @@ struct bond_params {
>>       int tlb_dynamic_lb;
>>       struct reciprocal_value reciprocal_packets_per_slave;
>>       u16 ad_actor_sysprio;
>>+      u8 ad_actor_sys_macaddr[ETH_ALEN];
>> };
>>
>> struct bond_parm_tbl {
>>--
>>2.2.0.rc0.207.ga3a616c
>>
>
> ---
>         -Jay Vosburgh, jay.vosburgh@canonical.com

^ permalink raw reply

* Re: [PATCH next 5/6] bonding: Allow userspace to set macaddr on bonding-dev
From: Maciej Żenczykowski @ 2015-02-07  1:22 UTC (permalink / raw)
  To: Jay Vosburgh
  Cc: Mahesh Bandewar, Andy Gospodarek, Veaceslav Falico,
	Nikolay Aleksandrov, David Miller, netdev, Eric Dumazet
In-Reply-To: <4192.1423272023@famine>

The bonding master's MAC address is visible to the entire L2 domain
while the LACP frames are link local.

Acked-by: Maciej Żenczykowski <maze@google.com>

^ permalink raw reply

* Re: [PATCH next 1/6] bonding: Verify RX LACPDU has proper dest mac-addr
From: Maciej Żenczykowski @ 2015-02-07  1:21 UTC (permalink / raw)
  To: Mahesh Bandewar
  Cc: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
	Nikolay Aleksandrov, David Miller, netdev, Eric Dumazet
In-Reply-To: <1423270307-9139-1-git-send-email-maheshb@google.com>

Acked-by: Maciej Żenczykowski <maze@google.com>

On Fri, Feb 6, 2015 at 4:51 PM, Mahesh Bandewar <maheshb@google.com> wrote:
> The 802.1AX standard states:
> "The DA in LACPDUs is the Slow_Protocols_Multicast address."
>
> This patch enforces that and drops LACPDUs with destination MAC
> addresses other than Slow_Protocols_Multicast address
>
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> ---
>  drivers/net/bonding/bond_3ad.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> index cfc4a9c1000a..9b436696b95e 100644
> --- a/drivers/net/bonding/bond_3ad.c
> +++ b/drivers/net/bonding/bond_3ad.c
> @@ -2485,6 +2485,9 @@ int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
>         if (skb->protocol != PKT_TYPE_LACPDU)
>                 return RX_HANDLER_ANOTHER;
>
> +       if (!MAC_ADDRESS_EQUAL(eth_hdr(skb)->h_dest, lacpdu_mcast_addr))
> +               return RX_HANDLER_ANOTHER;
> +
>         lacpdu = skb_header_pointer(skb, 0, sizeof(_lacpdu), &_lacpdu);
>         if (!lacpdu)
>                 return RX_HANDLER_ANOTHER;
> --
> 2.2.0.rc0.207.ga3a616c
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH next 5/6] bonding: Allow userspace to set macaddr on bonding-dev
From: Jay Vosburgh @ 2015-02-07  1:20 UTC (permalink / raw)
  To: Mahesh Bandewar
  Cc: Andy Gospodarek, Veaceslav Falico, Nikolay Aleksandrov,
	David Miller, netdev, Eric Dumazet
In-Reply-To: <1423270316-9311-1-git-send-email-maheshb@google.com>

Mahesh Bandewar <maheshb@google.com> wrote:

>This patch allows user-space to set the mac-address on the bonding device.
>This mac-address can not be NULL or a Multicast. If the mac-address is set
>from user-space; kernel will honor it and will not overwrite it. In the
>absense (value from user space); the logic will default to using the
>masters' mac as the mac address for the bonding device.
>
>It can be set using example code below -
>
>   # modprobe bonding mode=4
>   # sys_mac_addr=$(printf '%02x:%02x:%02x:%02x:%02x:%02x' \
>                    $(( (RANDOM & 0xFE) | 0x02 )) \
>                    $(( RANDOM & 0xFF )) \
>                    $(( RANDOM & 0xFF )) \
>                    $(( RANDOM & 0xFF )) \
>                    $(( RANDOM & 0xFF )) \
>                    $(( RANDOM & 0xFF )))
>   # echo $sys_mac_addr > /sys/class/net/bond0/bonding/ad_actor_system_mac_address
>   # echo +eth1 > /sys/class/net/bond0/bonding/slaves
>   ...
>   # ip link set bond0 up

	How is this patch functionally different from setting the
bonding master's MAC address to a particular value prior to adding any
slaves?

	-J

>Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>---
> drivers/net/bonding/bond_3ad.c     |  7 ++++++-
> drivers/net/bonding/bond_main.c    |  1 +
> drivers/net/bonding/bond_options.c | 29 +++++++++++++++++++++++++++++
> drivers/net/bonding/bond_procfs.c  |  6 ++++++
> drivers/net/bonding/bond_sysfs.c   | 16 ++++++++++++++++
> include/net/bond_options.h         |  1 +
> include/net/bonding.h              |  1 +
> 7 files changed, 60 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index 1177f96194dd..373d3db3809f 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -1914,7 +1914,12 @@ void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution)
> 
> 		BOND_AD_INFO(bond).system.sys_priority =
> 			bond->params.ad_actor_sysprio;
>-		BOND_AD_INFO(bond).system.sys_mac_addr = *((struct mac_addr *)bond->dev->dev_addr);
>+		if (is_zero_ether_addr(bond->params.ad_actor_sys_macaddr))
>+			BOND_AD_INFO(bond).system.sys_mac_addr =
>+			    *((struct mac_addr *)bond->dev->dev_addr);
>+		else
>+			BOND_AD_INFO(bond).system.sys_mac_addr =
>+			    *((struct mac_addr *)bond->params.ad_actor_sys_macaddr);
> 
> 		/* initialize how many times this module is called in one
> 		 * second (should be about every 100ms)
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 561b2bde5aeb..1a6735ef2ea7 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -4440,6 +4440,7 @@ static int bond_check_params(struct bond_params *params)
> 	params->packets_per_slave = packets_per_slave;
> 	params->tlb_dynamic_lb = 1; /* Default value */
> 	params->ad_actor_sysprio = ad_actor_sysprio;
>+	eth_zero_addr(params->ad_actor_sys_macaddr);
> 	if (packets_per_slave > 0) {
> 		params->reciprocal_packets_per_slave =
> 			reciprocal_value(packets_per_slave);
>diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>index d8f6760143ae..330d48b6a1e6 100644
>--- a/drivers/net/bonding/bond_options.c
>+++ b/drivers/net/bonding/bond_options.c
>@@ -72,6 +72,8 @@ static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
> 				  const struct bond_opt_value *newval);
> static int bond_option_ad_actor_sysprio_set(struct bonding *bond,
> 				  const struct bond_opt_value *newval);
>+static int bond_option_ad_actor_sys_macaddr_set(struct bonding *bond,
>+				  const struct bond_opt_value *newval);
> 
> 
> static const struct bond_opt_value bond_mode_tbl[] = {
>@@ -396,6 +398,13 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
> 		.values = bond_ad_actor_sysprio_tbl,
> 		.set = bond_option_ad_actor_sysprio_set,
> 	},
>+	[BOND_OPT_AD_ACTOR_SYS_MACADDR] = {
>+		.id = BOND_OPT_AD_ACTOR_SYS_MACADDR,
>+		.name = "ad_actor_system_mac_address",
>+		.unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
>+		.flags = BOND_OPTFLAG_RAWVAL | BOND_OPTFLAG_IFDOWN,
>+		.set = bond_option_ad_actor_sys_macaddr_set,
>+	},
> };
> 
> /* Searches for an option by name */
>@@ -1376,3 +1385,23 @@ static int bond_option_ad_actor_sysprio_set(struct bonding *bond,
> 	bond->params.ad_actor_sysprio = newval->value;
> 	return 0;
> }
>+
>+static int bond_option_ad_actor_sys_macaddr_set(struct bonding *bond,
>+					    const struct bond_opt_value *newval)
>+{
>+	u8 macaddr[ETH_ALEN];
>+	int i;
>+
>+	i = sscanf(newval->string, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
>+		   &macaddr[0], &macaddr[1], &macaddr[2],
>+		   &macaddr[3], &macaddr[4], &macaddr[5]);
>+
>+	if (i != ETH_ALEN || !is_valid_ether_addr(macaddr)) {
>+		netdev_err(bond->dev, "Invalid MAC address.\n");
>+		return -EINVAL;
>+	}
>+
>+	ether_addr_copy(bond->params.ad_actor_sys_macaddr, macaddr);
>+
>+	return 0;
>+}
>diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
>index 9e33c48886ef..81452ced852f 100644
>--- a/drivers/net/bonding/bond_procfs.c
>+++ b/drivers/net/bonding/bond_procfs.c
>@@ -136,6 +136,8 @@ static void bond_info_show_master(struct seq_file *seq)
> 			   optval->string);
> 		seq_printf(seq, "System priority: %d\n",
> 				BOND_AD_INFO(bond).system.sys_priority);
>+		seq_printf(seq, "System MAC address: %pM\n",
>+				&BOND_AD_INFO(bond).system.sys_mac_addr);
> 
> 		if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
> 			seq_printf(seq, "bond %s has no active aggregator\n",
>@@ -198,6 +200,8 @@ static void bond_info_show_slave(struct seq_file *seq,
> 			seq_puts(seq, "details actor lacp pdu:\n");
> 			seq_printf(seq, "    system priority: %d\n",
> 				   port->actor_system_priority);
>+			seq_printf(seq, "    system mac address: %pM\n",
>+				   &port->actor_system);
> 			seq_printf(seq, "    port key: %d\n",
> 				   port->actor_oper_port_key);
> 			seq_printf(seq, "    port priority: %d\n",
>@@ -210,6 +214,8 @@ static void bond_info_show_slave(struct seq_file *seq,
> 			seq_puts(seq, "details partner lacp pdu:\n");
> 			seq_printf(seq, "    system priority: %d\n",
> 				   port->partner_oper.system_priority);
>+			seq_printf(seq, "    system mac address: %pM\n",
>+				   &port->partner_oper.system);
> 			seq_printf(seq, "    oper key: %d\n",
> 				   port->partner_oper.key);
> 			seq_printf(seq, "    port priority: %d\n",
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index 4350aa06f867..91713d0b6685 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -706,6 +706,21 @@ static ssize_t bonding_show_ad_actor_sysprio(struct device *d,
> static DEVICE_ATTR(ad_actor_system_priority, S_IRUGO | S_IWUSR,
> 		   bonding_show_ad_actor_sysprio, bonding_sysfs_store_option);
> 
>+static ssize_t bonding_show_ad_actor_sys_macaddr(struct device *d,
>+						 struct device_attribute *attr,
>+						 char *buf)
>+{
>+	struct bonding *bond = to_bond(d);
>+
>+	if (BOND_MODE(bond) == BOND_MODE_8023AD)
>+		return sprintf(buf, "%pM\n", bond->params.ad_actor_sys_macaddr);
>+
>+	return 0;
>+}
>+static DEVICE_ATTR(ad_actor_system_mac_address, S_IRUGO | S_IWUSR,
>+		   bonding_show_ad_actor_sys_macaddr,
>+		   bonding_sysfs_store_option);
>+
> static struct attribute *per_bond_attrs[] = {
> 	&dev_attr_slaves.attr,
> 	&dev_attr_mode.attr,
>@@ -740,6 +755,7 @@ static struct attribute *per_bond_attrs[] = {
> 	&dev_attr_packets_per_slave.attr,
> 	&dev_attr_tlb_dynamic_lb.attr,
> 	&dev_attr_ad_actor_system_priority.attr,
>+	&dev_attr_ad_actor_system_mac_address.attr,
> 	NULL,
> };
> 
>diff --git a/include/net/bond_options.h b/include/net/bond_options.h
>index c2af1db37354..993ef73cd050 100644
>--- a/include/net/bond_options.h
>+++ b/include/net/bond_options.h
>@@ -64,6 +64,7 @@ enum {
> 	BOND_OPT_SLAVES,
> 	BOND_OPT_TLB_DYNAMIC_LB,
> 	BOND_OPT_AD_ACTOR_SYSPRIO,
>+	BOND_OPT_AD_ACTOR_SYS_MACADDR,
> 	BOND_OPT_LAST
> };
> 
>diff --git a/include/net/bonding.h b/include/net/bonding.h
>index 7a5c79fcf866..cd2092e6bc71 100644
>--- a/include/net/bonding.h
>+++ b/include/net/bonding.h
>@@ -144,6 +144,7 @@ struct bond_params {
> 	int tlb_dynamic_lb;
> 	struct reciprocal_value reciprocal_packets_per_slave;
> 	u16 ad_actor_sysprio;
>+	u8 ad_actor_sys_macaddr[ETH_ALEN];
> };
> 
> struct bond_parm_tbl {
>-- 
>2.2.0.rc0.207.ga3a616c
>

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

^ permalink raw reply

* [PATCH next 1/6] bonding: Verify RX LACPDU has proper dest mac-addr
From: Mahesh Bandewar @ 2015-02-07  0:51 UTC (permalink / raw)
  To: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
	Nikolay Aleksandrov, David Miller
  Cc: Mahesh Bandewar, netdev, Eric Dumazet

The 802.1AX standard states:
"The DA in LACPDUs is the Slow_Protocols_Multicast address."

This patch enforces that and drops LACPDUs with destination MAC
addresses other than Slow_Protocols_Multicast address

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
 drivers/net/bonding/bond_3ad.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index cfc4a9c1000a..9b436696b95e 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2485,6 +2485,9 @@ int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
 	if (skb->protocol != PKT_TYPE_LACPDU)
 		return RX_HANDLER_ANOTHER;
 
+	if (!MAC_ADDRESS_EQUAL(eth_hdr(skb)->h_dest, lacpdu_mcast_addr))
+		return RX_HANDLER_ANOTHER;
+
 	lacpdu = skb_header_pointer(skb, 0, sizeof(_lacpdu), &_lacpdu);
 	if (!lacpdu)
 		return RX_HANDLER_ANOTHER;
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* [PATCH next 6/6] bonding: Implement user key part of port_key in an AD system.
From: Mahesh Bandewar @ 2015-02-07  0:51 UTC (permalink / raw)
  To: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
	Nikolay Aleksandrov, David Miller
  Cc: Mahesh Bandewar, netdev, Eric Dumazet

The port key has three components - user-key, speed-part, and duplex-part.
The LSBit is for the duplex-part, next 5 bits are for the speed while the
remaining 10 bits are the user defined key bits. Get these 10 bits
from the user-space (through the SysFs interface) and use it to form the
admin port-key. Allowed range for the user-key is 0 - 1023 (10 bits). If
it is not provided then use zero for the user-key-bits (default).

It can set using following example code -

   # modprobe bonding mode=4
   # usr_port_key=$(( RANDOM & 0x3FF ))
   # echo $usr_port_key > /sys/class/net/bond0/bonding/ad_actor_user_port_key
   # echo +eth1 > /sys/class/net/bond0/bonding/slaves
   ...
   # ip link set bond0 up

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
 drivers/net/bonding/bond_3ad.c     |  6 +++---
 drivers/net/bonding/bond_main.c    | 10 ++++++++++
 drivers/net/bonding/bond_options.c | 26 ++++++++++++++++++++++++++
 drivers/net/bonding/bond_sysfs.c   | 15 +++++++++++++++
 include/net/bond_options.h         |  1 +
 include/net/bonding.h              |  1 +
 6 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 373d3db3809f..c69c393ba8c5 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -1955,10 +1955,10 @@ void bond_3ad_bind_slave(struct slave *slave)
 
 		port->slave = slave;
 		port->actor_port_number = SLAVE_AD_INFO(slave)->id;
-		/* key is determined according to the link speed, duplex and user key(which
-		 * is yet not supported)
+		/* key is determined according to the link speed, duplex and
+		 * user key
 		 */
-		port->actor_admin_port_key = 0;
+		port->actor_admin_port_key = bond->params.ad_actor_portkey << 6;
 		port->actor_admin_port_key |= __get_duplex(port);
 		port->actor_admin_port_key |= (__get_link_speed(port) << 1);
 		port->actor_oper_port_key = port->actor_admin_port_key;
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 1a6735ef2ea7..f9f001f35a91 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4105,6 +4105,7 @@ static int bond_check_params(struct bond_params *params)
 	const struct bond_opt_value *valptr;
 	int arp_all_targets_value;
 	u16 ad_actor_sysprio = 0;
+	u16 ad_actor_portkey = 0;
 
 	/* Convert string parameters. */
 	if (mode) {
@@ -4409,6 +4410,14 @@ static int bond_check_params(struct bond_params *params)
 			return -EINVAL;
 		}
 		ad_actor_sysprio = valptr->value;
+
+		valptr = bond_opt_parse(bond_opt_get(BOND_OPT_AD_ACTOR_PORTKEY),
+					&newval);
+		if (!valptr) {
+			pr_err("Error: No ad_actor_portkey default value");
+			return -EINVAL;
+		}
+		ad_actor_portkey = valptr->value;
 	}
 
 	if (lp_interval == 0) {
@@ -4441,6 +4450,7 @@ static int bond_check_params(struct bond_params *params)
 	params->tlb_dynamic_lb = 1; /* Default value */
 	params->ad_actor_sysprio = ad_actor_sysprio;
 	eth_zero_addr(params->ad_actor_sys_macaddr);
+	params->ad_actor_portkey = ad_actor_portkey;
 	if (packets_per_slave > 0) {
 		params->reciprocal_packets_per_slave =
 			reciprocal_value(packets_per_slave);
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 330d48b6a1e6..c3f77c6d0d2e 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -74,6 +74,8 @@ static int bond_option_ad_actor_sysprio_set(struct bonding *bond,
 				  const struct bond_opt_value *newval);
 static int bond_option_ad_actor_sys_macaddr_set(struct bonding *bond,
 				  const struct bond_opt_value *newval);
+static int bond_option_ad_actor_portkey_set(struct bonding *bond,
+				  const struct bond_opt_value *newval);
 
 
 static const struct bond_opt_value bond_mode_tbl[] = {
@@ -196,6 +198,12 @@ static const struct bond_opt_value bond_ad_actor_sysprio_tbl[] = {
 	{ NULL,      -1,    0},
 };
 
+static const struct bond_opt_value bond_ad_actor_portkey_tbl[] = {
+	{ "minval",  0,     BOND_VALFLAG_MIN | BOND_VALFLAG_DEFAULT},
+	{ "maxval",  1023,  BOND_VALFLAG_MAX},
+	{ NULL,      -1,    0},
+};
+
 static const struct bond_option bond_opts[BOND_OPT_LAST] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -405,6 +413,14 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
 		.flags = BOND_OPTFLAG_RAWVAL | BOND_OPTFLAG_IFDOWN,
 		.set = bond_option_ad_actor_sys_macaddr_set,
 	},
+	[BOND_OPT_AD_ACTOR_PORTKEY] = {
+		.id = BOND_OPT_AD_ACTOR_PORTKEY,
+		.name = "ad_actor_user_port_key",
+		.unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
+		.flags = BOND_OPTFLAG_IFDOWN,
+		.values = bond_ad_actor_portkey_tbl,
+		.set = bond_option_ad_actor_portkey_set,
+	}
 };
 
 /* Searches for an option by name */
@@ -1405,3 +1421,13 @@ static int bond_option_ad_actor_sys_macaddr_set(struct bonding *bond,
 
 	return 0;
 }
+
+static int bond_option_ad_actor_portkey_set(struct bonding *bond,
+					    const struct bond_opt_value *newval)
+{
+	netdev_info(bond->dev, "Setting ad_actor_portkey to (%llu)\n",
+		    newval->value);
+
+	bond->params.ad_actor_portkey = newval->value;
+	return 0;
+}
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 91713d0b6685..17d84d5a1608 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -721,6 +721,20 @@ static DEVICE_ATTR(ad_actor_system_mac_address, S_IRUGO | S_IWUSR,
 		   bonding_show_ad_actor_sys_macaddr,
 		   bonding_sysfs_store_option);
 
+static ssize_t bonding_show_ad_actor_portkey(struct device *d,
+					     struct device_attribute *attr,
+					     char *buf)
+{
+	struct bonding *bond = to_bond(d);
+
+	if (BOND_MODE(bond) == BOND_MODE_8023AD)
+		return sprintf(buf, "%hu\n", bond->params.ad_actor_portkey);
+
+	return 0;
+}
+static DEVICE_ATTR(ad_actor_user_port_key, S_IRUGO | S_IWUSR,
+		   bonding_show_ad_actor_portkey, bonding_sysfs_store_option);
+
 static struct attribute *per_bond_attrs[] = {
 	&dev_attr_slaves.attr,
 	&dev_attr_mode.attr,
@@ -756,6 +770,7 @@ static struct attribute *per_bond_attrs[] = {
 	&dev_attr_tlb_dynamic_lb.attr,
 	&dev_attr_ad_actor_system_priority.attr,
 	&dev_attr_ad_actor_system_mac_address.attr,
+	&dev_attr_ad_actor_user_port_key.attr,
 	NULL,
 };
 
diff --git a/include/net/bond_options.h b/include/net/bond_options.h
index 993ef73cd050..4c36455aacb4 100644
--- a/include/net/bond_options.h
+++ b/include/net/bond_options.h
@@ -65,6 +65,7 @@ enum {
 	BOND_OPT_TLB_DYNAMIC_LB,
 	BOND_OPT_AD_ACTOR_SYSPRIO,
 	BOND_OPT_AD_ACTOR_SYS_MACADDR,
+	BOND_OPT_AD_ACTOR_PORTKEY,
 	BOND_OPT_LAST
 };
 
diff --git a/include/net/bonding.h b/include/net/bonding.h
index cd2092e6bc71..e91db2566437 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -144,6 +144,7 @@ struct bond_params {
 	int tlb_dynamic_lb;
 	struct reciprocal_value reciprocal_packets_per_slave;
 	u16 ad_actor_sysprio;
+	u16 ad_actor_portkey;
 	u8 ad_actor_sys_macaddr[ETH_ALEN];
 };
 
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* [PATCH next 5/6] bonding: Allow userspace to set macaddr on bonding-dev
From: Mahesh Bandewar @ 2015-02-07  0:51 UTC (permalink / raw)
  To: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
	Nikolay Aleksandrov, David Miller
  Cc: Mahesh Bandewar, netdev, Eric Dumazet

This patch allows user-space to set the mac-address on the bonding device.
This mac-address can not be NULL or a Multicast. If the mac-address is set
from user-space; kernel will honor it and will not overwrite it. In the
absense (value from user space); the logic will default to using the
masters' mac as the mac address for the bonding device.

It can be set using example code below -

   # modprobe bonding mode=4
   # sys_mac_addr=$(printf '%02x:%02x:%02x:%02x:%02x:%02x' \
                    $(( (RANDOM & 0xFE) | 0x02 )) \
                    $(( RANDOM & 0xFF )) \
                    $(( RANDOM & 0xFF )) \
                    $(( RANDOM & 0xFF )) \
                    $(( RANDOM & 0xFF )) \
                    $(( RANDOM & 0xFF )))
   # echo $sys_mac_addr > /sys/class/net/bond0/bonding/ad_actor_system_mac_address
   # echo +eth1 > /sys/class/net/bond0/bonding/slaves
   ...
   # ip link set bond0 up

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
 drivers/net/bonding/bond_3ad.c     |  7 ++++++-
 drivers/net/bonding/bond_main.c    |  1 +
 drivers/net/bonding/bond_options.c | 29 +++++++++++++++++++++++++++++
 drivers/net/bonding/bond_procfs.c  |  6 ++++++
 drivers/net/bonding/bond_sysfs.c   | 16 ++++++++++++++++
 include/net/bond_options.h         |  1 +
 include/net/bonding.h              |  1 +
 7 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 1177f96194dd..373d3db3809f 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -1914,7 +1914,12 @@ void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution)
 
 		BOND_AD_INFO(bond).system.sys_priority =
 			bond->params.ad_actor_sysprio;
-		BOND_AD_INFO(bond).system.sys_mac_addr = *((struct mac_addr *)bond->dev->dev_addr);
+		if (is_zero_ether_addr(bond->params.ad_actor_sys_macaddr))
+			BOND_AD_INFO(bond).system.sys_mac_addr =
+			    *((struct mac_addr *)bond->dev->dev_addr);
+		else
+			BOND_AD_INFO(bond).system.sys_mac_addr =
+			    *((struct mac_addr *)bond->params.ad_actor_sys_macaddr);
 
 		/* initialize how many times this module is called in one
 		 * second (should be about every 100ms)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 561b2bde5aeb..1a6735ef2ea7 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4440,6 +4440,7 @@ static int bond_check_params(struct bond_params *params)
 	params->packets_per_slave = packets_per_slave;
 	params->tlb_dynamic_lb = 1; /* Default value */
 	params->ad_actor_sysprio = ad_actor_sysprio;
+	eth_zero_addr(params->ad_actor_sys_macaddr);
 	if (packets_per_slave > 0) {
 		params->reciprocal_packets_per_slave =
 			reciprocal_value(packets_per_slave);
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index d8f6760143ae..330d48b6a1e6 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -72,6 +72,8 @@ static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
 				  const struct bond_opt_value *newval);
 static int bond_option_ad_actor_sysprio_set(struct bonding *bond,
 				  const struct bond_opt_value *newval);
+static int bond_option_ad_actor_sys_macaddr_set(struct bonding *bond,
+				  const struct bond_opt_value *newval);
 
 
 static const struct bond_opt_value bond_mode_tbl[] = {
@@ -396,6 +398,13 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
 		.values = bond_ad_actor_sysprio_tbl,
 		.set = bond_option_ad_actor_sysprio_set,
 	},
+	[BOND_OPT_AD_ACTOR_SYS_MACADDR] = {
+		.id = BOND_OPT_AD_ACTOR_SYS_MACADDR,
+		.name = "ad_actor_system_mac_address",
+		.unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
+		.flags = BOND_OPTFLAG_RAWVAL | BOND_OPTFLAG_IFDOWN,
+		.set = bond_option_ad_actor_sys_macaddr_set,
+	},
 };
 
 /* Searches for an option by name */
@@ -1376,3 +1385,23 @@ static int bond_option_ad_actor_sysprio_set(struct bonding *bond,
 	bond->params.ad_actor_sysprio = newval->value;
 	return 0;
 }
+
+static int bond_option_ad_actor_sys_macaddr_set(struct bonding *bond,
+					    const struct bond_opt_value *newval)
+{
+	u8 macaddr[ETH_ALEN];
+	int i;
+
+	i = sscanf(newval->string, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
+		   &macaddr[0], &macaddr[1], &macaddr[2],
+		   &macaddr[3], &macaddr[4], &macaddr[5]);
+
+	if (i != ETH_ALEN || !is_valid_ether_addr(macaddr)) {
+		netdev_err(bond->dev, "Invalid MAC address.\n");
+		return -EINVAL;
+	}
+
+	ether_addr_copy(bond->params.ad_actor_sys_macaddr, macaddr);
+
+	return 0;
+}
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index 9e33c48886ef..81452ced852f 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -136,6 +136,8 @@ static void bond_info_show_master(struct seq_file *seq)
 			   optval->string);
 		seq_printf(seq, "System priority: %d\n",
 				BOND_AD_INFO(bond).system.sys_priority);
+		seq_printf(seq, "System MAC address: %pM\n",
+				&BOND_AD_INFO(bond).system.sys_mac_addr);
 
 		if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
 			seq_printf(seq, "bond %s has no active aggregator\n",
@@ -198,6 +200,8 @@ static void bond_info_show_slave(struct seq_file *seq,
 			seq_puts(seq, "details actor lacp pdu:\n");
 			seq_printf(seq, "    system priority: %d\n",
 				   port->actor_system_priority);
+			seq_printf(seq, "    system mac address: %pM\n",
+				   &port->actor_system);
 			seq_printf(seq, "    port key: %d\n",
 				   port->actor_oper_port_key);
 			seq_printf(seq, "    port priority: %d\n",
@@ -210,6 +214,8 @@ static void bond_info_show_slave(struct seq_file *seq,
 			seq_puts(seq, "details partner lacp pdu:\n");
 			seq_printf(seq, "    system priority: %d\n",
 				   port->partner_oper.system_priority);
+			seq_printf(seq, "    system mac address: %pM\n",
+				   &port->partner_oper.system);
 			seq_printf(seq, "    oper key: %d\n",
 				   port->partner_oper.key);
 			seq_printf(seq, "    port priority: %d\n",
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 4350aa06f867..91713d0b6685 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -706,6 +706,21 @@ static ssize_t bonding_show_ad_actor_sysprio(struct device *d,
 static DEVICE_ATTR(ad_actor_system_priority, S_IRUGO | S_IWUSR,
 		   bonding_show_ad_actor_sysprio, bonding_sysfs_store_option);
 
+static ssize_t bonding_show_ad_actor_sys_macaddr(struct device *d,
+						 struct device_attribute *attr,
+						 char *buf)
+{
+	struct bonding *bond = to_bond(d);
+
+	if (BOND_MODE(bond) == BOND_MODE_8023AD)
+		return sprintf(buf, "%pM\n", bond->params.ad_actor_sys_macaddr);
+
+	return 0;
+}
+static DEVICE_ATTR(ad_actor_system_mac_address, S_IRUGO | S_IWUSR,
+		   bonding_show_ad_actor_sys_macaddr,
+		   bonding_sysfs_store_option);
+
 static struct attribute *per_bond_attrs[] = {
 	&dev_attr_slaves.attr,
 	&dev_attr_mode.attr,
@@ -740,6 +755,7 @@ static struct attribute *per_bond_attrs[] = {
 	&dev_attr_packets_per_slave.attr,
 	&dev_attr_tlb_dynamic_lb.attr,
 	&dev_attr_ad_actor_system_priority.attr,
+	&dev_attr_ad_actor_system_mac_address.attr,
 	NULL,
 };
 
diff --git a/include/net/bond_options.h b/include/net/bond_options.h
index c2af1db37354..993ef73cd050 100644
--- a/include/net/bond_options.h
+++ b/include/net/bond_options.h
@@ -64,6 +64,7 @@ enum {
 	BOND_OPT_SLAVES,
 	BOND_OPT_TLB_DYNAMIC_LB,
 	BOND_OPT_AD_ACTOR_SYSPRIO,
+	BOND_OPT_AD_ACTOR_SYS_MACADDR,
 	BOND_OPT_LAST
 };
 
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 7a5c79fcf866..cd2092e6bc71 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -144,6 +144,7 @@ struct bond_params {
 	int tlb_dynamic_lb;
 	struct reciprocal_value reciprocal_packets_per_slave;
 	u16 ad_actor_sysprio;
+	u8 ad_actor_sys_macaddr[ETH_ALEN];
 };
 
 struct bond_parm_tbl {
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* [PATCH next 4/6] bonding: Allow userspace to set system_priority
From: Mahesh Bandewar @ 2015-02-07  0:51 UTC (permalink / raw)
  To: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
	Nikolay Aleksandrov, David Miller
  Cc: Mahesh Bandewar, netdev, Eric Dumazet

This patch allows user to randomize the system-priority in an ad-system.
The allowed range is 1 - 0xFFFF while default value is 0xFFFF. If user
does not specify this value, the system defaults to 0xFFFF, which is
what it was before this patch.

Following example code could set the value -
    # modprobe bonding mode=4
    # sys_prio=$(( 1 + RANDOM + RANDOM ))
    # echo $sys_prio > /sys/class/net/bond0/bonding/ad_actor_system_priority
    # echo +eth1 > /sys/class/net/bond0/bonding/slaves
    ...
    # ip link set bond0 up

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
 drivers/net/bonding/bond_3ad.c     |  5 ++++-
 drivers/net/bonding/bond_main.c    | 14 ++++++++++++++
 drivers/net/bonding/bond_options.c | 29 ++++++++++++++++++++++++++++-
 drivers/net/bonding/bond_procfs.c  |  2 ++
 drivers/net/bonding/bond_sysfs.c   | 15 +++++++++++++++
 include/net/bond_options.h         |  1 +
 include/net/bonding.h              |  1 +
 7 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 2a69095266c1..1177f96194dd 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -1912,7 +1912,8 @@ void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution)
 
 		BOND_AD_INFO(bond).aggregator_identifier = 0;
 
-		BOND_AD_INFO(bond).system.sys_priority = 0xFFFF;
+		BOND_AD_INFO(bond).system.sys_priority =
+			bond->params.ad_actor_sysprio;
 		BOND_AD_INFO(bond).system.sys_mac_addr = *((struct mac_addr *)bond->dev->dev_addr);
 
 		/* initialize how many times this module is called in one
@@ -1963,6 +1964,8 @@ void bond_3ad_bind_slave(struct slave *slave)
 			port->sm_vars &= ~AD_PORT_LACP_ENABLED;
 		/* actor system is the bond's system */
 		port->actor_system = BOND_AD_INFO(bond).system.sys_mac_addr;
+		port->actor_system_priority =
+		    BOND_AD_INFO(bond).system.sys_priority;
 		/* tx timer(to verify that no more than MAX_TX_IN_SECOND
 		 * lacpdu's are sent in one second)
 		 */
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index a50ec87486f3..561b2bde5aeb 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4104,6 +4104,7 @@ static int bond_check_params(struct bond_params *params)
 	struct bond_opt_value newval;
 	const struct bond_opt_value *valptr;
 	int arp_all_targets_value;
+	u16 ad_actor_sysprio = 0;
 
 	/* Convert string parameters. */
 	if (mode) {
@@ -4398,6 +4399,18 @@ static int bond_check_params(struct bond_params *params)
 		fail_over_mac_value = BOND_FOM_NONE;
 	}
 
+	if (bond_mode == BOND_MODE_8023AD) {
+		bond_opt_initstr(&newval, "default");
+		valptr = bond_opt_parse(
+				bond_opt_get(BOND_OPT_AD_ACTOR_SYSPRIO),
+					     &newval);
+		if (!valptr) {
+			pr_err("Error: No ad_actor_sysprio default value");
+			return -EINVAL;
+		}
+		ad_actor_sysprio = valptr->value;
+	}
+
 	if (lp_interval == 0) {
 		pr_warn("Warning: ip_interval must be between 1 and %d, so it was reset to %d\n",
 			INT_MAX, BOND_ALB_DEFAULT_LP_INTERVAL);
@@ -4426,6 +4439,7 @@ static int bond_check_params(struct bond_params *params)
 	params->lp_interval = lp_interval;
 	params->packets_per_slave = packets_per_slave;
 	params->tlb_dynamic_lb = 1; /* Default value */
+	params->ad_actor_sysprio = ad_actor_sysprio;
 	if (packets_per_slave > 0) {
 		params->reciprocal_packets_per_slave =
 			reciprocal_value(packets_per_slave);
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 4df28943d222..d8f6760143ae 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -70,6 +70,8 @@ static int bond_option_slaves_set(struct bonding *bond,
 				  const struct bond_opt_value *newval);
 static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
 				  const struct bond_opt_value *newval);
+static int bond_option_ad_actor_sysprio_set(struct bonding *bond,
+				  const struct bond_opt_value *newval);
 
 
 static const struct bond_opt_value bond_mode_tbl[] = {
@@ -186,6 +188,12 @@ static const struct bond_opt_value bond_tlb_dynamic_lb_tbl[] = {
 	{ NULL,  -1, 0}
 };
 
+static const struct bond_opt_value bond_ad_actor_sysprio_tbl[] = {
+	{ "minval",  1,     BOND_VALFLAG_MIN},
+	{ "maxval",  65535, BOND_VALFLAG_MAX | BOND_VALFLAG_DEFAULT},
+	{ NULL,      -1,    0},
+};
+
 static const struct bond_option bond_opts[BOND_OPT_LAST] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -379,7 +387,15 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
 		.values = bond_tlb_dynamic_lb_tbl,
 		.flags = BOND_OPTFLAG_IFDOWN,
 		.set = bond_option_tlb_dynamic_lb_set,
-	}
+	},
+	[BOND_OPT_AD_ACTOR_SYSPRIO] = {
+		.id = BOND_OPT_AD_ACTOR_SYSPRIO,
+		.name = "ad_actor_system_priority",
+		.unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
+		.flags = BOND_OPTFLAG_IFDOWN,
+		.values = bond_ad_actor_sysprio_tbl,
+		.set = bond_option_ad_actor_sysprio_set,
+	},
 };
 
 /* Searches for an option by name */
@@ -1349,3 +1365,14 @@ static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
 
 	return 0;
 }
+
+
+static int bond_option_ad_actor_sysprio_set(struct bonding *bond,
+					    const struct bond_opt_value *newval)
+{
+	netdev_info(bond->dev, "Setting ad_actor_sysprio to (%llu)\n",
+		    newval->value);
+
+	bond->params.ad_actor_sysprio = newval->value;
+	return 0;
+}
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index 83095a0b4b90..9e33c48886ef 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -134,6 +134,8 @@ static void bond_info_show_master(struct seq_file *seq)
 					  bond->params.ad_select);
 		seq_printf(seq, "Aggregator selection policy (ad_select): %s\n",
 			   optval->string);
+		seq_printf(seq, "System priority: %d\n",
+				BOND_AD_INFO(bond).system.sys_priority);
 
 		if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
 			seq_printf(seq, "bond %s has no active aggregator\n",
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 7e9e151d4d61..4350aa06f867 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -692,6 +692,20 @@ static ssize_t bonding_show_packets_per_slave(struct device *d,
 static DEVICE_ATTR(packets_per_slave, S_IRUGO | S_IWUSR,
 		   bonding_show_packets_per_slave, bonding_sysfs_store_option);
 
+static ssize_t bonding_show_ad_actor_sysprio(struct device *d,
+					     struct device_attribute *attr,
+					     char *buf)
+{
+	struct bonding *bond = to_bond(d);
+
+	if (BOND_MODE(bond) == BOND_MODE_8023AD)
+		return sprintf(buf, "%hu\n", bond->params.ad_actor_sysprio);
+
+	return 0;
+}
+static DEVICE_ATTR(ad_actor_system_priority, S_IRUGO | S_IWUSR,
+		   bonding_show_ad_actor_sysprio, bonding_sysfs_store_option);
+
 static struct attribute *per_bond_attrs[] = {
 	&dev_attr_slaves.attr,
 	&dev_attr_mode.attr,
@@ -725,6 +739,7 @@ static struct attribute *per_bond_attrs[] = {
 	&dev_attr_lp_interval.attr,
 	&dev_attr_packets_per_slave.attr,
 	&dev_attr_tlb_dynamic_lb.attr,
+	&dev_attr_ad_actor_system_priority.attr,
 	NULL,
 };
 
diff --git a/include/net/bond_options.h b/include/net/bond_options.h
index ea6546d2c946..c2af1db37354 100644
--- a/include/net/bond_options.h
+++ b/include/net/bond_options.h
@@ -63,6 +63,7 @@ enum {
 	BOND_OPT_LP_INTERVAL,
 	BOND_OPT_SLAVES,
 	BOND_OPT_TLB_DYNAMIC_LB,
+	BOND_OPT_AD_ACTOR_SYSPRIO,
 	BOND_OPT_LAST
 };
 
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 29f53eacac0a..7a5c79fcf866 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -143,6 +143,7 @@ struct bond_params {
 	int packets_per_slave;
 	int tlb_dynamic_lb;
 	struct reciprocal_value reciprocal_packets_per_slave;
+	u16 ad_actor_sysprio;
 };
 
 struct bond_parm_tbl {
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* [PATCH next 3/6] bonding: Implement port churn-machine (AD standard 43.4.17).
From: Mahesh Bandewar @ 2015-02-07  0:51 UTC (permalink / raw)
  To: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
	Nikolay Aleksandrov, David Miller
  Cc: Mahesh Bandewar, netdev, Eric Dumazet

The Chrun Detection machines detect the situation where a port is operable,
but the Actor and Partner have not attached the link to an Aggregator and
brought the link into operation within a bound time period. Under normal
operation of the LACP, aggrement between Actor and Partner should be reached
very rapidly. Continued failure to reach aggrement can be symptomatic of
device failure.

Actor-churn-dection state-machine
=================================

BEGIN=True + PortEnable=False
           |
           v
 +------------------------+   ActorPort.Sync=True  +------------------+
 |   ACTOR_CHURN_MONITOR  | ---------------------> |  NO_ACTOR_CHURN  |
 |========================|                        |==================|
 |    ActorChurn=False    |  ActorPort.Sync=False  | ActorChurn=False |
 | ActorChurn.Timer=Start | <--------------------- |                  |
 +------------------------+                        +------------------+
           |                                                ^
           |                                                |
  ActorChurn.Timer=Expired                                  |
           |                                       ActorPort.Sync=True
           |                                                |
           |                +-----------------+             |
           |                |   ACTOR_CHURN   |             |
           |                |=================|             |
           +--------------> | ActorChurn=True | ------------+
                            |                 |
                            +-----------------+

Similar for the Partner-churn-detection.

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
 drivers/net/bonding/bond_3ad.c    | 56 +++++++++++++++++++++++++++++++++++++--
 drivers/net/bonding/bond_procfs.c | 40 +++++++++++++++++++++++++---
 include/net/bond_3ad.h            | 29 ++++++++++++++++++++
 3 files changed, 119 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 14f2ebe786c5..2a69095266c1 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -38,6 +38,7 @@
 #define AD_STANDBY                 0x2
 #define AD_MAX_TX_IN_SECOND        3
 #define AD_COLLECTOR_MAX_DELAY     0
+#define AD_MONITOR_CHURNED         0x1000
 
 /* Timer definitions (43.4.4 in the 802.3ad standard) */
 #define AD_FAST_PERIODIC_TIME      1
@@ -1013,16 +1014,19 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 	/* check if state machine should change state */
 
 	/* first, check if port was reinitialized */
-	if (port->sm_vars & AD_PORT_BEGIN)
+	if (port->sm_vars & AD_PORT_BEGIN) {
 		port->sm_rx_state = AD_RX_INITIALIZE;
+		port->sm_vars |= AD_MONITOR_CHURNED;
 	/* check if port is not enabled */
-	else if (!(port->sm_vars & AD_PORT_BEGIN)
+	} else if (!(port->sm_vars & AD_PORT_BEGIN)
 		 && !port->is_enabled && !(port->sm_vars & AD_PORT_MOVED))
 		port->sm_rx_state = AD_RX_PORT_DISABLED;
 	/* check if new lacpdu arrived */
 	else if (lacpdu && ((port->sm_rx_state == AD_RX_EXPIRED) ||
 		 (port->sm_rx_state == AD_RX_DEFAULTED) ||
 		 (port->sm_rx_state == AD_RX_CURRENT))) {
+		if (port->sm_rx_state != AD_RX_CURRENT)
+			port->sm_vars |= AD_MONITOR_CHURNED;
 		port->sm_rx_timer_counter = 0;
 		port->sm_rx_state = AD_RX_CURRENT;
 	} else {
@@ -1100,9 +1104,11 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			 */
 			port->partner_oper.port_state &= ~AD_STATE_SYNCHRONIZATION;
 			port->sm_vars &= ~AD_PORT_MATCHED;
+			port->partner_oper.port_state |= AD_STATE_LACP_TIMEOUT;
 			port->partner_oper.port_state |= AD_STATE_LACP_ACTIVITY;
 			port->sm_rx_timer_counter = __ad_timer_to_ticks(AD_CURRENT_WHILE_TIMER, (u16)(AD_SHORT_TIMEOUT));
 			port->actor_oper_port_state |= AD_STATE_EXPIRED;
+			port->sm_vars |= AD_MONITOR_CHURNED;
 			break;
 		case AD_RX_DEFAULTED:
 			__update_default_selected(port);
@@ -1131,6 +1137,44 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 	}
 }
 
+/* ad_churn_machine - handle port churn's state machine
+ * @port: the port we're looking at
+ *
+ */
+static void ad_churn_machine(struct port *port)
+{
+	if (port->sm_vars & AD_MONITOR_CHURNED) {
+		port->sm_vars &= ~AD_MONITOR_CHURNED;
+		port->sm_churn_actor_state = AD_CHURN_MONITOR;
+		port->sm_churn_partner_state = AD_CHURN_MONITOR;
+		port->sm_churn_actor_timer_counter =
+			__ad_timer_to_ticks(AD_ACTOR_CHURN_TIMER, 0);
+		 port->sm_churn_partner_timer_counter =
+			 __ad_timer_to_ticks(AD_PARTNER_CHURN_TIMER, 0);
+		return;
+	}
+	if (port->sm_churn_actor_timer_counter &&
+		!(--port->sm_churn_actor_timer_counter) &&
+		(port->sm_churn_actor_state == AD_CHURN_MONITOR)) {
+		if (port->actor_oper_port_state & AD_STATE_SYNCHRONIZATION) {
+			port->sm_churn_actor_state = AD_NO_CHURN;
+		} else {
+			port->churn_actor_count++;
+			port->sm_churn_actor_state = AD_CHURN;
+		}
+	}
+	if (port->sm_churn_partner_timer_counter &&
+		!(--port->sm_churn_partner_timer_counter) &&
+		(port->sm_churn_partner_state == AD_CHURN_MONITOR)) {
+		if (port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION) {
+			port->sm_churn_partner_state = AD_NO_CHURN;
+		} else {
+			port->churn_partner_count++;
+			port->sm_churn_partner_state = AD_CHURN;
+		}
+	}
+}
+
 /**
  * ad_tx_machine - handle a port's tx state machine
  * @port: the port we're looking at
@@ -1745,6 +1789,13 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
 		port->next_port_in_aggregator = NULL;
 		port->transaction_id = 0;
 
+		port->sm_churn_actor_timer_counter = 0;
+		port->sm_churn_actor_state = 0;
+		port->churn_actor_count = 0;
+		port->sm_churn_partner_timer_counter = 0;
+		port->sm_churn_partner_state = 0;
+		port->churn_partner_count = 0;
+
 		memcpy(&port->lacpdu, &lacpdu, sizeof(lacpdu));
 	}
 }
@@ -2164,6 +2215,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 		ad_port_selection_logic(port, &update_slave_arr);
 		ad_mux_machine(port, &update_slave_arr);
 		ad_tx_machine(port);
+		ad_churn_machine(port);
 
 		/* turn off the BEGIN bit, since we already handled it */
 		if (port->sm_vars & AD_PORT_BEGIN)
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index 976f5ad2a0f2..83095a0b4b90 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -178,13 +178,45 @@ static void bond_info_show_slave(struct seq_file *seq,
 	seq_printf(seq, "Permanent HW addr: %pM\n", slave->perm_hwaddr);
 
 	if (BOND_MODE(bond) == BOND_MODE_8023AD) {
-		const struct aggregator *agg
-			= SLAVE_AD_INFO(slave)->port.aggregator;
+		const struct port *port = &SLAVE_AD_INFO(slave)->port;
+		const struct aggregator *agg = port->aggregator;
 
-		if (agg)
+		if (agg) {
 			seq_printf(seq, "Aggregator ID: %d\n",
 				   agg->aggregator_identifier);
-		else
+			seq_printf(seq, "Actor Churn State: %s\n",
+				   bond_3ad_churn_desc(port->sm_churn_actor_state));
+			seq_printf(seq, "Partner Churn State: %s\n",
+				   bond_3ad_churn_desc(port->sm_churn_partner_state));
+			seq_printf(seq, "Actor Churned Count: %d\n",
+				   port->churn_actor_count);
+			seq_printf(seq, "Partner Churned Count: %d\n",
+				   port->churn_partner_count);
+
+			seq_puts(seq, "details actor lacp pdu:\n");
+			seq_printf(seq, "    system priority: %d\n",
+				   port->actor_system_priority);
+			seq_printf(seq, "    port key: %d\n",
+				   port->actor_oper_port_key);
+			seq_printf(seq, "    port priority: %d\n",
+				   port->actor_port_priority);
+			seq_printf(seq, "    port number: %d\n",
+				   port->actor_port_number);
+			seq_printf(seq, "    port state: %d\n",
+				   port->actor_oper_port_state);
+
+			seq_puts(seq, "details partner lacp pdu:\n");
+			seq_printf(seq, "    system priority: %d\n",
+				   port->partner_oper.system_priority);
+			seq_printf(seq, "    oper key: %d\n",
+				   port->partner_oper.key);
+			seq_printf(seq, "    port priority: %d\n",
+				   port->partner_oper.port_priority);
+			seq_printf(seq, "    port number: %d\n",
+				   port->partner_oper.port_number);
+			seq_printf(seq, "    port state: %d\n",
+				   port->partner_oper.port_state);
+		} else
 			seq_puts(seq, "Aggregator ID: N/A\n");
 	}
 	seq_printf(seq, "Slave queue ID: %d\n", slave->queue_id);
diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
index 6c455c646d61..170f8926ff66 100644
--- a/include/net/bond_3ad.h
+++ b/include/net/bond_3ad.h
@@ -82,6 +82,13 @@ typedef enum {
 	AD_TRANSMIT		/* tx Machine */
 } tx_states_t;
 
+/* churn machine states(43.4.17 in the 802.3ad standard) */
+typedef enum {
+	 AD_CHURN_MONITOR, /* monitoring for churn */
+	 AD_CHURN,         /* churn detected (error) */
+	 AD_NO_CHURN       /* no churn (no error) */
+} churn_state_t;
+
 /* rx indication types */
 typedef enum {
 	AD_TYPE_LACPDU = 1,	/* type lacpdu */
@@ -229,6 +236,12 @@ typedef struct port {
 	u16 sm_mux_timer_counter;	/* state machine mux timer counter */
 	tx_states_t sm_tx_state;	/* state machine tx state */
 	u16 sm_tx_timer_counter;	/* state machine tx timer counter(allways on - enter to transmit state 3 time per second) */
+	u16 sm_churn_actor_timer_counter;
+	u16 sm_churn_partner_timer_counter;
+	u32 churn_actor_count;
+	u32 churn_partner_count;
+	churn_state_t sm_churn_actor_state;
+	churn_state_t sm_churn_partner_state;
 	struct slave *slave;		/* pointer to the bond slave that this port belongs to */
 	struct aggregator *aggregator;	/* pointer to an aggregator that this port related to */
 	struct port *next_port_in_aggregator;	/* Next port on the linked list of the parent aggregator */
@@ -262,6 +275,22 @@ struct ad_slave_info {
 	u16 id;
 };
 
+static inline const char *bond_3ad_churn_desc(churn_state_t state)
+{
+	static const char *const churn_description[] =
+		{ "monitoring",
+		  "churned",
+		  "none",
+		  "unknown"
+		};
+	int max_size = sizeof(churn_description) / sizeof(churn_description[0]);
+
+	if (state >= max_size)
+		state = max_size - 1;
+
+	return churn_description[state];
+}
+
 /* ========== AD Exported functions to the main bonding code ========== */
 void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution);
 void bond_3ad_bind_slave(struct slave *slave);
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* [PATCH next 2/6] bonding: implement bond_poll_controller()
From: Mahesh Bandewar @ 2015-02-07  0:51 UTC (permalink / raw)
  To: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
	Nikolay Aleksandrov, David Miller
  Cc: Mahesh Bandewar, netdev, Eric Dumazet

This patches implements the poll_controller support for all
bonding driver. If the slaves have poll_controller net_op defined,
this implementation calls them. This is mode agnostic implementation
and iterates through all slaves (based on mode) and calls respective
handler.

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
 drivers/net/bonding/bond_3ad.c  | 24 +++++++++++++++++++++
 drivers/net/bonding/bond_main.c | 47 +++++++++++++++++++++++++++++++++++++++++
 include/net/bond_3ad.h          |  1 +
 3 files changed, 72 insertions(+)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 9b436696b95e..14f2ebe786c5 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2477,6 +2477,30 @@ int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info)
 	return ret;
 }
 
+#define BOND_3AD_PORT_OPERATIONAL \
+		(AD_STATE_DISTRIBUTING | AD_STATE_COLLECTING | \
+		 AD_STATE_SYNCHRONIZATION | AD_STATE_AGGREGATION)
+
+static int bond_3ad_port_operational(struct slave *slave)
+{
+	port_t *port = &SLAVE_AD_INFO(slave)->port;
+
+	return bond_slave_can_tx(slave) &&
+	       (port->actor_oper_port_state & port->partner_oper.port_state &
+		BOND_3AD_PORT_OPERATIONAL) == BOND_3AD_PORT_OPERATIONAL;
+}
+
+/* bond_3ad_port_is_active - check if a slave port is active or not. A port
+ * is active when it can forward traffic.
+ *
+ * @slave: slave port to check state for.
+ * Returns: 0 if not active else is active.
+ */
+int bond_3ad_port_is_active(struct slave *slave)
+{
+	return bond_3ad_port_operational(slave);
+}
+
 int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
 			 struct slave *slave)
 {
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index c9e519cb9214..a50ec87486f3 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -928,6 +928,53 @@ static inline void slave_disable_netpoll(struct slave *slave)
 
 static void bond_poll_controller(struct net_device *bond_dev)
 {
+	struct bonding *bond = netdev_priv(bond_dev);
+	struct slave *slave = NULL;
+	struct list_head *iter;
+	struct ad_info ad_info;
+	struct aggregator *agg;
+	const struct net_device_ops *ops;
+	bool call_slave_netpoll;
+
+	if (BOND_MODE(bond) == BOND_MODE_8023AD)
+		if (bond_3ad_get_active_agg_info(bond, &ad_info))
+			return;
+
+	bond_for_each_slave(bond, slave, iter) {
+		call_slave_netpoll = false;
+		switch (BOND_MODE(bond)) {
+		case BOND_MODE_8023AD:
+			agg = SLAVE_AD_INFO(slave)->port.aggregator;
+			if (!bond_slave_is_up(slave))
+				break;
+			if (agg && agg->aggregator_identifier !=
+				ad_info.aggregator_id)
+				break;
+			if (!bond_3ad_port_is_active(slave) &&
+			    ad_info.ports != 1)
+				break;
+
+			call_slave_netpoll = true;
+			break;
+		default:
+			if (bond_slave_is_up(slave))
+				call_slave_netpoll = true;
+			break;
+		}
+
+		if (call_slave_netpoll) {
+			ops = slave->dev->netdev_ops;
+			if (ops->ndo_poll_controller) {
+				struct netpoll_info *ni =
+					rcu_dereference_bh(slave->dev->npinfo);
+
+				if (down_trylock(&ni->dev_lock))
+					continue;
+				ops->ndo_poll_controller(slave->dev);
+				up(&ni->dev_lock);
+			}
+		}
+	}
 }
 
 static void bond_netpoll_cleanup(struct net_device *bond_dev)
diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
index f04cdbb7848e..6c455c646d61 100644
--- a/include/net/bond_3ad.h
+++ b/include/net/bond_3ad.h
@@ -278,5 +278,6 @@ int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
 			 struct slave *slave);
 int bond_3ad_set_carrier(struct bonding *bond);
 void bond_3ad_update_lacp_rate(struct bonding *bond);
+int bond_3ad_port_is_active(struct slave *slave);
 #endif /* _NET_BOND_3AD_H */
 
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* Re: panic on boot with latest net-next
From: Josh Hunt @ 2015-02-06 23:42 UTC (permalink / raw)
  To: Thomas Graf; +Cc: herbert, netdev
In-Reply-To: <54D53C11.1000208@akamai.com>

On 02/06/2015 04:11 PM, Josh Hunt wrote:
> On 02/06/2015 04:10 PM, Thomas Graf wrote:
>> On 02/06/15 at 03:02pm, Josh Hunt wrote:
>>> I'm hitting the following crash on boot with the latest net-next
>>> (2ca292d968ef20cb04f31192d1f626bd8d782960):
>>
>> Thanks for the report.
>>
>> This should be fixed by the patches posted in the thread:
>> [PATCH 0/6 v2 net-next] rhashtable fixes
>
> Thanks. I will apply those and see if it resolves my issue.

Applying those fixed my problem. I also just pulled the latest net-next 
with this series applied and that works as well.

Thanks
Josh

^ permalink raw reply

* Re: [PATCH net-next] rhashtable: Fix remove logic to avoid cross references between buckets
From: David Miller @ 2015-02-06 23:20 UTC (permalink / raw)
  To: tgraf; +Cc: ying.xue, netdev, herbert
In-Reply-To: <20150206160843.GA31371@casper.infradead.org>

From: Thomas Graf <tgraf@suug.ch>
Date: Fri, 6 Feb 2015 16:08:43 +0000

> The remove logic properly searched the remaining chain for a matching
> entry with an identical hash but it did this while searching from both
> the old and new table. Instead in order to not leave stale references
> behind we need to:
> 
>  1. When growing and searching from the new table:
>     Search remaining chain for entry with same hash to avoid having
>     the new table directly point to a entry with a different hash.
> 
>  2. When shrinking and searching from the old table:
>     Check if the element after the removed would create a cross
>     reference and avoid it if so.
> 
> These bugs were present from the beginning in nft_hash.
> 
> Also, both insert functions calculated the hash based on the mask of
> the new table. This worked while growing. Wwhile shrinking, the mask
> of the inew table is smaller than the mask of the old table. This lead
> to a bit not being taken into account when selecting the bucket lock
> and thus caused the wrong bucket to be locked eventually.
> 
> Fixes: 7e1e77636e36 ("lib: Resizable, Scalable, Concurrent Hash Table")
> Fixes: 97defe1ecf86 ("rhashtable: Per bucket locks & deferred expansion/shrinking")
> Reported-by: Ying Xue <ying.xue@windriver.com>
> Signed-off-by: Thomas Graf <tgraf@suug.ch>

Applied.

^ permalink raw reply

* Re: [PATCH 0/6 v2 net-next] rhashtable fixes
From: David Miller @ 2015-02-06 23:20 UTC (permalink / raw)
  To: tgraf; +Cc: netdev, herbert, ying.xue
In-Reply-To: <cover.1423097592.git.tgraf@suug.ch>

From: Thomas Graf <tgraf@suug.ch>
Date: Thu,  5 Feb 2015 02:03:30 +0100

> This series fixes all remaining known issues with rhashtable that
> have been reported. In particular the race condition reported by
> Ying Xue.
> 
> ---
> Dave/Herbert: I'm posting this now as it fixes real issues. I'm
> fine with taking Herbert's rehash patches instead if they resolve
> all the reported issues as well.
> 
> v2:
>  - Rebased on top of Herbert Xu's iterator code
>  - Fixed last remaining race that remained. Special thanks
>    to Daniel Borkmann for assistance while debugging.

Since this supposedly fixes crashes on bootup for some people
I'm applying this series now.

Herbert, I apologize in advance if this makes your work more
difficult but I asked you yesterday what you wanted me to
do with this series...

Thanks.

^ permalink raw reply

* Re: [PATCH] cipso: don't use IPCB() to locate the CIPSO IP option
From: Casey Schaufler @ 2015-02-06 22:51 UTC (permalink / raw)
  To: Paul Moore, davem; +Cc: linux-security-module, selinux, netdev, Casey Schaufler
In-Reply-To: <79075962.e3G0NVBHuh@sifl>

On 2/6/2015 12:03 PM, Paul Moore wrote:
> On Friday, February 06, 2015 02:57:28 PM Paul Moore wrote:
>> Using the IPCB() macro to get the IPv4 options is convenient, but
>> unfortunately NetLabel often needs to examine the CIPSO option outside
>> of the scope of the IP layer in the stack.  While historically IPCB()
>> worked above the IP layer, due to the inclusion of the inet_skb_param
>> struct at the head of the {tcp,udp}_skb_cb structs, recent commit
>> 971f10ec ("tcp: better TCP_SKB_CB layout to reduce cache line misses")
>> reordered the tcp_skb_cb struct and invalidated this IPCB() trick.
>>
>> This patch fixes the problem by creating a new function,
>> cipso_v4_optptr(), which locates the CIPSO option inside the IP header
>> without calling IPCB().  Unfortunately, this isn't as fast as a simple
>> lookup so some additional tweaks were made to limit the use of this
>> new function.
>>
>> Cc: <stable@vger.kernel.org> # 3.18
>> Reported-by: Casey Schaufler <casey@schaufler-ca.com>
>> Signed-off-by: Paul Moore <pmoore@redhat.com>

Tested-by: Casey Schaufler <casey@schaufler-ca.com>
 

> DaveM, I'd prefer this go upstream via the SELinux/security tree so we don't 
> have to worry about syncing up with the netdev tree to get this fix.  Any 
> objections on your part (this patch only touches NetLabel/CIPSO)?
>


^ permalink raw reply

* Re: [PATCH] cipso: don't use IPCB() to locate the CIPSO IP option
From: David Miller @ 2015-02-06 22:27 UTC (permalink / raw)
  To: pmoore; +Cc: linux-security-module, selinux, casey, netdev
In-Reply-To: <79075962.e3G0NVBHuh@sifl>

From: Paul Moore <pmoore@redhat.com>
Date: Fri, 06 Feb 2015 15:03:05 -0500

> On Friday, February 06, 2015 02:57:28 PM Paul Moore wrote:
>> Using the IPCB() macro to get the IPv4 options is convenient, but
>> unfortunately NetLabel often needs to examine the CIPSO option outside
>> of the scope of the IP layer in the stack.  While historically IPCB()
>> worked above the IP layer, due to the inclusion of the inet_skb_param
>> struct at the head of the {tcp,udp}_skb_cb structs, recent commit
>> 971f10ec ("tcp: better TCP_SKB_CB layout to reduce cache line misses")
>> reordered the tcp_skb_cb struct and invalidated this IPCB() trick.
>> 
>> This patch fixes the problem by creating a new function,
>> cipso_v4_optptr(), which locates the CIPSO option inside the IP header
>> without calling IPCB().  Unfortunately, this isn't as fast as a simple
>> lookup so some additional tweaks were made to limit the use of this
>> new function.
>> 
>> Cc: <stable@vger.kernel.org> # 3.18
>> Reported-by: Casey Schaufler <casey@schaufler-ca.com>
>> Signed-off-by: Paul Moore <pmoore@redhat.com>
> 
> DaveM, I'd prefer this go upstream via the SELinux/security tree so we don't 
> have to worry about syncing up with the netdev tree to get this fix.  Any 
> objections on your part (this patch only touches NetLabel/CIPSO)?

No objections, please take it.

^ permalink raw reply

* Re: [PATCH] ath9k_htc: add adaptive usb receive flow control to repair soft lockup with monitor mode
From: Oleksij Rempel @ 2015-02-06 22:24 UTC (permalink / raw)
  To: yuweizheng, linux-kernel, ath9k-devel, linux-wireless, kvalo,
	ath9k-devel
  Cc: netdev, zhengyuwei
In-Reply-To: <1423219598-17982-1-git-send-email-yuweizheng@139.com>

[-- Attachment #1: Type: text/plain, Size: 14700 bytes --]

formatting of this patch is still badly broken.
./scripts/checkpatch.pl ~/Downloads/patch.eml
....
ERROR: DOS line endings
#442: FILE: drivers/net/wireless/ath/ath9k/htc_drv_txrx.c:1170:
+^I^IAURFC_STAT_SET(aurfc_submit_delay, delay);^M$

total: 195 errors, 6 warnings, 1 checks, 310 lines checked


Am 06.02.2015 um 11:46 schrieb yuweizheng@139.com:
> From: Yuwei Zheng <yuweizheng@139.com>
> 
> In the environment with heavy wifi traffic, set the ar9271 into monitor mode, will
> trigger a deadloop panic.
>  
> The ath9k_hif_usb_rx_cb function excute on  the interrupt context, and ath9k_rx_tasklet excute
> on the soft irq context. In other words, the ath9k_hif_usb_rx_cb have more chance to excute than
> ath9k_rx_tasklet.  So in the worst condition,  the rx.rxbuf receive list is always full,
> and the do {}while(true) loop will not be break. The kernel get a soft lockup panic. 
>  
> [59011.007210] BUG: soft lockup - CPU#0 stuck for 23s!
> [kworker/0:0:30609]
> [59011.030560] BUG: scheduling while atomic: kworker/0:0/30609/0x40010100
> [59013.804486] BUG: scheduling while atomic: kworker/0:0/30609/0x40010100
> [59013.858522] Kernel panic - not syncing: softlockup: hung tasks
>  
> [59014.038891] Exception stack(0xdf4bbc38 to 0xdf4bbc80)
> [59014.046834] bc20:                                                       de57b950 60000113
> [59014.059579] bc40: 00000000 bb32bb32 60000113 de57b948 de57b500 dc7bb440 df4bbcd0 00000000
> [59014.072337] bc60: de57b950 60000113 df4bbcd0 df4bbc80 c04c259d c04c25a0 60000133 ffffffff
> [59014.085233] [<c04c28db>] (__irq_svc+0x3b/0x5c) from [<c04c25a0>] (_raw_spin_unlock_irqrestore+0xc/0x10)
> [59014.100437] [<c04c25a0>] (_raw_spin_unlock_irqrestore+0xc/0x10) from [<bf9c2089>] (ath9k_rx_tasklet+0x290/0x490 [ath9k_htc])
> [59014.118267] [<bf9c2089>] (ath9k_rx_tasklet+0x290/0x490 [ath9k_htc]) from [<c0036d23>] (tasklet_action+0x3b/0x98)
> [59014.134132] [<c0036d23>] (tasklet_action+0x3b/0x98) from [<c0036709>] (__do_softirq+0x99/0x16c)
> [59014.147784] [<c0036709>] (__do_softirq+0x99/0x16c) from [<c00369f7>] (irq_exit+0x5b/0x5c)
> [59014.160653] [<c00369f7>] (irq_exit+0x5b/0x5c) from [<c000cfc3>] (handle_IRQ+0x37/0x78)
> [59014.173124] [<c000cfc3>] (handle_IRQ+0x37/0x78) from [<c00085df>] (omap3_intc_handle_irq+0x5f/0x68)
> [59014.187225] [<c00085df>] (omap3_intc_handle_irq+0x5f/0x68) from [<c04c28db>](__irq_svc+0x3b/0x5c)
>  
> This bug can be see with low performance board, such as uniprocessor beagle bone board. 
>  
> Signed-off-by: Yuwei Zheng <yuweizheng@139.com>
> 
> ---
>  drivers/net/wireless/ath/ath9k/hif_usb.c       | 74 +++++++++++++++++++++++---
>  drivers/net/wireless/ath/ath9k/hif_usb.h       |  9 ++++
>  drivers/net/wireless/ath/ath9k/htc.h           | 19 +++++++
>  drivers/net/wireless/ath/ath9k/htc_drv_debug.c | 53 ++++++++++++++++++
>  drivers/net/wireless/ath/ath9k/htc_drv_txrx.c  | 44 +++++++++++++++
>  5 files changed, 192 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
> index 8e7153b..90ee568 100644
> --- a/drivers/net/wireless/ath/ath9k/hif_usb.c
> +++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
> @@ -640,6 +640,7 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
>  	struct hif_device_usb *hif_dev =
>  		usb_get_intfdata(usb_ifnum_to_if(urb->dev, 0));
>  	int ret;
> +	int delay;
>  
>  	if (!skb)
>  		return;
> @@ -658,7 +659,6 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
>  	default:
>  		goto resubmit;
>  	}
> -
>  	if (likely(urb->actual_length != 0)) {
>  		skb_put(skb, urb->actual_length);
>  		ath9k_hif_usb_rx_stream(hif_dev, skb);
> @@ -667,12 +667,22 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
>  resubmit:
>  	skb_reset_tail_pointer(skb);
>  	skb_trim(skb, 0);
> -
> -	usb_anchor_urb(urb, &hif_dev->rx_submitted);
> -	ret = usb_submit_urb(urb, GFP_ATOMIC);
> -	if (ret) {
> -		usb_unanchor_urb(urb);
> -		goto free;
> +	spin_lock(&hif_dev->aurfc_lock);
> +	if (atomic_read(&hif_dev->aurfc_submit_delay) > 0 &&
> +	    hif_dev->aurfc_active == 1) {
> +		usb_anchor_urb(urb, &hif_dev->rx_delayed_submitted);
> +		delay = atomic_read(&hif_dev->aurfc_submit_delay);
> +		schedule_delayed_work(&hif_dev->aurfc_delayed_work,
> +				      msecs_to_jiffies(delay));
> +		spin_unlock(&hif_dev->aurfc_lock);
> +	} else {
> +		spin_unlock(&hif_dev->aurfc_lock);
> +		usb_anchor_urb(urb, &hif_dev->rx_submitted);
> +		ret = usb_submit_urb(urb, GFP_ATOMIC);
> +		if (ret) {
> +			usb_unanchor_urb(urb);
> +			goto free;
> +		}
>  	}
>  
>  	return;
> @@ -818,9 +828,50 @@ err:
>  	return -ENOMEM;
>  }
>  
> +static void aurfc_submit_handler(struct work_struct *work)
> +{
> +	struct hif_device_usb *hif_dev =
> +		container_of(work,
> +			     struct hif_device_usb,
> +			     aurfc_delayed_work.work);
> +
> +	struct urb *urb = NULL;
> +	struct sk_buff *skb = NULL;
> +	int ret;
> +	int loop_times = 0;
> +
> +	while (true) {
> +		loop_times++;
> +		if (loop_times > MAX_RX_URB_NUM)
> +			atomic_add(AURFC_STEP,
> +				   &hif_dev->aurfc_submit_delay);
> +
> +		urb = usb_get_from_anchor(
> +				&hif_dev->rx_delayed_submitted);
> +		if (urb) {
> +			skb = (struct sk_buff *)urb->context;
> +			ret = usb_submit_urb(urb, GFP_KERNEL);
> +			if (ret != 0) {
> +				usb_unanchor_urb(urb);
> +				dev_kfree_skb_any(skb);
> +				urb->context = NULL;
> +			}
> +		} else {
> +			break;
> +		}
> +	}
> +}
> +
>  static void ath9k_hif_usb_dealloc_rx_urbs(struct hif_device_usb *hif_dev)
>  {
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&hif_dev->aurfc_lock, flags);
> +	hif_dev->aurfc_active = 0;
> +	mod_delayed_work(system_wq, &hif_dev->aurfc_delayed_work, 0);
> +	spin_unlock_irqrestore(&hif_dev->aurfc_lock, flags);
>  	usb_kill_anchored_urbs(&hif_dev->rx_submitted);
> +	usb_kill_anchored_urbs(&hif_dev->rx_delayed_submitted);
>  }
>  
>  static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
> @@ -830,8 +881,17 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
>  	int i, ret;
>  
>  	init_usb_anchor(&hif_dev->rx_submitted);
> +	init_usb_anchor(&hif_dev->rx_delayed_submitted);
> +
>  	spin_lock_init(&hif_dev->rx_lock);
>  
> +	/* add for adaptive usb receive control*/
> +	atomic_set(&hif_dev->aurfc_submit_delay, 0);
> +	INIT_DELAYED_WORK(&hif_dev->aurfc_delayed_work,
> +			  aurfc_submit_handler);
> +	spin_lock_init(&hif_dev->aurfc_lock);
> +	hif_dev->aurfc_active = 1;
> +
>  	for (i = 0; i < MAX_RX_URB_NUM; i++) {
>  
>  		/* Allocate URB */
> diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h b/drivers/net/wireless/ath/ath9k/hif_usb.h
> index 51496e7..2050a74 100644
> --- a/drivers/net/wireless/ath/ath9k/hif_usb.h
> +++ b/drivers/net/wireless/ath/ath9k/hif_usb.h
> @@ -41,6 +41,7 @@
>  #define MAX_RX_URB_NUM  8
>  #define MAX_RX_BUF_SIZE 16384
>  #define MAX_PKT_NUM_IN_TRANSFER 10
> +#define AURFC_STEP 10  /* ms */
>  
>  #define MAX_REG_OUT_URB_NUM  1
>  #define MAX_REG_IN_URB_NUM   64
> @@ -98,9 +99,17 @@ struct hif_device_usb {
>  	struct hif_usb_tx tx;
>  	struct usb_anchor regout_submitted;
>  	struct usb_anchor rx_submitted;
> +	struct usb_anchor rx_delayed_submitted;
>  	struct usb_anchor reg_in_submitted;
>  	struct usb_anchor mgmt_submitted;
>  	struct sk_buff *remain_skb;
> +
> +	/* adaptive usb receive flow control */
> +	struct delayed_work aurfc_delayed_work;
> +	spinlock_t aurfc_lock; /* to protect work */
> +	atomic_t  aurfc_submit_delay; /* ms */
> +	int aurfc_active;
> +
>  	const char *fw_name;
>  	int rx_remain_len;
>  	int rx_pkt_len;
> diff --git a/drivers/net/wireless/ath/ath9k/htc.h b/drivers/net/wireless/ath/ath9k/htc.h
> index 9dde265..1586bd2 100644
> --- a/drivers/net/wireless/ath/ath9k/htc.h
> +++ b/drivers/net/wireless/ath/ath9k/htc.h
> @@ -331,6 +331,13 @@ static inline struct ath9k_htc_tx_ctl *HTC_SKB_CB(struct sk_buff *skb)
>  
>  #define TX_QSTAT_INC(q) (priv->debug.tx_stats.queue_stats[q]++)
>  
> +#define AURFC_STAT_INC(c) \
> +	(hif_dev->htc_handle->drv_priv->debug.aurfc_stats.c++)
> +#define AURFC_STAT_ADD(c, a) \
> +	(hif_dev->htc_handle->drv_priv->debug.aurfc_stats.c += a)
> +#define AURFC_STAT_SET(c, a) \
> +	(hif_dev->htc_handle->drv_priv->debug.aurfc_stats.c = a)
> +
>  void ath9k_htc_err_stat_rx(struct ath9k_htc_priv *priv,
>  			   struct ath_rx_status *rs);
>  
> @@ -352,11 +359,20 @@ struct ath_skbrx_stats {
>  	u32 skb_dropped;
>  };
>  
> +struct ath_aurfc_stats {
> +	u32 aurfc_highwater;
> +	u32 aurfc_lowwater;
> +	u32 aurfc_wm_triggered;
> +	u32 aurfc_submit_delay;
> +	u32 aurfc_called;
> +};
> +
>  struct ath9k_debug {
>  	struct dentry *debugfs_phy;
>  	struct ath_tx_stats tx_stats;
>  	struct ath_rx_stats rx_stats;
>  	struct ath_skbrx_stats skbrx_stats;
> +	struct ath_aurfc_stats aurfc_stats;
>  };
>  
>  void ath9k_htc_get_et_strings(struct ieee80211_hw *hw,
> @@ -377,6 +393,9 @@ void ath9k_htc_get_et_stats(struct ieee80211_hw *hw,
>  
>  #define TX_QSTAT_INC(c) do { } while (0)
>  
> +#define AURFC_STAT_INC(c) do {} while (0)
> +#define AURFC_STAT_ADD(c, a) do {} while (0)
> +#define AURFC_STAT_SET(c, a) do {} while (0)
>  static inline void ath9k_htc_err_stat_rx(struct ath9k_htc_priv *priv,
>  					 struct ath_rx_status *rs)
>  {
> diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
> index 8cef1ed..a6be9be 100644
> --- a/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
> +++ b/drivers/net/wireless/ath/ath9k/htc_drv_debug.c
> @@ -286,6 +286,54 @@ static const struct file_operations fops_skb_rx = {
>  	.llseek = default_llseek,
>  };
>  
> +static ssize_t read_file_aurfc(struct file *file,
> +			       char __user *user_buf,
> +			       size_t count, loff_t *ppos)
> +{
> +	struct ath9k_htc_priv *priv = file->private_data;
> +	char *buf;
> +	unsigned int len = 0, size = 1500;
> +	ssize_t retval = 0;
> +
> +	buf = kzalloc(size, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "High watermark",
> +			priv->debug.aurfc_stats.aurfc_highwater);
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "Low watermark",
> +			priv->debug.aurfc_stats.aurfc_lowwater);
> +
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "WM triggered",
> +			priv->debug.aurfc_stats.aurfc_wm_triggered);
> +
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "Handler called",
> +			priv->debug.aurfc_stats.aurfc_called);
> +
> +	len += scnprintf(buf + len, size - len,
> +			"%20s : %10u\n", "Submit delay",
> +			priv->debug.aurfc_stats.aurfc_submit_delay);
> +	if (len > size)
> +		len = size;
> +
> +	retval = simple_read_from_buffer(user_buf, count,
> +					 ppos, buf, len);
> +	kfree(buf);
> +
> +	return retval;
> +}
> +
> +static const struct file_operations fops_aurfc = {
> +	.read = read_file_aurfc,
> +	.open = simple_open,
> +	.owner = THIS_MODULE,
> +	.llseek = default_llseek,
> +};
> +
>  static ssize_t read_file_slot(struct file *file, char __user *user_buf,
>  			      size_t count, loff_t *ppos)
>  {
> @@ -518,7 +566,12 @@ int ath9k_htc_init_debug(struct ath_hw *ah)
>  	debugfs_create_file("skb_rx", S_IRUSR, priv->debug.debugfs_phy,
>  			    priv, &fops_skb_rx);
>  
> +	debugfs_create_file("aurfc_stats", S_IRUSR,
> +			    priv->debug.debugfs_phy,
> +			    priv, &fops_aurfc);
> +
>  	ath9k_cmn_debug_recv(priv->debug.debugfs_phy, &priv->debug.rx_stats);
> +
>  	ath9k_cmn_debug_phy_err(priv->debug.debugfs_phy, &priv->debug.rx_stats);
>  
>  	debugfs_create_file("slot", S_IRUSR, priv->debug.debugfs_phy,
> diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> index a0f58e2..939d008 100644
> --- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> +++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> @@ -1061,7 +1061,28 @@ void ath9k_rx_tasklet(unsigned long data)
>  	unsigned long flags;
>  	struct ieee80211_hdr *hdr;
>  
> +	/* add for adaptive usb receive flow control*/
> +	int looptimes = 0;
> +	int highwatermark = ATH9K_HTC_RXBUF*3/4;
> +	int lowwatermark = ATH9K_HTC_RXBUF/32;
> +	unsigned int delay = 0;
> +
> +	struct htc_target *htc = priv->htc;
> +	struct hif_device_usb *hif_dev = htc->hif_dev;
> +
> +	AURFC_STAT_SET(aurfc_highwater, highwatermark);
> +	AURFC_STAT_SET(aurfc_lowwater, lowwatermark);
> +
>  	do {
> +		looptimes++;
> +		if (looptimes > highwatermark) {
> +			delay = looptimes*AURFC_STEP;
> +			atomic_set(&hif_dev->aurfc_submit_delay,
> +				   delay);
> +			AURFC_STAT_INC(aurfc_wm_triggered);
> +			AURFC_STAT_SET(aurfc_submit_delay, delay);
> +		}
> +
>  		spin_lock_irqsave(&priv->rx.rxbuflock, flags);
>  		list_for_each_entry(tmp_buf, &priv->rx.rxbuf, list) {
>  			if (tmp_buf->in_process) {
> @@ -1072,6 +1093,22 @@ void ath9k_rx_tasklet(unsigned long data)
>  
>  		if (rxbuf == NULL) {
>  			spin_unlock_irqrestore(&priv->rx.rxbuflock, flags);
> +			spin_lock_irqsave(&hif_dev->aurfc_lock,
> +					  flags);
> +			if (atomic_read(
> +				&hif_dev->aurfc_submit_delay) > 0 &&
> +			    hif_dev->aurfc_active > 0)
> +				mod_delayed_work(system_wq,
> +					&hif_dev->aurfc_delayed_work,
> +						 0);
> +			spin_unlock_irqrestore(&hif_dev->aurfc_lock,
> +					       flags);
> +			if (looptimes < lowwatermark) {
> +				atomic_set(&hif_dev->aurfc_submit_delay
> +					   , 0);
> +				AURFC_STAT_SET(aurfc_submit_delay,
> +					       0);
> +			}
>  			break;
>  		}
>  
> @@ -1114,6 +1151,10 @@ void ath9k_htc_rxep(void *drv_priv, struct sk_buff *skb,
>  	struct ath_common *common = ath9k_hw_common(ah);
>  	struct ath9k_htc_rxbuf *rxbuf = NULL, *tmp_buf = NULL;
>  
> +	struct htc_target *htc = priv->htc;
> +	struct hif_device_usb *hif_dev = htc->hif_dev;
> +	int delay = ATH9K_HTC_RXBUF * AURFC_STEP;
> +
>  	spin_lock(&priv->rx.rxbuflock);
>  	list_for_each_entry(tmp_buf, &priv->rx.rxbuf, list) {
>  		if (!tmp_buf->in_process) {
> @@ -1124,6 +1165,9 @@ void ath9k_htc_rxep(void *drv_priv, struct sk_buff *skb,
>  	spin_unlock(&priv->rx.rxbuflock);
>  
>  	if (rxbuf == NULL) {
> +		atomic_set(&hif_dev->aurfc_submit_delay, delay);
> +		AURFC_STAT_INC(aurfc_wm_triggered);
> +		AURFC_STAT_SET(aurfc_submit_delay, delay);
>  		ath_dbg(common, ANY, "No free RX buffer\n");
>  		goto err;
>  	}
> 


-- 
Regards,
Oleksij


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

^ permalink raw reply

* Re: [PATCH net-next] net: rfs: add hash collision detection
From: Tom Herbert @ 2015-02-06 22:21 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, Ying Cai, Willem de Bruijn
In-Reply-To: <1423256341.31870.160.camel@edumazet-glaptop2.roam.corp.google.com>

On Fri, Feb 6, 2015 at 12:59 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Receive Flow Steering is a nice solution but suffers from
> hash collisions when a mix of connected and unconnected traffic
> is received on the host, when flow hash table is populated.
>
> Also, clearing flow in inet_release() makes RFS not very good
> for short lived flows, as many packets can follow close().
> (FIN , ACK packets, ...)
>
> This patch extends the information stored into global hash table
> to not only include cpu number, but upper part of the hash value.
>
> I use a 32bit value, and dynamically split it in two parts.
>
> For host with less than 64 possible cpus, this gives 6 bits for the
> cpu number, and 26 (32-6) bits for the upper part of the hash.
>
> Since hash bucket selection use low order bits of the hash, we have
> a full hash match, if /proc/sys/net/core/rps_sock_flow_entries is big
> enough.
>
> If the hash found in flow table does not match, we fallback to RPS (if
> it is enabled for the rxqueue).
>
> This means that a packet for an non connected flow can avoid the
> IPI through a unrelated/victim CPU.
>
> This also means we no longer have to clear the table at socket
> close time, and this helps short lived flows performance.
>

Acked-by: Tom Herbert <therbert@google.com>

Eric, looks awesome! Can you share any performance numbers?

Thanks,
Tom

> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  drivers/net/tun.c          |    5 ---
>  include/linux/netdevice.h  |   34 ++++++++++++------------
>  include/net/sock.h         |   24 -----------------
>  net/core/dev.c             |   48 +++++++++++++++++++----------------
>  net/core/sysctl_net_core.c |    2 -
>  net/ipv4/af_inet.c         |    2 -
>  6 files changed, 47 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index ad7d3d5f3ee5..857dca47bf80 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -256,7 +256,6 @@ static void tun_flow_delete(struct tun_struct *tun, struct tun_flow_entry *e)
>  {
>         tun_debug(KERN_INFO, tun, "delete flow: hash %u index %u\n",
>                   e->rxhash, e->queue_index);
> -       sock_rps_reset_flow_hash(e->rps_rxhash);
>         hlist_del_rcu(&e->hash_link);
>         kfree_rcu(e, rcu);
>         --tun->flow_count;
> @@ -373,10 +372,8 @@ unlock:
>   */
>  static inline void tun_flow_save_rps_rxhash(struct tun_flow_entry *e, u32 hash)
>  {
> -       if (unlikely(e->rps_rxhash != hash)) {
> -               sock_rps_reset_flow_hash(e->rps_rxhash);
> +       if (unlikely(e->rps_rxhash != hash))
>                 e->rps_rxhash = hash;
> -       }
>  }
>
>  /* We try to identify a flow through its rxhash first. The reason that
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index ce784d5018e0..ab3b7cef4638 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -644,39 +644,39 @@ struct rps_dev_flow_table {
>  /*
>   * The rps_sock_flow_table contains mappings of flows to the last CPU
>   * on which they were processed by the application (set in recvmsg).
> + * Each entry is a 32bit value. Upper part is the high order bits
> + * of flow hash, lower part is cpu number.
> + * rps_cpu_mask is used to partition the space, depending on number of
> + * possible cpus : rps_cpu_mask = roundup_pow_of_two(nr_cpu_ids) - 1
> + * For example, if 64 cpus are possible, rps_cpu_mask = 0x3f,
> + * meaning we use 32-6=26 bits for the hash.
>   */
>  struct rps_sock_flow_table {
> -       unsigned int mask;
> -       u16 ents[0];
> +       u32     mask;
> +       u32     ents[0];
>  };
> -#define        RPS_SOCK_FLOW_TABLE_SIZE(_num) (sizeof(struct rps_sock_flow_table) + \
> -    ((_num) * sizeof(u16)))
> +#define        RPS_SOCK_FLOW_TABLE_SIZE(_num) (offsetof(struct rps_sock_flow_table, ents[_num]))
>
>  #define RPS_NO_CPU 0xffff
>
> +extern u32 rps_cpu_mask;
> +extern struct rps_sock_flow_table __rcu *rps_sock_flow_table;
> +
>  static inline void rps_record_sock_flow(struct rps_sock_flow_table *table,
>                                         u32 hash)
>  {
>         if (table && hash) {
> -               unsigned int cpu, index = hash & table->mask;
> +               unsigned int index = hash & table->mask;
> +               u32 val = hash & ~rps_cpu_mask;
>
>                 /* We only give a hint, preemption can change cpu under us */
> -               cpu = raw_smp_processor_id();
> +               val |= raw_smp_processor_id();
>
> -               if (table->ents[index] != cpu)
> -                       table->ents[index] = cpu;
> +               if (table->ents[index] != val)
> +                       table->ents[index] = val;
>         }
>  }
>
> -static inline void rps_reset_sock_flow(struct rps_sock_flow_table *table,
> -                                      u32 hash)
> -{
> -       if (table && hash)
> -               table->ents[hash & table->mask] = RPS_NO_CPU;
> -}
> -
> -extern struct rps_sock_flow_table __rcu *rps_sock_flow_table;
> -
>  #ifdef CONFIG_RFS_ACCEL
>  bool rps_may_expire_flow(struct net_device *dev, u16 rxq_index, u32 flow_id,
>                          u16 filter_id);
> diff --git a/include/net/sock.h b/include/net/sock.h
> index d28b8fededd6..e13824570b0f 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -857,18 +857,6 @@ static inline void sock_rps_record_flow_hash(__u32 hash)
>  #endif
>  }
>
> -static inline void sock_rps_reset_flow_hash(__u32 hash)
> -{
> -#ifdef CONFIG_RPS
> -       struct rps_sock_flow_table *sock_flow_table;
> -
> -       rcu_read_lock();
> -       sock_flow_table = rcu_dereference(rps_sock_flow_table);
> -       rps_reset_sock_flow(sock_flow_table, hash);
> -       rcu_read_unlock();
> -#endif
> -}
> -
>  static inline void sock_rps_record_flow(const struct sock *sk)
>  {
>  #ifdef CONFIG_RPS
> @@ -876,28 +864,18 @@ static inline void sock_rps_record_flow(const struct sock *sk)
>  #endif
>  }
>
> -static inline void sock_rps_reset_flow(const struct sock *sk)
> -{
> -#ifdef CONFIG_RPS
> -       sock_rps_reset_flow_hash(sk->sk_rxhash);
> -#endif
> -}
> -
>  static inline void sock_rps_save_rxhash(struct sock *sk,
>                                         const struct sk_buff *skb)
>  {
>  #ifdef CONFIG_RPS
> -       if (unlikely(sk->sk_rxhash != skb->hash)) {
> -               sock_rps_reset_flow(sk);
> +       if (unlikely(sk->sk_rxhash != skb->hash))
>                 sk->sk_rxhash = skb->hash;
> -       }
>  #endif
>  }
>
>  static inline void sock_rps_reset_rxhash(struct sock *sk)
>  {
>  #ifdef CONFIG_RPS
> -       sock_rps_reset_flow(sk);
>         sk->sk_rxhash = 0;
>  #endif
>  }
> diff --git a/net/core/dev.c b/net/core/dev.c
> index a3a96ffc67f4..8be38675e1a8 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3030,6 +3030,8 @@ static inline void ____napi_schedule(struct softnet_data *sd,
>  /* One global table that all flow-based protocols share. */
>  struct rps_sock_flow_table __rcu *rps_sock_flow_table __read_mostly;
>  EXPORT_SYMBOL(rps_sock_flow_table);
> +u32 rps_cpu_mask __read_mostly;
> +EXPORT_SYMBOL(rps_cpu_mask);
>
>  struct static_key rps_needed __read_mostly;
>
> @@ -3086,16 +3088,17 @@ set_rps_cpu(struct net_device *dev, struct sk_buff *skb,
>  static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
>                        struct rps_dev_flow **rflowp)
>  {
> -       struct netdev_rx_queue *rxqueue;
> -       struct rps_map *map;
> +       const struct rps_sock_flow_table *sock_flow_table;
> +       struct netdev_rx_queue *rxqueue = dev->_rx;
>         struct rps_dev_flow_table *flow_table;
> -       struct rps_sock_flow_table *sock_flow_table;
> +       struct rps_map *map;
>         int cpu = -1;
> -       u16 tcpu;
> +       u32 tcpu;
>         u32 hash;
>
>         if (skb_rx_queue_recorded(skb)) {
>                 u16 index = skb_get_rx_queue(skb);
> +
>                 if (unlikely(index >= dev->real_num_rx_queues)) {
>                         WARN_ONCE(dev->real_num_rx_queues > 1,
>                                   "%s received packet on queue %u, but number "
> @@ -3103,39 +3106,40 @@ static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
>                                   dev->name, index, dev->real_num_rx_queues);
>                         goto done;
>                 }
> -               rxqueue = dev->_rx + index;
> -       } else
> -               rxqueue = dev->_rx;
> +               rxqueue += index;
> +       }
>
> +       /* Avoid computing hash if RFS/RPS is not active for this rxqueue */
> +
> +       flow_table = rcu_dereference(rxqueue->rps_flow_table);
>         map = rcu_dereference(rxqueue->rps_map);
> -       if (map) {
> -               if (map->len == 1 &&
> -                   !rcu_access_pointer(rxqueue->rps_flow_table)) {
> -                       tcpu = map->cpus[0];
> -                       if (cpu_online(tcpu))
> -                               cpu = tcpu;
> -                       goto done;
> -               }
> -       } else if (!rcu_access_pointer(rxqueue->rps_flow_table)) {
> +       if (!flow_table && !map)
>                 goto done;
> -       }
>
>         skb_reset_network_header(skb);
>         hash = skb_get_hash(skb);
>         if (!hash)
>                 goto done;
>
> -       flow_table = rcu_dereference(rxqueue->rps_flow_table);
>         sock_flow_table = rcu_dereference(rps_sock_flow_table);
>         if (flow_table && sock_flow_table) {
> -               u16 next_cpu;
>                 struct rps_dev_flow *rflow;
> +               u32 next_cpu;
> +               u32 ident;
> +
> +               /* First check into global flow table if there is a match */
> +               ident = sock_flow_table->ents[hash & sock_flow_table->mask];
> +               if ((ident ^ hash) & ~rps_cpu_mask)
> +                       goto try_rps;
>
> +               next_cpu = ident & rps_cpu_mask;
> +
> +               /* OK, now we know there is a match,
> +                * we can look at the local (per receive queue) flow table
> +                */
>                 rflow = &flow_table->flows[hash & flow_table->mask];
>                 tcpu = rflow->cpu;
>
> -               next_cpu = sock_flow_table->ents[hash & sock_flow_table->mask];
> -
>                 /*
>                  * If the desired CPU (where last recvmsg was done) is
>                  * different from current CPU (one in the rx-queue flow
> @@ -3162,6 +3166,8 @@ static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
>                 }
>         }
>
> +try_rps:
> +
>         if (map) {
>                 tcpu = map->cpus[reciprocal_scale(hash, map->len)];
>                 if (cpu_online(tcpu)) {
> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> index fde21d19e61b..7a31be5e361f 100644
> --- a/net/core/sysctl_net_core.c
> +++ b/net/core/sysctl_net_core.c
> @@ -65,7 +65,7 @@ static int rps_sock_flow_sysctl(struct ctl_table *table, int write,
>                                         mutex_unlock(&sock_flow_mutex);
>                                         return -ENOMEM;
>                                 }
> -
> +                               rps_cpu_mask = roundup_pow_of_two(nr_cpu_ids) - 1;
>                                 sock_table->mask = size - 1;
>                         } else
>                                 sock_table = orig_sock_table;
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index a44773c8346c..d2e49baaff63 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -395,8 +395,6 @@ int inet_release(struct socket *sock)
>         if (sk) {
>                 long timeout;
>
> -               sock_rps_reset_flow(sk);
> -
>                 /* Applications forget to leave groups before exiting */
>                 ip_mc_drop_socket(sk);
>
>
>

^ permalink raw reply

* Re: panic on boot with latest net-next
From: Josh Hunt @ 2015-02-06 22:11 UTC (permalink / raw)
  To: Thomas Graf; +Cc: herbert, netdev
In-Reply-To: <20150206221028.GA13706@casper.infradead.org>

On 02/06/2015 04:10 PM, Thomas Graf wrote:
> On 02/06/15 at 03:02pm, Josh Hunt wrote:
>> I'm hitting the following crash on boot with the latest net-next
>> (2ca292d968ef20cb04f31192d1f626bd8d782960):
>
> Thanks for the report.
>
> This should be fixed by the patches posted in the thread:
> [PATCH 0/6 v2 net-next] rhashtable fixes

Thanks. I will apply those and see if it resolves my issue.

Josh

^ permalink raw reply

* Re: panic on boot with latest net-next
From: Thomas Graf @ 2015-02-06 22:10 UTC (permalink / raw)
  To: Josh Hunt; +Cc: herbert, netdev
In-Reply-To: <54D52BD6.2080109@akamai.com>

On 02/06/15 at 03:02pm, Josh Hunt wrote:
> I'm hitting the following crash on boot with the latest net-next
> (2ca292d968ef20cb04f31192d1f626bd8d782960):

Thanks for the report.

This should be fixed by the patches posted in the thread:
[PATCH 0/6 v2 net-next] rhashtable fixes

^ permalink raw reply

* RE: [PATCH net-next v4] Add support of Cavium Liquidio ethernet adapters
From: Chickles, Derek @ 2015-02-06 20:28 UTC (permalink / raw)
  To: David Miller, Vatsavayi, Raghu
  Cc: netdev@vger.kernel.org, Burla, Satananda, Manlunas, Felix,
	Vatsavayi, Raghu
In-Reply-To: <20150202.190049.1051229698764700160.davem@davemloft.net>

> From: David Miller [mailto:davem@davemloft.net]
> Sent: Monday, February 02, 2015 7:01 PM
> >  create mode 100644
> drivers/net/ethernet/cavium/liquidio/cn66xx_device.c
>  ...
> >  create mode 100644
> drivers/net/ethernet/cavium/liquidio/cn68xx_device.c
> 
> These two file have several functions which are nearly identical.
> 
> Especially the device init register programming sequence.
> 
> Please consolidate the common code some more.
> 
> THanks.

Hi,

We've now consolidated 66xx and 68xx device init and access functions as
well as ethtool-related redundant stuff. This will come in a new patch
coming shortly.

Is there anything else you'd like to see us address?

Thanks,
Derek

^ permalink raw reply

* [PATCH net-next] Driver: Vmxnet3: Change the hex constant to its decimal equivalent
From: Shrikrishna Khare @ 2015-02-06 21:48 UTC (permalink / raw)
  To: sbhatewara, pv-drivers, netdev, linux-kernel; +Cc: Shrikrishna Khare

The hex constant chosen for VMXNET3_REV1_MAGIC is offensive,
replace it with its decimal equivalent.

Signed-off-by: Shrikrishna Khare <skhare@vmware.com>
Reviewed-by: Shreyas Bhatewara <sbhatewara@vmware.com>
---
 drivers/net/vmxnet3/vmxnet3_defs.h |    2 +-
 drivers/net/vmxnet3/vmxnet3_int.h  |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_defs.h b/drivers/net/vmxnet3/vmxnet3_defs.h
index 25b6fa4..3718d02 100644
--- a/drivers/net/vmxnet3/vmxnet3_defs.h
+++ b/drivers/net/vmxnet3/vmxnet3_defs.h
@@ -393,7 +393,7 @@ struct Vmxnet3_DriverInfo {
 };
 
 
-#define VMXNET3_REV1_MAGIC  0xbabefee1
+#define VMXNET3_REV1_MAGIC  3133079265u
 
 /*
  * QueueDescPA must be 128 bytes aligned. It points to an array of
diff --git a/drivers/net/vmxnet3/vmxnet3_int.h b/drivers/net/vmxnet3/vmxnet3_int.h
index 406144b..cd71c77 100644
--- a/drivers/net/vmxnet3/vmxnet3_int.h
+++ b/drivers/net/vmxnet3/vmxnet3_int.h
@@ -69,10 +69,10 @@
 /*
  * Version numbers
  */
-#define VMXNET3_DRIVER_VERSION_STRING   "1.3.3.0-k"
+#define VMXNET3_DRIVER_VERSION_STRING   "1.3.4.0-k"
 
 /* a 32-bit int, each byte encode a verion number in VMXNET3_DRIVER_VERSION */
-#define VMXNET3_DRIVER_VERSION_NUM      0x01030300
+#define VMXNET3_DRIVER_VERSION_NUM      0x01030400
 
 #if defined(CONFIG_PCI_MSI)
 	/* RSS only makes sense if MSI-X is supported. */
-- 
1.7.4.1

^ permalink raw reply related

* Re: Invalid timestamp? causing tight ack loop (hundreds of thousands of packets / sec)
From: Neal Cardwell @ 2015-02-06 21:16 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Avery Fay, Netdev
In-Reply-To: <1423181871.31870.94.camel@edumazet-glaptop2.roam.corp.google.com>

On Thu, Feb 5, 2015 at 7:17 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2015-02-05 at 15:25 -0800, Avery Fay wrote:
>
>> Are these available anywhere? Or could they be made available? This
>> seems like a much better solution than disabling timestamps.
>
> Neal is currently polishing a serie of 4 patches.
>
> This should be land to netdev shortly ;)

I emailed the patch series to the list. There is a a "[PATCH net-next
0/4] tcp: mitigate TCP ACK loops due to out-of-window validation
dupacks" cover letter with an overview. And you can find the patches
here:

http://patchwork.ozlabs.org/patch/437462/
http://patchwork.ozlabs.org/patch/437463/
http://patchwork.ozlabs.org/patch/437464/
http://patchwork.ozlabs.org/patch/437465/

Please feel free to try these out and let us know how they work for you.

Thanks!

neal

^ permalink raw reply

* [PATCH net-next 4/4] tcp: mitigate ACK loops for connections as tcp_timewait_sock
From: Neal Cardwell @ 2015-02-06 21:04 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Neal Cardwell, Yuchung Cheng, Eric Dumazet
In-Reply-To: <1423256681-31716-1-git-send-email-ncardwell@google.com>

Ensure that in state FIN_WAIT2 or TIME_WAIT, where the connection is
represented by a tcp_timewait_sock, we rate limit dupacks in response
to incoming packets (a) with TCP timestamps that fail PAWS checks, or
(b) with sequence numbers that are out of the acceptable window.

We do not send a dupack in response to out-of-window packets if it has
been less than sysctl_tcp_invalid_ratelimit (default 500ms) since we
last sent a dupack in response to an out-of-window packet.

Reported-by: Avery Fay <avery@mixpanel.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/tcp.h      |  4 ++++
 net/ipv4/tcp_minisocks.c | 29 ++++++++++++++++++++++++-----
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 66d85a8..1a7adb4 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -342,6 +342,10 @@ struct tcp_timewait_sock {
 	u32			  tw_rcv_wnd;
 	u32			  tw_ts_offset;
 	u32			  tw_ts_recent;
+
+	/* The time we sent the last out-of-window ACK: */
+	u32			  tw_last_oow_ack_time;
+
 	long			  tw_ts_recent_stamp;
 #ifdef CONFIG_TCP_MD5SIG
 	struct tcp_md5sig_key	  *tw_md5_key;
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 98a8405..dd11ac7 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -58,6 +58,25 @@ static bool tcp_in_window(u32 seq, u32 end_seq, u32 s_win, u32 e_win)
 	return seq == e_win && seq == end_seq;
 }
 
+static enum tcp_tw_status
+tcp_timewait_check_oow_rate_limit(struct inet_timewait_sock *tw,
+				  const struct sk_buff *skb, int mib_idx)
+{
+	struct tcp_timewait_sock *tcptw = tcp_twsk((struct sock *)tw);
+
+	if (!tcp_oow_rate_limited(twsk_net(tw), skb, mib_idx,
+				  &tcptw->tw_last_oow_ack_time)) {
+		/* Send ACK. Note, we do not put the bucket,
+		 * it will be released by caller.
+		 */
+		return TCP_TW_ACK;
+	}
+
+	/* We are rate-limiting, so just release the tw sock and drop skb. */
+	inet_twsk_put(tw);
+	return TCP_TW_SUCCESS;
+}
+
 /*
  * * Main purpose of TIME-WAIT state is to close connection gracefully,
  *   when one of ends sits in LAST-ACK or CLOSING retransmitting FIN
@@ -116,7 +135,8 @@ tcp_timewait_state_process(struct inet_timewait_sock *tw, struct sk_buff *skb,
 		    !tcp_in_window(TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq,
 				   tcptw->tw_rcv_nxt,
 				   tcptw->tw_rcv_nxt + tcptw->tw_rcv_wnd))
-			return TCP_TW_ACK;
+			return tcp_timewait_check_oow_rate_limit(
+				tw, skb, LINUX_MIB_TCPACKSKIPPEDFINWAIT2);
 
 		if (th->rst)
 			goto kill;
@@ -250,10 +270,8 @@ kill:
 			inet_twsk_schedule(tw, &tcp_death_row, TCP_TIMEWAIT_LEN,
 					   TCP_TIMEWAIT_LEN);
 
-		/* Send ACK. Note, we do not put the bucket,
-		 * it will be released by caller.
-		 */
-		return TCP_TW_ACK;
+		return tcp_timewait_check_oow_rate_limit(
+			tw, skb, LINUX_MIB_TCPACKSKIPPEDTIMEWAIT);
 	}
 	inet_twsk_put(tw);
 	return TCP_TW_SUCCESS;
@@ -289,6 +307,7 @@ void tcp_time_wait(struct sock *sk, int state, int timeo)
 		tcptw->tw_ts_recent	= tp->rx_opt.ts_recent;
 		tcptw->tw_ts_recent_stamp = tp->rx_opt.ts_recent_stamp;
 		tcptw->tw_ts_offset	= tp->tsoffset;
+		tcptw->tw_last_oow_ack_time = 0;
 
 #if IS_ENABLED(CONFIG_IPV6)
 		if (tw->tw_family == PF_INET6) {
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox