Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v1 2/5] ixgbevf: Add a RETA query code
From: Alexander Duyck @ 2015-01-01 18:09 UTC (permalink / raw)
  To: Vlad Zolotarov, netdev
  Cc: gleb, avi, jeffrey.t.kirsher, Don Skidmore, tantilov, Emil S
In-Reply-To: <54A44188.204@cloudius-systems.com>

On 12/31/2014 10:33 AM, Vlad Zolotarov wrote:
>
> On 12/31/14 20:00, Alexander Duyck wrote:
>> I suspect this code is badly broken as it doesn't take several things
>> into account.
>>
>> First the PF redirection table can have values outside of the range
>> supported by the VF.  This is allowed as the VF can set how many bits of
>> the redirection table it actually wants to use.  This is controlled via
>> the PSRTYPE register.  So for example the PF can be running with 4
>> queues, and the VF can run either in single queue or as just a pair of
>> queues.
>>
>> Second you could compress this data much more tightly by taking
>> advantage of the bit widths allowed.  So for everything x540 and older
>> they only use a 4 bit value per entry.  That means you could
>> theoretically stuff 8 entries per u32 instead of just 4.
>
> Compression is nice but I think ethtool expects it in a certain
> format: one entry per byte. And since this patch is targeting the
> ethtool the output format should be as ethtool expects it to be and
> this is what this patch does. However I agree that masking the
> appropriate bits according to PSRTYPE is required. Good catch!

The idea of compression comes into play when you consider there is
significant latency trying to get messages across the mailbox.  By
reducing the number of messages needed to get the redirection table you
should be able to significantly reduce the amount of time needed to
fetch it.  The job of compressing/expanding the values is actually
pretty straight forward when you consider all that should be needed is a
simple loop to perform some shift, and, and or operations.

- Alex

^ permalink raw reply

* Re: [PATCH iproute2 v2] bridge/link: add learning_sync policy flag
From: Stephen Hemminger @ 2015-01-01 18:03 UTC (permalink / raw)
  To: sfeldma; +Cc: netdev, jiri, roopa
In-Reply-To: <1419884407-21998-1-git-send-email-sfeldma@gmail.com>

On Mon, 29 Dec 2014 12:20:07 -0800
sfeldma@gmail.com wrote:

> From: Scott Feldman <sfeldma@gmail.com>
> 
> v2:
> 
> Resending now that the dust has cleared in 3.18 on "self" vs. hwmode debate for
> brport settings.  learning_sync is now set/cleared using "self" qualifier on
> brport.
> 
> v1:
> 
> Add 'learned_sync' flag to turn on/off syncing of learned MAC addresses from
> offload device to bridge's FDB.   Flag is be set/cleared on offload device port
> using "self" qualifier:
> 
>   $ sudo bridge link set dev swp1 learning_sync on self
> 
>   $ bridge -d link show dev swp1
>   2: swp1 state UNKNOWN : <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 master br0 state forwarding priority 32 cost 2
>       hairpin off guard off root_block off fastleave off learning off flood off
>   2: swp1 state UNKNOWN : <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 master br0
>       learning on learning_sync on
> 
> Adds new IFLA_BRPORT_LEARNED_SYNCED attribute for IFLA_PROTINFO on the SELF
> brport.
> 
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>

Applied

^ permalink raw reply

* [PATCH] net: fddi: skfp: smt.c:  Remove unused function
From: Rickard Strandqvist @ 2015-01-01 17:01 UTC (permalink / raw)
  To: netdev, linux-kernel; +Cc: Rickard Strandqvist

Remove the function smt_ifconfig() that is not used anywhere.

This was partially found by using a static code analysis program called cppcheck.

Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
 drivers/net/fddi/skfp/smt.c |   12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/net/fddi/skfp/smt.c b/drivers/net/fddi/skfp/smt.c
index 9edada8..cd78b7c 100644
--- a/drivers/net/fddi/skfp/smt.c
+++ b/drivers/net/fddi/skfp/smt.c
@@ -1736,18 +1736,6 @@ char *addr_to_string(struct fddi_addr *addr)
 }
 #endif
 
-#ifdef	AM29K
-int smt_ifconfig(int argc, char *argv[])
-{
-	if (argc >= 2 && !strcmp(argv[0],"opt_bypass") &&
-	    !strcmp(argv[1],"yes")) {
-		smc->mib.fddiSMTBypassPresent = 1 ;
-		return 0;
-	}
-	return amdfddi_config(0, argc, argv);
-}
-#endif
-
 /*
  * return static mac index
  */
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH] net: ethernet: micrel: ksz884x.c:  Remove some unused functions
From: Rickard Strandqvist @ 2015-01-01 17:00 UTC (permalink / raw)
  To: David S. Miller, Andrew Morton
  Cc: Rickard Strandqvist, Wilfried Klaebe, Eric W. Biederman,
	Benoit Taine, Julia Lawall, Manuel Schölling, Joe Perches,
	netdev, linux-kernel

Removes some functions that are not used anywhere:
hw_w_phy_link_md() hw_r_phy_link_md() hw_w_phy_polarity()
hw_r_phy_polarity() hw_w_phy_crossover() hw_r_phy_crossover()
hw_r_phy_rem_cap() hw_w_phy_auto_neg() hw_r_phy_auto_neg()
hw_r_phy_link_stat() sw_get_addr() port_chk_prio() port_chk_replace_vid()
port_chk_802_1p() port_chk_diffserv() sw_chk_unk_def_port()
sw_cfg_unk_def_port() sw_cfg_chk_unk_def_deliver() sw_cfg_unk_def_deliver()
port_chk_in_filter() port_chk_dis_non_vid() port_cfg_in_filter()
port_cfg_dis_non_vid() port_chk_rmv_tag() port_chk_ins_tag()
port_cfg_rmv_tag() port_cfg_ins_tag() sw_flush_dyn_mac_table() port_cfg_tx()
port_cfg_rx() port_chk_force_flow_ctrl() port_chk_back_pressure()
port_cfg_force_flow_ctrl() port_chk_broad_storm() hw_ena_intr_bit()

This was partially found by using a static code analysis program called cppcheck.

Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
 drivers/net/ethernet/micrel/ksz884x.c |  217 ---------------------------------
 1 file changed, 217 deletions(-)

diff --git a/drivers/net/ethernet/micrel/ksz884x.c b/drivers/net/ethernet/micrel/ksz884x.c
index f1ebed6c..582973f 100644
--- a/drivers/net/ethernet/micrel/ksz884x.c
+++ b/drivers/net/ethernet/micrel/ksz884x.c
@@ -1551,15 +1551,6 @@ static void hw_turn_on_intr(struct ksz_hw *hw, u32 bit)
 		hw_set_intr(hw, hw->intr_mask);
 }
 
-static inline void hw_ena_intr_bit(struct ksz_hw *hw, uint interrupt)
-{
-	u32 read_intr;
-
-	read_intr = readl(hw->io + KS884X_INTERRUPTS_ENABLE);
-	hw->intr_set = read_intr | interrupt;
-	writel(hw->intr_set, hw->io + KS884X_INTERRUPTS_ENABLE);
-}
-
 static inline void hw_read_intr(struct ksz_hw *hw, uint *status)
 {
 	*status = readl(hw->io + KS884X_INTERRUPTS_STATUS);
@@ -2125,12 +2116,6 @@ static inline void port_cfg_broad_storm(struct ksz_hw *hw, int p, int set)
 		KS8842_PORT_CTRL_1_OFFSET, PORT_BROADCAST_STORM, set);
 }
 
-static inline int port_chk_broad_storm(struct ksz_hw *hw, int p)
-{
-	return port_chk(hw, p,
-		KS8842_PORT_CTRL_1_OFFSET, PORT_BROADCAST_STORM);
-}
-
 /* Driver set switch broadcast storm protection at 10% rate. */
 #define BROADCAST_STORM_PROTECTION_RATE	10
 
@@ -2283,24 +2268,6 @@ static inline void port_cfg_back_pressure(struct ksz_hw *hw, int p, int set)
 		KS8842_PORT_CTRL_2_OFFSET, PORT_BACK_PRESSURE, set);
 }
 
-static inline void port_cfg_force_flow_ctrl(struct ksz_hw *hw, int p, int set)
-{
-	port_cfg(hw, p,
-		KS8842_PORT_CTRL_2_OFFSET, PORT_FORCE_FLOW_CTRL, set);
-}
-
-static inline int port_chk_back_pressure(struct ksz_hw *hw, int p)
-{
-	return port_chk(hw, p,
-		KS8842_PORT_CTRL_2_OFFSET, PORT_BACK_PRESSURE);
-}
-
-static inline int port_chk_force_flow_ctrl(struct ksz_hw *hw, int p)
-{
-	return port_chk(hw, p,
-		KS8842_PORT_CTRL_2_OFFSET, PORT_FORCE_FLOW_CTRL);
-}
-
 /* Spanning Tree */
 
 static inline void port_cfg_dis_learn(struct ksz_hw *hw, int p, int set)
@@ -2309,82 +2276,11 @@ static inline void port_cfg_dis_learn(struct ksz_hw *hw, int p, int set)
 		KS8842_PORT_CTRL_2_OFFSET, PORT_LEARN_DISABLE, set);
 }
 
-static inline void port_cfg_rx(struct ksz_hw *hw, int p, int set)
-{
-	port_cfg(hw, p,
-		KS8842_PORT_CTRL_2_OFFSET, PORT_RX_ENABLE, set);
-}
-
-static inline void port_cfg_tx(struct ksz_hw *hw, int p, int set)
-{
-	port_cfg(hw, p,
-		KS8842_PORT_CTRL_2_OFFSET, PORT_TX_ENABLE, set);
-}
-
 static inline void sw_cfg_fast_aging(struct ksz_hw *hw, int set)
 {
 	sw_cfg(hw, KS8842_SWITCH_CTRL_1_OFFSET, SWITCH_FAST_AGING, set);
 }
 
-static inline void sw_flush_dyn_mac_table(struct ksz_hw *hw)
-{
-	if (!(hw->overrides & FAST_AGING)) {
-		sw_cfg_fast_aging(hw, 1);
-		mdelay(1);
-		sw_cfg_fast_aging(hw, 0);
-	}
-}
-
-/* VLAN */
-
-static inline void port_cfg_ins_tag(struct ksz_hw *hw, int p, int insert)
-{
-	port_cfg(hw, p,
-		KS8842_PORT_CTRL_1_OFFSET, PORT_INSERT_TAG, insert);
-}
-
-static inline void port_cfg_rmv_tag(struct ksz_hw *hw, int p, int remove)
-{
-	port_cfg(hw, p,
-		KS8842_PORT_CTRL_1_OFFSET, PORT_REMOVE_TAG, remove);
-}
-
-static inline int port_chk_ins_tag(struct ksz_hw *hw, int p)
-{
-	return port_chk(hw, p,
-		KS8842_PORT_CTRL_1_OFFSET, PORT_INSERT_TAG);
-}
-
-static inline int port_chk_rmv_tag(struct ksz_hw *hw, int p)
-{
-	return port_chk(hw, p,
-		KS8842_PORT_CTRL_1_OFFSET, PORT_REMOVE_TAG);
-}
-
-static inline void port_cfg_dis_non_vid(struct ksz_hw *hw, int p, int set)
-{
-	port_cfg(hw, p,
-		KS8842_PORT_CTRL_2_OFFSET, PORT_DISCARD_NON_VID, set);
-}
-
-static inline void port_cfg_in_filter(struct ksz_hw *hw, int p, int set)
-{
-	port_cfg(hw, p,
-		KS8842_PORT_CTRL_2_OFFSET, PORT_INGRESS_VLAN_FILTER, set);
-}
-
-static inline int port_chk_dis_non_vid(struct ksz_hw *hw, int p)
-{
-	return port_chk(hw, p,
-		KS8842_PORT_CTRL_2_OFFSET, PORT_DISCARD_NON_VID);
-}
-
-static inline int port_chk_in_filter(struct ksz_hw *hw, int p)
-{
-	return port_chk(hw, p,
-		KS8842_PORT_CTRL_2_OFFSET, PORT_INGRESS_VLAN_FILTER);
-}
-
 /* Mirroring */
 
 static inline void port_cfg_mirror_sniffer(struct ksz_hw *hw, int p, int set)
@@ -2422,28 +2318,6 @@ static void sw_init_mirror(struct ksz_hw *hw)
 	sw_cfg_mirror_rx_tx(hw, 0);
 }
 
-static inline void sw_cfg_unk_def_deliver(struct ksz_hw *hw, int set)
-{
-	sw_cfg(hw, KS8842_SWITCH_CTRL_7_OFFSET,
-		SWITCH_UNK_DEF_PORT_ENABLE, set);
-}
-
-static inline int sw_cfg_chk_unk_def_deliver(struct ksz_hw *hw)
-{
-	return sw_chk(hw, KS8842_SWITCH_CTRL_7_OFFSET,
-		SWITCH_UNK_DEF_PORT_ENABLE);
-}
-
-static inline void sw_cfg_unk_def_port(struct ksz_hw *hw, int port, int set)
-{
-	port_cfg_shift(hw, port, KS8842_SWITCH_CTRL_7_OFFSET, 0, set);
-}
-
-static inline int sw_chk_unk_def_port(struct ksz_hw *hw, int port)
-{
-	return port_chk_shift(hw, port, KS8842_SWITCH_CTRL_7_OFFSET, 0);
-}
-
 /* Priority */
 
 static inline void port_cfg_diffserv(struct ksz_hw *hw, int p, int set)
@@ -2470,30 +2344,6 @@ static inline void port_cfg_prio(struct ksz_hw *hw, int p, int set)
 		KS8842_PORT_CTRL_1_OFFSET, PORT_PRIO_QUEUE_ENABLE, set);
 }
 
-static inline int port_chk_diffserv(struct ksz_hw *hw, int p)
-{
-	return port_chk(hw, p,
-		KS8842_PORT_CTRL_1_OFFSET, PORT_DIFFSERV_ENABLE);
-}
-
-static inline int port_chk_802_1p(struct ksz_hw *hw, int p)
-{
-	return port_chk(hw, p,
-		KS8842_PORT_CTRL_1_OFFSET, PORT_802_1P_ENABLE);
-}
-
-static inline int port_chk_replace_vid(struct ksz_hw *hw, int p)
-{
-	return port_chk(hw, p,
-		KS8842_PORT_CTRL_2_OFFSET, PORT_USER_PRIORITY_CEILING);
-}
-
-static inline int port_chk_prio(struct ksz_hw *hw, int p)
-{
-	return port_chk(hw, p,
-		KS8842_PORT_CTRL_1_OFFSET, PORT_PRIO_QUEUE_ENABLE);
-}
-
 /**
  * sw_dis_diffserv - disable switch DiffServ priority
  * @hw: 	The hardware instance.
@@ -2694,23 +2544,6 @@ static void sw_cfg_port_base_vlan(struct ksz_hw *hw, int port, u8 member)
 }
 
 /**
- * sw_get_addr - get the switch MAC address.
- * @hw: 	The hardware instance.
- * @mac_addr:	Buffer to store the MAC address.
- *
- * This function retrieves the MAC address of the switch.
- */
-static inline void sw_get_addr(struct ksz_hw *hw, u8 *mac_addr)
-{
-	int i;
-
-	for (i = 0; i < 6; i += 2) {
-		mac_addr[i] = readb(hw->io + KS8842_MAC_ADDR_0_OFFSET + i);
-		mac_addr[1 + i] = readb(hw->io + KS8842_MAC_ADDR_1_OFFSET + i);
-	}
-}
-
-/**
  * sw_set_addr - configure switch MAC address
  * @hw: 	The hardware instance.
  * @mac_addr:	The MAC address.
@@ -2917,56 +2750,6 @@ static inline void hw_w_phy_ctrl(struct ksz_hw *hw, int phy, u16 data)
 	writew(data, hw->io + phy + KS884X_PHY_CTRL_OFFSET);
 }
 
-static inline void hw_r_phy_link_stat(struct ksz_hw *hw, int phy, u16 *data)
-{
-	*data = readw(hw->io + phy + KS884X_PHY_STATUS_OFFSET);
-}
-
-static inline void hw_r_phy_auto_neg(struct ksz_hw *hw, int phy, u16 *data)
-{
-	*data = readw(hw->io + phy + KS884X_PHY_AUTO_NEG_OFFSET);
-}
-
-static inline void hw_w_phy_auto_neg(struct ksz_hw *hw, int phy, u16 data)
-{
-	writew(data, hw->io + phy + KS884X_PHY_AUTO_NEG_OFFSET);
-}
-
-static inline void hw_r_phy_rem_cap(struct ksz_hw *hw, int phy, u16 *data)
-{
-	*data = readw(hw->io + phy + KS884X_PHY_REMOTE_CAP_OFFSET);
-}
-
-static inline void hw_r_phy_crossover(struct ksz_hw *hw, int phy, u16 *data)
-{
-	*data = readw(hw->io + phy + KS884X_PHY_CTRL_OFFSET);
-}
-
-static inline void hw_w_phy_crossover(struct ksz_hw *hw, int phy, u16 data)
-{
-	writew(data, hw->io + phy + KS884X_PHY_CTRL_OFFSET);
-}
-
-static inline void hw_r_phy_polarity(struct ksz_hw *hw, int phy, u16 *data)
-{
-	*data = readw(hw->io + phy + KS884X_PHY_PHY_CTRL_OFFSET);
-}
-
-static inline void hw_w_phy_polarity(struct ksz_hw *hw, int phy, u16 data)
-{
-	writew(data, hw->io + phy + KS884X_PHY_PHY_CTRL_OFFSET);
-}
-
-static inline void hw_r_phy_link_md(struct ksz_hw *hw, int phy, u16 *data)
-{
-	*data = readw(hw->io + phy + KS884X_PHY_LINK_MD_OFFSET);
-}
-
-static inline void hw_w_phy_link_md(struct ksz_hw *hw, int phy, u16 data)
-{
-	writew(data, hw->io + phy + KS884X_PHY_LINK_MD_OFFSET);
-}
-
 /**
  * hw_r_phy - read data from PHY register
  * @hw: 	The hardware instance.
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH] net: ethernet: chelsio: cxgb3: mc5.c:  Remove some unused functions
From: Rickard Strandqvist @ 2015-01-01 16:49 UTC (permalink / raw)
  To: Santosh Raspatur, netdev; +Cc: Rickard Strandqvist, linux-kernel

Removes some functions that are not used anywhere:
dbgi_rd_rsp3() dbgi_wr_addr3()

This was partially found by using a static code analysis program called cppcheck.

Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
 drivers/net/ethernet/chelsio/cxgb3/mc5.c |   16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb3/mc5.c b/drivers/net/ethernet/chelsio/cxgb3/mc5.c
index e13b7fe..338301b 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/mc5.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/mc5.c
@@ -97,14 +97,6 @@ static int mc5_cmd_write(struct adapter *adapter, u32 cmd)
 			       F_DBGIRSPVALID, 1, MAX_WRITE_ATTEMPTS, 1);
 }
 
-static inline void dbgi_wr_addr3(struct adapter *adapter, u32 v1, u32 v2,
-				 u32 v3)
-{
-	t3_write_reg(adapter, A_MC5_DB_DBGI_REQ_ADDR0, v1);
-	t3_write_reg(adapter, A_MC5_DB_DBGI_REQ_ADDR1, v2);
-	t3_write_reg(adapter, A_MC5_DB_DBGI_REQ_ADDR2, v3);
-}
-
 static inline void dbgi_wr_data3(struct adapter *adapter, u32 v1, u32 v2,
 				 u32 v3)
 {
@@ -113,14 +105,6 @@ static inline void dbgi_wr_data3(struct adapter *adapter, u32 v1, u32 v2,
 	t3_write_reg(adapter, A_MC5_DB_DBGI_REQ_DATA2, v3);
 }
 
-static inline void dbgi_rd_rsp3(struct adapter *adapter, u32 *v1, u32 *v2,
-				u32 *v3)
-{
-	*v1 = t3_read_reg(adapter, A_MC5_DB_DBGI_RSP_DATA0);
-	*v2 = t3_read_reg(adapter, A_MC5_DB_DBGI_RSP_DATA1);
-	*v3 = t3_read_reg(adapter, A_MC5_DB_DBGI_RSP_DATA2);
-}
-
 /*
  * Write data to the TCAM register at address (0, 0, addr_lo) using the TCAM
  * command cmd.  The data to be written must have been set up by the caller.
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH net-next 0/7] Fixing the "Time Counter fixes and improvements"
From: Sedat Dilek @ 2015-01-01 16:30 UTC (permalink / raw)
  To: Richard Cochran; +Cc: David Miller, netdev@vger.kernel.org

Richard wrote:

Dave,

I did not catch the missing includes in my x86 and arm testing,
because those archs somehow do include clocksource.h for the drivers
in question. Sorry.

This is how I would like to fix the header fallout. We really should
decouple the timecounter/cyclecounter code from the clocksource code
where possible.


Thanks,
Richard


Richard Cochran (7):
  timecounter: provide a macro to initialize the cyclecounter mask
    field.
  bnx2x: convert to CYCLECOUNTER_MASK macro.
  e1000e: convert to CYCLECOUNTER_MASK macro.
  igb: convert to CYCLECOUNTER_MASK macro.
  ixgbe: convert to CYCLECOUNTER_MASK macro.
  mlx4: include clocksource.h again
  microblaze: include the new timecounter header.

[...]

With this conversion those 2 commits in net-next.git#master are obsolete now...

e1000e: Include clocksource.h to get CLOCKSOURCE_MASK.
igb_ptp: Include clocksource.h to get CLOCKSOURCE_MASK.


- Sedat -

^ permalink raw reply

* [PATCH] net: wireless: b43legacy: radio.c:  Remove unused function
From: Rickard Strandqvist @ 2015-01-01 15:46 UTC (permalink / raw)
  To: Larry Finger, Stefano Brivio
  Cc: Rickard Strandqvist, Kalle Valo, linux-wireless, b43-dev, netdev,
	linux-kernel

Remove the function b43legacy_radio_set_tx_iq() that is not used anywhere.

This was partially found by using a static code analysis program called cppcheck.

Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
 drivers/net/wireless/b43legacy/radio.c |   19 -------------------
 drivers/net/wireless/b43legacy/radio.h |    1 -
 2 files changed, 20 deletions(-)

diff --git a/drivers/net/wireless/b43legacy/radio.c b/drivers/net/wireless/b43legacy/radio.c
index 8961776..9501420 100644
--- a/drivers/net/wireless/b43legacy/radio.c
+++ b/drivers/net/wireless/b43legacy/radio.c
@@ -1743,25 +1743,6 @@ u16 freq_r3A_value(u16 frequency)
 	return value;
 }
 
-void b43legacy_radio_set_tx_iq(struct b43legacy_wldev *dev)
-{
-	static const u8 data_high[5] = { 0x00, 0x40, 0x80, 0x90, 0xD0 };
-	static const u8 data_low[5]  = { 0x00, 0x01, 0x05, 0x06, 0x0A };
-	u16 tmp = b43legacy_radio_read16(dev, 0x001E);
-	int i;
-	int j;
-
-	for (i = 0; i < 5; i++) {
-		for (j = 0; j < 5; j++) {
-			if (tmp == (data_high[i] | data_low[j])) {
-				b43legacy_phy_write(dev, 0x0069, (i - j) << 8 |
-						    0x00C0);
-				return;
-			}
-		}
-	}
-}
-
 int b43legacy_radio_selectchannel(struct b43legacy_wldev *dev,
 				  u8 channel,
 				  int synthetic_pu_workaround)
diff --git a/drivers/net/wireless/b43legacy/radio.h b/drivers/net/wireless/b43legacy/radio.h
index bccb3d7..dd2976d 100644
--- a/drivers/net/wireless/b43legacy/radio.h
+++ b/drivers/net/wireless/b43legacy/radio.h
@@ -92,7 +92,6 @@ void b43legacy_nrssi_hw_write(struct b43legacy_wldev *dev, u16 offset, s16 val);
 void b43legacy_nrssi_hw_update(struct b43legacy_wldev *dev, u16 val);
 void b43legacy_nrssi_mem_update(struct b43legacy_wldev *dev);
 
-void b43legacy_radio_set_tx_iq(struct b43legacy_wldev *dev);
 u16 b43legacy_radio_calibrationvalue(struct b43legacy_wldev *dev);
 
 #endif /* B43legacy_RADIO_H_ */
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH] TCP: Add support for TCP Stealth
From: Christian Grothoff @ 2015-01-01 15:32 UTC (permalink / raw)
  To: Daniel Borkmann, Julian Kirsch; +Cc: netdev, Jacob Appelbaum, Pavel Emelyanov
In-Reply-To: <54A566F2.4070401@redhat.com>

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

Dear Daniel,

That approach is highly vulnerable to timing attacks, and doesn't answer
how TCP clients without special capabilities could set the ISN correctly
either. Playing with raw sockets is the kind of geeky hack that is
unlikely to give us the combination of usability and security required
to significantly reduce the ongoing large-scale compromise of network
equipment by spy agencies.

Christian

On 01/01/2015 04:25 PM, Daniel Borkmann wrote:
> 
> /me wondering (haven't tried that though) ... have you considered f.e.
> building a library using a raw packet socket with a BPF filter to capture
> SYN packets and then TCP_REPAIR [1] to build a full-blown TCP socket out
> of it in case of a correct authentication from the ISN?
> 
> Thanks,
> Daniel


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

^ permalink raw reply

* [PATCH] net: wireless: ipw2x00: ipw2100.c:  Remove some unused functions
From: Rickard Strandqvist @ 2015-01-01 15:32 UTC (permalink / raw)
  To: Stanislav Yakovlev, Kalle Valo
  Cc: Rickard Strandqvist, linux-wireless, netdev, linux-kernel

Removes some functions that are not used anywhere:
write_nic_dword_auto_inc() write_nic_auto_inc_address()

This was partially found by using a static code analysis program called cppcheck.

Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
 drivers/net/wireless/ipw2x00/ipw2100.c |   11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/net/wireless/ipw2x00/ipw2100.c b/drivers/net/wireless/ipw2x00/ipw2100.c
index 6fabea0..fa88839 100644
--- a/drivers/net/wireless/ipw2x00/ipw2100.c
+++ b/drivers/net/wireless/ipw2x00/ipw2100.c
@@ -433,17 +433,6 @@ static inline void write_nic_byte(struct net_device *dev, u32 addr, u8 val)
 	write_register_byte(dev, IPW_REG_INDIRECT_ACCESS_DATA, val);
 }
 
-static inline void write_nic_auto_inc_address(struct net_device *dev, u32 addr)
-{
-	write_register(dev, IPW_REG_AUTOINCREMENT_ADDRESS,
-		       addr & IPW_REG_INDIRECT_ADDR_MASK);
-}
-
-static inline void write_nic_dword_auto_inc(struct net_device *dev, u32 val)
-{
-	write_register(dev, IPW_REG_AUTOINCREMENT_DATA, val);
-}
-
 static void write_nic_memory(struct net_device *dev, u32 addr, u32 len,
 				    const u8 * buf)
 {
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH] TCP: Add support for TCP Stealth
From: Daniel Borkmann @ 2015-01-01 15:25 UTC (permalink / raw)
  To: Julian Kirsch
  Cc: netdev, Christian Grothoff, Jacob Appelbaum, Pavel Emelyanov
In-Reply-To: <54A470B3.3010501@sec.in.tum.de>

Hi Julian,

On 12/31/2014 10:54 PM, Julian Kirsch wrote:
...
> one year ago [0] we tried to convince you to add support for a new
> socket option to the linux kernel. Equipped with an improved version of
> our patch we're back to accomplish this task today. :-)
>
> TCP Stealth is a modern variant of port knocking which borrows
> techniques from network steganography to enable clients to authenticate
> themselves towards a server on TCP level. You can find technical details
> in an rfc draft we wrote earlier this year [1] and in my master's thesis
> [2]. In summary, TCP Stealth derives authentication information from a
> pre-shared secret and embeds it into the ISN sent along with the first
> SYN from the client.

/me wondering (haven't tried that though) ... have you considered f.e.
building a library using a raw packet socket with a BPF filter to capture
SYN packets and then TCP_REPAIR [1] to build a full-blown TCP socket out
of it in case of a correct authentication from the ISN?

Thanks,
Daniel

   [1] http://www.criu.org/TCP_connection

> Our motivation is simple: During this year we gained hard evidence on
> secret services actively port scanning the internets followed by
> exploitation of your services using 0-day exploits [3, 4]. We don't want
> our machines to be turned into relays from where they continue to
> cascade their attacks. TCP Stealth makes port scanning more expensive by
> a factor of 2^31 (on average).
>
> A copy of this patch as well as patches for several user space
> applications can be found on the project's home page [5].
>
> All the best for the upcoming year,
> Julian & Christian
>
>
>
> [0] https://lkml.org/lkml/2013/12/10/1155
> [1] https://datatracker.ietf.org/doc/draft-kirsch-ietf-tcp-stealth/
> [2] https://gnunet.org/kirsch2014knock
> [3]
> http://www.heise.de/ct/artikel/NSA-GCHQ-The-HACIENDA-Program-for-Internet-Colonization-2292681.html
> [4]
> https://firstlook.org/theintercept/2014/12/13/belgacom-hack-gchq-inside-story/
> [5] https://gnunet.org/knock
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1
>
> iQEcBAEBAgAGBQJUpHCvAAoJENwkOWttRRA4g10IALbJZU9/5Gp8tVdpXqbkOIMp
> Kz+yOMyYULqYeM8yguSBZjZLbaz/VAS7SNpQxKGU+W0aAXa22FsSfVoUU7wqp3NT
> 3EGRuPkMaJkQ66IP8MtX+6/hSeWSh78tEaIFWVjyutihPyQGz0LefFc66gm54X4T
> s8IYW7jKFhNmmROu9CXLTxq4B5t2v+Evv/qWqotZqR1t3IbIUmZAiKrlkMRd7dtM
> SaS5JwFeiObxn+0M/7javQCAhfgPXYEOU0QKAGY55MXcPAner/5PuExIZdOJ41R3
> XD9tgoLGhHEiQkxj0/bP2cs3Cl5xfJl9t2iecVfTIR7PytaTJ/kFuE4gNgWEcTA=
> =T6/C
> -----END PGP SIGNATURE-----
>

^ permalink raw reply

* [PULL] vhost: cleanups and fixes
From: Michael S. Tsirkin @ 2015-01-01 12:26 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: kvm, virtualization, netdev, linux-kernel, mst, rusty

The following changes since commit b7392d2247cfe6771f95d256374f1a8e6a6f48d6:

  Linux 3.19-rc2 (2014-12-28 16:49:37 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus

for you to fetch changes up to 5d9a07b0de512b77bf28d2401e5fe3351f00a240:

  vhost: relax used address alignment (2014-12-29 10:55:06 +0200)

----------------------------------------------------------------
vhost: virtio 1.0 bugfix

There's a single change here, fixing a vhost bug where vhost initialization
fails due to used ring alignment check being too strict.

Cc: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----------------------------------------------------------------
Michael S. Tsirkin (2):
      virtio_ring: document alignment requirements
      vhost: relax used address alignment

 include/uapi/linux/virtio_ring.h |  7 +++++++
 drivers/vhost/vhost.c            | 10 +++++++---
 2 files changed, 14 insertions(+), 3 deletions(-)

^ permalink raw reply

* [PATCH net-next 0/7] Fixing the "Time Counter fixes and improvements"
From: Richard Cochran @ 2015-01-01 10:39 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, David Miller, Jeff Kirsher, John Stultz,
	Thomas Gleixner
In-Reply-To: <20141231.183347.862533634176009078.davem@davemloft.net>

Dave,

I did not catch the missing includes in my x86 and arm testing,
because those archs somehow do include clocksource.h for the drivers
in question. Sorry.

This is how I would like to fix the header fallout. We really should
decouple the timecounter/cyclecounter code from the clocksource code
where possible.


Thanks,
Richard


Richard Cochran (7):
  timecounter: provide a macro to initialize the cyclecounter mask
    field.
  bnx2x: convert to CYCLECOUNTER_MASK macro.
  e1000e: convert to CYCLECOUNTER_MASK macro.
  igb: convert to CYCLECOUNTER_MASK macro.
  ixgbe: convert to CYCLECOUNTER_MASK macro.
  mlx4: include clocksource.h again
  microblaze: include the new timecounter header.

 arch/microblaze/kernel/timer.c                   |    1 +
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |    2 +-
 drivers/net/ethernet/intel/e1000e/netdev.c       |    2 +-
 drivers/net/ethernet/intel/igb/igb_ptp.c         |    4 ++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c     |    2 +-
 drivers/net/ethernet/mellanox/mlx4/en_clock.c    |    1 +
 include/linux/timecounter.h                      |    5 ++++-
 7 files changed, 11 insertions(+), 6 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* [PATCH net-next 1/7] timecounter: provide a macro to initialize the cyclecounter mask field.
From: Richard Cochran @ 2015-01-01 10:39 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, David Miller, Jeff Kirsher, John Stultz,
	Thomas Gleixner
In-Reply-To: <cover.1420108214.git.richardcochran@gmail.com>

There is no need for users of the timecounter/cyclecounter code to include
clocksource.h just for a single macro.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 include/linux/timecounter.h |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/timecounter.h b/include/linux/timecounter.h
index 74f4549..4382035 100644
--- a/include/linux/timecounter.h
+++ b/include/linux/timecounter.h
@@ -19,6 +19,9 @@
 
 #include <linux/types.h>
 
+/* simplify initialization of mask field */
+#define CYCLECOUNTER_MASK(bits) (cycle_t)((bits) < 64 ? ((1ULL<<(bits))-1) : -1)
+
 /**
  * struct cyclecounter - hardware abstraction for a free running counter
  *	Provides completely state-free accessors to the underlying hardware.
@@ -29,7 +32,7 @@
  * @read:		returns the current cycle value
  * @mask:		bitmask for two's complement
  *			subtraction of non 64 bit counters,
- *			see CLOCKSOURCE_MASK() helper macro
+ *			see CYCLECOUNTER_MASK() helper macro
  * @mult:		cycle to nanosecond multiplier
  * @shift:		cycle to nanosecond divisor (power of two)
  */
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next 7/7] microblaze: include the new timecounter header.
From: Richard Cochran @ 2015-01-01 10:40 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, David Miller, Jeff Kirsher, John Stultz,
	Thomas Gleixner
In-Reply-To: <cover.1420108214.git.richardcochran@gmail.com>

The timecounter/cyclecounter code has moved, so users need the new include.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 arch/microblaze/kernel/timer.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/microblaze/kernel/timer.c b/arch/microblaze/kernel/timer.c
index dd96f0e..c897745 100644
--- a/arch/microblaze/kernel/timer.c
+++ b/arch/microblaze/kernel/timer.c
@@ -17,6 +17,7 @@
 #include <linux/clockchips.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
+#include <linux/timecounter.h>
 #include <asm/cpuinfo.h>
 
 static void __iomem *timer_baseaddr;
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next 6/7] mlx4: include clocksource.h again
From: Richard Cochran @ 2015-01-01 10:40 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, David Miller, Jeff Kirsher, John Stultz,
	Thomas Gleixner
In-Reply-To: <cover.1420108214.git.richardcochran@gmail.com>

This driver uses the function, clocksource_khz2mult, and so it really must
include clocksource.h.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_clock.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_clock.c b/drivers/net/ethernet/mellanox/mlx4/en_clock.c
index e9cce4f..90b5309 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_clock.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_clock.c
@@ -32,6 +32,7 @@
  */
 
 #include <linux/mlx4/device.h>
+#include <linux/clocksource.h>
 
 #include "mlx4_en.h"
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next 5/7] ixgbe: convert to CYCLECOUNTER_MASK macro.
From: Richard Cochran @ 2015-01-01 10:39 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, David Miller, Jeff Kirsher, John Stultz,
	Thomas Gleixner
In-Reply-To: <cover.1420108214.git.richardcochran@gmail.com>

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
index 47c29ea..79c00f5 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
@@ -793,7 +793,7 @@ void ixgbe_ptp_start_cyclecounter(struct ixgbe_adapter *adapter)
 
 	memset(&adapter->cc, 0, sizeof(adapter->cc));
 	adapter->cc.read = ixgbe_ptp_read;
-	adapter->cc.mask = CLOCKSOURCE_MASK(64);
+	adapter->cc.mask = CYCLECOUNTER_MASK(64);
 	adapter->cc.shift = shift;
 	adapter->cc.mult = 1;
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next 4/7] igb: convert to CYCLECOUNTER_MASK macro.
From: Richard Cochran @ 2015-01-01 10:39 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, David Miller, Jeff Kirsher, John Stultz,
	Thomas Gleixner
In-Reply-To: <cover.1420108214.git.richardcochran@gmail.com>

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/intel/igb/igb_ptp.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 1d27f2d..5e7a4e3 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -765,7 +765,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		adapter->ptp_caps.settime = igb_ptp_settime_82576;
 		adapter->ptp_caps.enable = igb_ptp_feature_enable;
 		adapter->cc.read = igb_ptp_read_82576;
-		adapter->cc.mask = CLOCKSOURCE_MASK(64);
+		adapter->cc.mask = CYCLECOUNTER_MASK(64);
 		adapter->cc.mult = 1;
 		adapter->cc.shift = IGB_82576_TSYNC_SHIFT;
 		/* Dial the nominal frequency. */
@@ -785,7 +785,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		adapter->ptp_caps.settime = igb_ptp_settime_82576;
 		adapter->ptp_caps.enable = igb_ptp_feature_enable;
 		adapter->cc.read = igb_ptp_read_82580;
-		adapter->cc.mask = CLOCKSOURCE_MASK(IGB_NBITS_82580);
+		adapter->cc.mask = CYCLECOUNTER_MASK(IGB_NBITS_82580);
 		adapter->cc.mult = 1;
 		adapter->cc.shift = 0;
 		/* Enable the timer functions by clearing bit 31. */
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next 3/7] e1000e: convert to CYCLECOUNTER_MASK macro.
From: Richard Cochran @ 2015-01-01 10:39 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, David Miller, Jeff Kirsher, John Stultz,
	Thomas Gleixner
In-Reply-To: <cover.1420108214.git.richardcochran@gmail.com>

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index e14fd85..332a298 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -4189,7 +4189,7 @@ static int e1000_sw_init(struct e1000_adapter *adapter)
 	/* Setup hardware time stamping cyclecounter */
 	if (adapter->flags & FLAG_HAS_HW_TIMESTAMP) {
 		adapter->cc.read = e1000e_cyclecounter_read;
-		adapter->cc.mask = CLOCKSOURCE_MASK(64);
+		adapter->cc.mask = CYCLECOUNTER_MASK(64);
 		adapter->cc.mult = 1;
 		/* cc.shift set in e1000e_get_base_tininca() */
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next 2/7] bnx2x: convert to CYCLECOUNTER_MASK macro.
From: Richard Cochran @ 2015-01-01 10:39 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, David Miller, Jeff Kirsher, John Stultz,
	Thomas Gleixner
In-Reply-To: <cover.1420108214.git.richardcochran@gmail.com>

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 2c95132..0758c8b 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -14610,7 +14610,7 @@ static void bnx2x_init_cyclecounter(struct bnx2x *bp)
 {
 	memset(&bp->cyclecounter, 0, sizeof(bp->cyclecounter));
 	bp->cyclecounter.read = bnx2x_cyclecounter_read;
-	bp->cyclecounter.mask = CLOCKSOURCE_MASK(64);
+	bp->cyclecounter.mask = CYCLECOUNTER_MASK(64);
 	bp->cyclecounter.shift = 1;
 	bp->cyclecounter.mult = 1;
 }
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH net-next v2 2/2] bridge: modify bridge af spec parser to accomodate vlan list and ranges
From: roopa @ 2015-01-01 10:01 UTC (permalink / raw)
  To: Arad, Ronen
  Cc: netdev@vger.kernel.org, hemminger@vyatta.com, vyasevic@redhat.com,
	sfeldma@gmail.com, wkok@cumulusnetworks.com
In-Reply-To: <E4CD12F19ABA0C4D8729E087A761DC3505DD2DE4@ORSMSX101.amr.corp.intel.com>

On 1/1/15, 12:54 AM, Arad, Ronen wrote:
>
>> -----Original Message-----
>> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>> Behalf Of roopa@cumulusnetworks.com
>> Sent: Wednesday, December 31, 2014 6:49 PM
>> To: netdev@vger.kernel.org; hemminger@vyatta.com; vyasevic@redhat.com
>> Cc: sfeldma@gmail.com; wkok@cumulusnetworks.com; Roopa Prabhu
>> Subject: [PATCH net-next v2 2/2] bridge: modify bridge af spec parser to
>> accomodate vlan list and ranges
>>
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This patch modifies br_afspec to parse incoming IFLA_BRIDGE_VLAN_INFO_LIST
>>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>> ---
>> net/bridge/br_netlink.c |  115 ++++++++++++++++++++++++++++++++++------------
>> -
>> 1 file changed, 85 insertions(+), 30 deletions(-)
>>
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index 492ef6a..bcba9d2 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -226,53 +226,108 @@ static const struct nla_policy
>> ifla_br_policy[IFLA_MAX+1] = {
>> 	[IFLA_BRIDGE_VLAN_INFO_LIST] = { .type = NLA_NESTED, },
>> };
>>
>> +static int br_vlan_info(struct net_bridge *br, struct net_bridge_port *p,
>> +			int cmd, struct bridge_vlan_info *vinfo)
>> +{
>> +	int err = 0;
>> +
>> +	switch (cmd) {
>> +	case RTM_SETLINK:
>> +		if (p) {
>> +			err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
>> +			if (err)
>> +				break;
>> +
>> +			if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
>> +				err = br_vlan_add(p->br, vinfo->vid,
>> +						  vinfo->flags);
>> +		} else {
>> +			err = br_vlan_add(br, vinfo->vid, vinfo->flags);
>> +		}
>> +		break;
>> +
>> +	case RTM_DELLINK:
>> +		if (p) {
>> +			nbp_vlan_delete(p, vinfo->vid);
>> +			if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
>> +				br_vlan_delete(p->br, vinfo->vid);
>> +		} else {
>> +			br_vlan_delete(br, vinfo->vid);
>> +		}
>> +		break;
>> +	}
>> +
>> +	return err;
>> +}
>> +
>> static int br_afspec(struct net_bridge *br,
>> 		     struct net_bridge_port *p,
>> 		     struct nlattr *af_spec,
>> 		     int cmd)
>> {
>> 	struct nlattr *tb[IFLA_BRIDGE_MAX+1];
>> +	struct nlattr *attr;
>> 	int err = 0;
>> +	int rem;
>>
>> 	err = nla_parse_nested(tb, IFLA_BRIDGE_MAX, af_spec, ifla_br_policy);
>> 	if (err)
>> 		return err;
>>
>> 	if (tb[IFLA_BRIDGE_VLAN_INFO]) {
>> -		struct bridge_vlan_info *vinfo;
>> -
>> -		vinfo = nla_data(tb[IFLA_BRIDGE_VLAN_INFO]);
>> -
>> -		if (!vinfo->vid || vinfo->vid >= VLAN_VID_MASK)
>> -			return -EINVAL;
>> -
>> -		switch (cmd) {
>> -		case RTM_SETLINK:
>> -			if (p) {
>> -				err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
>> -				if (err)
>> -					break;
>> -
>> -				if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
>> -					err = br_vlan_add(p->br, vinfo->vid,
>> -							  vinfo->flags);
>> -			} else
>> -				err = br_vlan_add(br, vinfo->vid, vinfo->flags);
>> -
>> -			break;
>> -
>> -		case RTM_DELLINK:
>> -			if (p) {
>> -				nbp_vlan_delete(p, vinfo->vid);
>> -				if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
>> -					br_vlan_delete(p->br, vinfo->vid);
>> -			} else
>> -				br_vlan_delete(br, vinfo->vid);
>> -			break;
>> +		attr = tb[IFLA_BRIDGE_VLAN_INFO];
>> +		if (nla_len(attr) != sizeof(struct bridge_vlan_info))
>> +			goto err_inval;
>> +
>> +		err = br_vlan_info(br, p, cmd,
>> +				   (struct bridge_vlan_info *)nla_data(attr));
>> +
>> +	} else if (tb[IFLA_BRIDGE_VLAN_INFO_LIST]) {
>> +		struct bridge_vlan_info *vinfo_start = NULL;
>> +		struct bridge_vlan_info *vinfo = NULL;
>> +
>> +		nla_for_each_nested(attr, tb[IFLA_BRIDGE_VLAN_INFO_LIST], rem) {
>> +			if (nla_len(attr) != sizeof(struct bridge_vlan_info) ||
>> +			    nla_type(attr) != IFLA_BRIDGE_VLAN_INFO)
>> +				goto err_inval;
>> +			vinfo = nla_data(attr);
>> +			if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_START) {
>> +				if (vinfo_start)
>> +					goto err_inval;
>> +				vinfo_start = vinfo;
>> +				continue;
>> +			}
>> +
>> +			if (vinfo_start) {
>> +				int v;
>> +
>> +				if (!(vinfo->flags & BRIDGE_VLAN_INFO_RANGE_END))
>> +					goto err_inval;
>> +
>> +				if (vinfo->vid < vinfo_start->vid)
> This check rejects inverted range. However it allows the RANGE_START and
> RANGE_END vinfos to have the same vid. Isn't it inconsistent with the rejection
> of a single vinfo with both RANGE_START and RANGE_END set?
sure, i will make it <= to error out if they are equal.
>> +					goto err_inval;
> Are additional validation such as consistency of flags between the RANGE_START
> and RANGE_END vinfos is needed here?
sure, i can add them (I have some of them in the iproute2 patch as well).
> The loop below applies flags (more precisely all data except for vid) from the
> RANGE_START vinfo to all vids in the range. All data from the RANGE_END vinfo
> is ignored.
>   
>> +
>> +				for (v = vinfo_start->vid; v <= vinfo->vid;
>> +					v++) {
>> +					vinfo_start->vid = v;
> This changes the vinfo with RANGE_START flag within the incoming message. Would
> it be better to left the input message unmodified and use a local copy of
> struct bridge_vlan_info?
agreed, I will make a copy.

>> +					err = br_vlan_info(br, p, cmd,
>> +							   vinfo_start);
>> +					if (err)
>> +						break;
>> +				}
>> +				vinfo_start = NULL;
>> +			} else {
>> +				err = br_vlan_info(br, p, cmd, vinfo);
>> +			}
>> +			if (err)
>> +				break;
>> 		}
>> 	}
>>
>> 	return err;
>> +
>> +err_inval:
>> +	return -EINVAL;
>> }
>>
>> static const struct nla_policy br_port_policy[IFLA_BRPORT_MAX + 1] = {
>> --
>> 1.7.10.4
>>
>> --
>> 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 net-next v2 2/2] bridge: modify bridge af spec parser to accomodate vlan list and ranges
From: Arad, Ronen @ 2015-01-01  8:54 UTC (permalink / raw)
  To: roopa@cumulusnetworks.com, netdev@vger.kernel.org,
	hemminger@vyatta.com, vyasevic@redhat.com
  Cc: sfeldma@gmail.com, wkok@cumulusnetworks.com
In-Reply-To: <1420044533-16963-3-git-send-email-roopa@cumulusnetworks.com>



>-----Original Message-----
>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>Behalf Of roopa@cumulusnetworks.com
>Sent: Wednesday, December 31, 2014 6:49 PM
>To: netdev@vger.kernel.org; hemminger@vyatta.com; vyasevic@redhat.com
>Cc: sfeldma@gmail.com; wkok@cumulusnetworks.com; Roopa Prabhu
>Subject: [PATCH net-next v2 2/2] bridge: modify bridge af spec parser to
>accomodate vlan list and ranges
>
>From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
>This patch modifies br_afspec to parse incoming IFLA_BRIDGE_VLAN_INFO_LIST
>
>Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>---
> net/bridge/br_netlink.c |  115 ++++++++++++++++++++++++++++++++++------------
>-
> 1 file changed, 85 insertions(+), 30 deletions(-)
>
>diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>index 492ef6a..bcba9d2 100644
>--- a/net/bridge/br_netlink.c
>+++ b/net/bridge/br_netlink.c
>@@ -226,53 +226,108 @@ static const struct nla_policy
>ifla_br_policy[IFLA_MAX+1] = {
> 	[IFLA_BRIDGE_VLAN_INFO_LIST] = { .type = NLA_NESTED, },
> };
>
>+static int br_vlan_info(struct net_bridge *br, struct net_bridge_port *p,
>+			int cmd, struct bridge_vlan_info *vinfo)
>+{
>+	int err = 0;
>+
>+	switch (cmd) {
>+	case RTM_SETLINK:
>+		if (p) {
>+			err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
>+			if (err)
>+				break;
>+
>+			if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
>+				err = br_vlan_add(p->br, vinfo->vid,
>+						  vinfo->flags);
>+		} else {
>+			err = br_vlan_add(br, vinfo->vid, vinfo->flags);
>+		}
>+		break;
>+
>+	case RTM_DELLINK:
>+		if (p) {
>+			nbp_vlan_delete(p, vinfo->vid);
>+			if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
>+				br_vlan_delete(p->br, vinfo->vid);
>+		} else {
>+			br_vlan_delete(br, vinfo->vid);
>+		}
>+		break;
>+	}
>+
>+	return err;
>+}
>+
> static int br_afspec(struct net_bridge *br,
> 		     struct net_bridge_port *p,
> 		     struct nlattr *af_spec,
> 		     int cmd)
> {
> 	struct nlattr *tb[IFLA_BRIDGE_MAX+1];
>+	struct nlattr *attr;
> 	int err = 0;
>+	int rem;
>
> 	err = nla_parse_nested(tb, IFLA_BRIDGE_MAX, af_spec, ifla_br_policy);
> 	if (err)
> 		return err;
>
> 	if (tb[IFLA_BRIDGE_VLAN_INFO]) {
>-		struct bridge_vlan_info *vinfo;
>-
>-		vinfo = nla_data(tb[IFLA_BRIDGE_VLAN_INFO]);
>-
>-		if (!vinfo->vid || vinfo->vid >= VLAN_VID_MASK)
>-			return -EINVAL;
>-
>-		switch (cmd) {
>-		case RTM_SETLINK:
>-			if (p) {
>-				err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
>-				if (err)
>-					break;
>-
>-				if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
>-					err = br_vlan_add(p->br, vinfo->vid,
>-							  vinfo->flags);
>-			} else
>-				err = br_vlan_add(br, vinfo->vid, vinfo->flags);
>-
>-			break;
>-
>-		case RTM_DELLINK:
>-			if (p) {
>-				nbp_vlan_delete(p, vinfo->vid);
>-				if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
>-					br_vlan_delete(p->br, vinfo->vid);
>-			} else
>-				br_vlan_delete(br, vinfo->vid);
>-			break;
>+		attr = tb[IFLA_BRIDGE_VLAN_INFO];
>+		if (nla_len(attr) != sizeof(struct bridge_vlan_info))
>+			goto err_inval;
>+
>+		err = br_vlan_info(br, p, cmd,
>+				   (struct bridge_vlan_info *)nla_data(attr));
>+
>+	} else if (tb[IFLA_BRIDGE_VLAN_INFO_LIST]) {
>+		struct bridge_vlan_info *vinfo_start = NULL;
>+		struct bridge_vlan_info *vinfo = NULL;
>+
>+		nla_for_each_nested(attr, tb[IFLA_BRIDGE_VLAN_INFO_LIST], rem) {
>+			if (nla_len(attr) != sizeof(struct bridge_vlan_info) ||
>+			    nla_type(attr) != IFLA_BRIDGE_VLAN_INFO)
>+				goto err_inval;
>+			vinfo = nla_data(attr);
>+			if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_START) {
>+				if (vinfo_start)
>+					goto err_inval;
>+				vinfo_start = vinfo;
>+				continue;
>+			}
>+
>+			if (vinfo_start) {
>+				int v;
>+
>+				if (!(vinfo->flags & BRIDGE_VLAN_INFO_RANGE_END))
>+					goto err_inval;
>+
>+				if (vinfo->vid < vinfo_start->vid)

This check rejects inverted range. However it allows the RANGE_START and
RANGE_END vinfos to have the same vid. Isn't it inconsistent with the rejection
of a single vinfo with both RANGE_START and RANGE_END set?
>+					goto err_inval;
Are additional validation such as consistency of flags between the RANGE_START
and RANGE_END vinfos is needed here?
The loop below applies flags (more precisely all data except for vid) from the
RANGE_START vinfo to all vids in the range. All data from the RANGE_END vinfo
is ignored.
 
>+
>+				for (v = vinfo_start->vid; v <= vinfo->vid;
>+					v++) {
>+					vinfo_start->vid = v;
This changes the vinfo with RANGE_START flag within the incoming message. Would
it be better to left the input message unmodified and use a local copy of
struct bridge_vlan_info? 
>+					err = br_vlan_info(br, p, cmd,
>+							   vinfo_start);
>+					if (err)
>+						break;
>+				}
>+				vinfo_start = NULL;
>+			} else {
>+				err = br_vlan_info(br, p, cmd, vinfo);
>+			}
>+			if (err)
>+				break;
> 		}
> 	}
>
> 	return err;
>+
>+err_inval:
>+	return -EINVAL;
> }
>
> static const struct nla_policy br_port_policy[IFLA_BRPORT_MAX + 1] = {
>--
>1.7.10.4
>
>--
>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 net-next v2 1/2] bridge: new attribute and flags to represent vlan info lists and ranges
From: roopa @ 2015-01-01  4:25 UTC (permalink / raw)
  To: Jeremiah Mahler, netdev, shemminger, vyasevic, sfeldma, wkok
In-Reply-To: <20150101030817.GA10710@hudson.localdomain>

On 12/31/14, 7:08 PM, Jeremiah Mahler wrote:
> Roopa,
>
> On Wed, Dec 31, 2014 at 01:17:31PM -0800, roopa wrote:
>> On 12/31/14, 10:48 AM, Jeremiah Mahler wrote:
>>> Roopa,
>>>
>>> On Wed, Dec 31, 2014 at 10:15:53AM -0800, roopa wrote:
>>>> On 12/31/14, 9:45 AM, Jeremiah Mahler wrote:
>>>>> Roopa,
>>>>>
>>>>> On Wed, Dec 31, 2014 at 08:48:52AM -0800, roopa@cumulusnetworks.com wrote:
>>>>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>>>
>>>>>> This patch adds (as suggested by scott feldman),
>>>>>>          - new netlink attribute IFLA_BRIDGE_VLAN_INFO_LIST to represent
>>>>>>            vlan list
>>>>>>          - And bridge_vlan_info flags BRIDGE_VLAN_INFO_RANGE_START and
>>>>>>            BRIDGE_VLAN_INFO_RANGE_END to indicate start and end of vlan range
>>>>>>
>>>>>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>>> ---
>>>>>>   include/uapi/linux/if_bridge.h |    4 ++++
>>>>>>   net/bridge/br_netlink.c        |    1 +
>>>>>>   2 files changed, 5 insertions(+)
>>>>>>
>>>>>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>>>>>> index b03ee8f..fa468aa 100644
>>>>>> --- a/include/uapi/linux/if_bridge.h
>>>>>> +++ b/include/uapi/linux/if_bridge.h
>>>>>> @@ -112,12 +112,14 @@ struct __fdb_entry {
>>>>>>    *     [IFLA_BRIDGE_FLAGS]
>>>>>>    *     [IFLA_BRIDGE_MODE]
>>>>>>    *     [IFLA_BRIDGE_VLAN_INFO]
>>>>>> + *     [IFLA_BRIDGE_VLAN_INFO_LIST]
>>>>>>    * }
>>>>>>    */
>>>>>>   enum {
>>>>>>   	IFLA_BRIDGE_FLAGS,
>>>>>>   	IFLA_BRIDGE_MODE,
>>>>>>   	IFLA_BRIDGE_VLAN_INFO,
>>>>>> +	IFLA_BRIDGE_VLAN_INFO_LIST,
>>>>>>   	__IFLA_BRIDGE_MAX,
>>>>>>   };
>>>>>>   #define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
>>>>>> @@ -125,6 +127,8 @@ enum {
>>>>>>   #define BRIDGE_VLAN_INFO_MASTER	(1<<0)	/* Operate on Bridge device as well */
>>>>>>   #define BRIDGE_VLAN_INFO_PVID	(1<<1)	/* VLAN is PVID, ingress untagged */
>>>>>>   #define BRIDGE_VLAN_INFO_UNTAGGED	(1<<2)	/* VLAN egresses untagged */
>>>>>> +#define BRIDGE_VLAN_INFO_RANGE_START	(1<<3) /* VLAN is start of vlan range */
>>>>>> +#define BRIDGE_VLAN_INFO_RANGE_END	(1<<4) /* VLAN is end of vlan range */
>>>>> You add these here but you don't use them until the next patch.
>>>>> If they were wrong a bisect would point to the next patch.
>>>>>
>>>>> I would add them in the next patch where you start to use them.
>>>> I thought it was ok to declare it first and use them in the next patch. Only
>>>> the other way around would be bad.
>>>>   I have submitted in a similar way before. If needed i will resubmit.
>>>>
>>>>
>>> Hmm.  I cannot see how the other way would be bad but maybe I am missing
>>> something.
>> sorry, i did not mean what you were saying would be bad. I was just trying
>> to say that, use first and declare later would be bad (ie if my patches 1
>> and 2 were swapped). Otherwise i don't see a problem.
>>
> Now I understand.  Yes, swapping the patches would be bad.
>
>> I know that you are saying i should combine the patches 1 and 2 into a
>> single patch. That is not a problem. If i need to respin again due to other
>> reasons i will consider merging them as well if that is a concern.
>>
> Er, well not quite.  I don't think both patches should be combined in to
> one.  I would only move those two #defines that I pointed out in the
> first patch in to the second patch.
>
> I hope that makes a little more sense :)
okay :).

thanks,
Roopa

>
>> thanks.
>>
>>>   Hopefully someone else has some insight.
>>>
>>>>>>   struct bridge_vlan_info {
>>>>>>   	__u16 flags;
>>>>>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>>>>>> index 9f5eb55..492ef6a 100644
>>>>>> --- a/net/bridge/br_netlink.c
>>>>>> +++ b/net/bridge/br_netlink.c
>>>>>> @@ -223,6 +223,7 @@ static const struct nla_policy ifla_br_policy[IFLA_MAX+1] = {
>>>>>>   	[IFLA_BRIDGE_MODE]	= { .type = NLA_U16 },
>>>>>>   	[IFLA_BRIDGE_VLAN_INFO]	= { .type = NLA_BINARY,
>>>>>>   				    .len = sizeof(struct bridge_vlan_info), },
>>>>>> +	[IFLA_BRIDGE_VLAN_INFO_LIST] = { .type = NLA_NESTED, },
>>>>>>   };
>>>>>>   static int br_afspec(struct net_bridge *br,
>>>>>> -- 
>>>>>> 1.7.10.4
>>>>>>
>>>>>> --
>>>>>> 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 net-next v2 1/2] bridge: new attribute and flags to represent vlan info lists and ranges
From: Jeremiah Mahler @ 2015-01-01  3:08 UTC (permalink / raw)
  To: roopa; +Cc: netdev, shemminger, vyasevic, sfeldma, wkok
In-Reply-To: <54A467EB.5010404@cumulusnetworks.com>

Roopa,

On Wed, Dec 31, 2014 at 01:17:31PM -0800, roopa wrote:
> On 12/31/14, 10:48 AM, Jeremiah Mahler wrote:
> >Roopa,
> >
> >On Wed, Dec 31, 2014 at 10:15:53AM -0800, roopa wrote:
> >>On 12/31/14, 9:45 AM, Jeremiah Mahler wrote:
> >>>Roopa,
> >>>
> >>>On Wed, Dec 31, 2014 at 08:48:52AM -0800, roopa@cumulusnetworks.com wrote:
> >>>>From: Roopa Prabhu <roopa@cumulusnetworks.com>
> >>>>
> >>>>This patch adds (as suggested by scott feldman),
> >>>>         - new netlink attribute IFLA_BRIDGE_VLAN_INFO_LIST to represent
> >>>>           vlan list
> >>>>         - And bridge_vlan_info flags BRIDGE_VLAN_INFO_RANGE_START and
> >>>>           BRIDGE_VLAN_INFO_RANGE_END to indicate start and end of vlan range
> >>>>
> >>>>Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> >>>>---
> >>>>  include/uapi/linux/if_bridge.h |    4 ++++
> >>>>  net/bridge/br_netlink.c        |    1 +
> >>>>  2 files changed, 5 insertions(+)
> >>>>
> >>>>diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> >>>>index b03ee8f..fa468aa 100644
> >>>>--- a/include/uapi/linux/if_bridge.h
> >>>>+++ b/include/uapi/linux/if_bridge.h
> >>>>@@ -112,12 +112,14 @@ struct __fdb_entry {
> >>>>   *     [IFLA_BRIDGE_FLAGS]
> >>>>   *     [IFLA_BRIDGE_MODE]
> >>>>   *     [IFLA_BRIDGE_VLAN_INFO]
> >>>>+ *     [IFLA_BRIDGE_VLAN_INFO_LIST]
> >>>>   * }
> >>>>   */
> >>>>  enum {
> >>>>  	IFLA_BRIDGE_FLAGS,
> >>>>  	IFLA_BRIDGE_MODE,
> >>>>  	IFLA_BRIDGE_VLAN_INFO,
> >>>>+	IFLA_BRIDGE_VLAN_INFO_LIST,
> >>>>  	__IFLA_BRIDGE_MAX,
> >>>>  };
> >>>>  #define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
> >>>>@@ -125,6 +127,8 @@ enum {
> >>>>  #define BRIDGE_VLAN_INFO_MASTER	(1<<0)	/* Operate on Bridge device as well */
> >>>>  #define BRIDGE_VLAN_INFO_PVID	(1<<1)	/* VLAN is PVID, ingress untagged */
> >>>>  #define BRIDGE_VLAN_INFO_UNTAGGED	(1<<2)	/* VLAN egresses untagged */
> >>>>+#define BRIDGE_VLAN_INFO_RANGE_START	(1<<3) /* VLAN is start of vlan range */
> >>>>+#define BRIDGE_VLAN_INFO_RANGE_END	(1<<4) /* VLAN is end of vlan range */
> >>>You add these here but you don't use them until the next patch.
> >>>If they were wrong a bisect would point to the next patch.
> >>>
> >>>I would add them in the next patch where you start to use them.
> >>I thought it was ok to declare it first and use them in the next patch. Only
> >>the other way around would be bad.
> >>  I have submitted in a similar way before. If needed i will resubmit.
> >>
> >>
> >Hmm.  I cannot see how the other way would be bad but maybe I am missing
> >something.
> sorry, i did not mean what you were saying would be bad. I was just trying
> to say that, use first and declare later would be bad (ie if my patches 1
> and 2 were swapped). Otherwise i don't see a problem.
> 
Now I understand.  Yes, swapping the patches would be bad.

> I know that you are saying i should combine the patches 1 and 2 into a
> single patch. That is not a problem. If i need to respin again due to other
> reasons i will consider merging them as well if that is a concern.
> 
Er, well not quite.  I don't think both patches should be combined in to
one.  I would only move those two #defines that I pointed out in the
first patch in to the second patch.

I hope that makes a little more sense :)

> thanks.
> 
> >  Hopefully someone else has some insight.
> >
> >>
> >>>>  struct bridge_vlan_info {
> >>>>  	__u16 flags;
> >>>>diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> >>>>index 9f5eb55..492ef6a 100644
> >>>>--- a/net/bridge/br_netlink.c
> >>>>+++ b/net/bridge/br_netlink.c
> >>>>@@ -223,6 +223,7 @@ static const struct nla_policy ifla_br_policy[IFLA_MAX+1] = {
> >>>>  	[IFLA_BRIDGE_MODE]	= { .type = NLA_U16 },
> >>>>  	[IFLA_BRIDGE_VLAN_INFO]	= { .type = NLA_BINARY,
> >>>>  				    .len = sizeof(struct bridge_vlan_info), },
> >>>>+	[IFLA_BRIDGE_VLAN_INFO_LIST] = { .type = NLA_NESTED, },
> >>>>  };
> >>>>  static int br_afspec(struct net_bridge *br,
> >>>>-- 
> >>>>1.7.10.4
> >>>>
> >>>>--
> >>>>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
> 

-- 
- Jeremiah Mahler

^ permalink raw reply

* Re: [net-next PATCH 00/17] fib_trie: Reduce time spent in fib_table_lookup by 35 to 75%
From: Alexander Duyck @ 2015-01-01  2:32 UTC (permalink / raw)
  To: David Miller, alexander.h.duyck; +Cc: netdev
In-Reply-To: <20141231.184610.1802958694945952516.davem@davemloft.net>

On 12/31/2014 03:46 PM, David Miller wrote:
> From: Alexander Duyck <alexander.h.duyck@redhat.com>
> Date: Wed, 31 Dec 2014 10:55:23 -0800
>
>> These patches are meant to address several performance issues I have seen 
>> in the fib_trie implementation, and fib_table_lookup specifically.  With 
>> these changes in place I have seen a reduction of up to 35 to 75% for the 
>> total time spent in fib_table_lookup depending on the type of search being 
>> performed.
>  ...
>> Changes since RFC:
>>   Replaced this_cpu_ptr with correct call to this_cpu_inc in patch 1
>>   Changed test for leaf_info mismatch to (key ^ n->key) & li->mask_plen in patch 10
> As before, this looks awesome.

Thanks.

> All applied to net-next, thanks!
>
> This knocks about 35 cpu cycles off of a lookup that ends up using the
> default route on sparc64.  From about ~438 cycles to ~403.

Did that 438 value include both fib_table_lookup and check_leaf?  Just
curious as the overall gain seems smaller than what I have been seeing
on the x86 system I was testing with, but then again it could just be a
sparc64 thing.

I've started work on a second round of patches.  With any luck they
should be ready by the time the next net-next opens.  My hope is to cut
the look-up time by another 30 to 50%, though it will take some time as
I have to go though and drop the leaf_info structure, and look at
splitting the tnode in half to break the key/pos/bits and child pointer
dependency chain which will hopefully allow for a significant reduction
in memory read stalls.

I am also planning to take a look at addressing the memory waste that
occurs on nodes larger than 256 bytes due to the way kmalloc allocates
memory as powers of 2.  I'm thinking I might try encouraging the growth
of smaller nodes, and discouraging anything over 256 by implementing a
"truesize" type logic that can be used in the inflate/halve functions so
that the memory usage is more accurately reflected.

- Alex

^ permalink raw reply

* WIP alternative - was Re: [PATCH v3 14/20] selftests/size: add install target to enable test install
From: Tim Bird @ 2015-01-01  2:31 UTC (permalink / raw)
  To: Shuah Khan, mmarek@suse.cz, gregkh@linuxfoundation.org,
	akpm@linux-foundation.org, rostedt@goodmis.org, mingo@redhat.com,
	davem@davemloft.net, keescook@chromium.org,
	tranmanphong@gmail.com, mpe@ellerman.id.au, cov@codeaurora.org,
	dh.herrmann@gmail.com, hughd@google.com, bobby.prani@gmail.com,
	serge.hallyn@ubuntu.com, ebiederm@xmission.com,
	josh@joshtriplett.org, koct9i@gmail.com
  Cc: linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-api@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <c961f80614b02a2f3c6455518d1f6fabdaf6c7f3.1419387513.git.shuahkh@osg.samsung.com>

On 12/24/2014 08:27 AM, Shuah Khan wrote:
> Add a new make target to enable installing test. This target
> installs test in the kselftest install location and add to the
> kselftest script to run the test. Install target can be run
> only from top level kernel source directory.
> 
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> ---
>  tools/testing/selftests/size/Makefile | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/size/Makefile b/tools/testing/selftests/size/Makefile
> index 04dc25e..bb7113b 100644
> --- a/tools/testing/selftests/size/Makefile
> +++ b/tools/testing/selftests/size/Makefile
> @@ -1,12 +1,22 @@
>  CC = $(CROSS_COMPILE)gcc
>  
> +TEST_STR = ./get_size || echo get_size selftests: [FAIL]
> +
>  all: get_size
>  
>  get_size: get_size.c
>  	$(CC) -static -ffreestanding -nostartfiles -s $< -o $@
>  
> +install:
> +ifdef INSTALL_KSFT_PATH
> +	install ./get_size $(INSTALL_KSFT_PATH)
> +	@echo "$(TEST_STR)" >> $(KSELFTEST)
> +else
> +	@echo Run make kselftest_install in top level source directory
> +endif
> +
>  run_tests: all
> -	./get_size
> +	@$(TEST_STR)
>  
>  clean:
>  	$(RM) get_size
> 

The install phase is desperately needed for usage of kselftest in
cross-target situations (applicable to almost all embedded).  So this
is great stuff.

I worked a bit on isolating the install stuff to a makefile include file.
This allows simplifying some of the sub-level Makefiles a bit, and allowing
control of some of the install and run logic in less places.

This is still a work in progress, but before I got too far along, I wanted
to post it for people to provide feedback.  A couple of problems cropped
up that are worth discussing, IMHO.

1) I think it should be a requirement that each test has a single
"main" program to execute to run the tests.  If multiple tests are supported
or more flexibility is desired for additional arguments, or that sort of
thing, then that's fine, but the automated script builder should be able
to run just a single program or script to have things work.  This also
makes things more consistent.  In the case of the firmware test, I created
a single fw_both.sh script to do this, instead of having two separate
blocks in the kselftest.sh script.

2) I've added a CROSS_INSTALL variable, which can call an arbitrary program
to place files on the target system (rather than just calling 'install').
In my case, I'd use my own 'ttc cp' command, which I can extend as necessary
to put things on a remote machine.  This works for a single directory,
but things get dicier with sub-directory trees full of files (like
the ftrace test uses.)

If additional items need to be installed to the target, then maybe a setup
program should be used, rather than just copying files.

3) Some of the scripts were using /bin/bash to execute them, rather
than rely on the interpreter line in the script itself (and having
the script have executable privileges).  Is there a reason for this?
I modified a few scripts to be executable, and got rid of the
explicit execution with /bin/bash.

The following is just a start...  Let me know if this direction looks
OK, and I'll finish this up.  The main item to look at is
kselftest.include file.  Note that these patches are based on Shuah's
series - but if you want to use these ideas I can rebase them onto
mainline, and break them out per test sub-level like Shuah did.


Let me know what you think.
>From 14164fd3117c97799a050f8cf791dedc93aa5e82 Mon Sep 17 00:00:00 2001
From: Tim Bird <tim.bird@sonymobile.com>
Date: Wed, 31 Dec 2014 18:04:08 -0800
Subject: [PATCH] Switch to using an include file for common kselftest_install
 actions

---
 tools/testing/selftests/cpu-hotplug/Makefile       | 15 +++-------
 .../selftests/cpu-hotplug/cpu-on-off-test.sh       |  0
 tools/testing/selftests/efivarfs/Makefile          | 21 ++++----------
 tools/testing/selftests/efivarfs/efivarfs.sh       |  0
 tools/testing/selftests/firmware/Makefile          | 32 ++++------------------
 tools/testing/selftests/firmware/fw_both.sh        | 13 +++++++++
 tools/testing/selftests/kselftest.include          | 26 ++++++++++++++++++
 tools/testing/selftests/size/Makefile              | 19 +++----------
 9 files changed, 60 insertions(+), 69 deletions(-)
 mode change 100644 => 100755 tools/testing/selftests/cpu-hotplug/cpu-on-off-test.sh
 mode change 100644 => 100755 tools/testing/selftests/efivarfs/efivarfs.sh
 create mode 100755 tools/testing/selftests/firmware/fw_both.sh
 create mode 100644 tools/testing/selftests/kselftest.include

diff --git a/tools/testing/selftests/cpu-hotplug/Makefile b/tools/testing/selftests/cpu-hotplug/Makefile
index c9e15ee..d2e540d 100644
--- a/tools/testing/selftests/cpu-hotplug/Makefile
+++ b/tools/testing/selftests/cpu-hotplug/Makefile
@@ -1,18 +1,11 @@
-TEST_STR=/bin/bash ./cpu-on-off-test.sh || echo cpu-hotplug selftests: [FAIL]
+TEST_TITLE = cpu-hotplug
+TEST_MAIN_PROG = cpu-on-off-test.sh
 
 all:
 
-install:
-ifdef INSTALL_KSFT_PATH
-	install ./cpu-on-off-test.sh $(INSTALL_KSFT_PATH)/cpu-on-off-test.sh
-	@echo "$(TEST_STR)" >> $(KSELFTEST)
-else
-	@echo Run make kselftest_install in top level source directory
-endif
-
-run_tests:
-	@$(TEST_STR)
+include ../kselftest.include
 
+# use -a to get all tests
 run_full_test:
 	@/bin/bash ./cpu-on-off-test.sh -a || echo "cpu-hotplug selftests: [FAIL]"
 
diff --git a/tools/testing/selftests/cpu-hotplug/cpu-on-off-test.sh b/tools/testing/selftests/cpu-hotplug/cpu-on-off-test.sh
old mode 100644
new mode 100755
diff --git a/tools/testing/selftests/efivarfs/Makefile b/tools/testing/selftests/efivarfs/Makefile
index aaf404b..8d48e9c 100644
--- a/tools/testing/selftests/efivarfs/Makefile
+++ b/tools/testing/selftests/efivarfs/Makefile
@@ -1,24 +1,15 @@
-CC = $(CROSS_COMPILE)gcc
-CFLAGS = -Wall
+TEST_TITLE = efivarfs
+TEST_MAIN_PROG = efivarfs.sh
 
-test_objs = open-unlink create-read
+CFLAGS = -Wall
 
-TEST_STR = /bin/bash ./efivarfs.sh || echo efivarfs selftests: [FAIL]
+TEST_PROGS = open-unlink create-read
 
 all:
 	gcc open-unlink.c -o open-unlink
 	gcc create-read.c -o create-read
 
-install:
-ifdef INSTALL_KSFT_PATH
-	install ./efivarfs.sh $(test_objs) $(INSTALL_KSFT_PATH)
-	@echo "$(TEST_STR)" >> $(KSELFTEST)
-else
-	@echo Run make kselftest_install in top level source directory
-endif
-
-run_tests: all
-	@$(TEST_STR)
+include ../kselftest.include
 
 clean:
-	rm -f $(test_objs)
+	rm -f $(TEST_PROGS)
diff --git a/tools/testing/selftests/efivarfs/efivarfs.sh b/tools/testing/selftests/efivarfs/efivarfs.sh
old mode 100644
new mode 100755
diff --git a/tools/testing/selftests/firmware/Makefile b/tools/testing/selftests/firmware/Makefile
index 7ac1cf3..b576308 100644
--- a/tools/testing/selftests/firmware/Makefile
+++ b/tools/testing/selftests/firmware/Makefile
@@ -1,36 +1,14 @@
 # Makefile for firmware loading selftests
+TEST_MAIN_PROG = fw_both.sh
+TEST_TITLE = firmware
+TEST_FILES = ./fw_filesystem.sh ./fw_userhelper.sh
 
 # No binaries, but make sure arg-less "make" doesn't trigger "run_tests"
-
-__fw_filesystem:
-fw_filesystem  = if /bin/sh ./fw_filesystem.sh ; then
-fw_filesystem += echo fw_filesystem: ok;
-fw_filesystem += else echo fw_filesystem: [FAIL];
-fw_filesystem += fi
-
-__fw_userhelper:
-fw_userhelper  = if /bin/sh ./fw_userhelper.sh ; then
-fw_userhelper += echo fw_userhelper: ok;
-fw_userhelper += else
-fw_userhelper += echo fw_userhelper: [FAIL];
-fw_userhelper += fi
-
 all:
 
-install:
-ifdef INSTALL_KSFT_PATH
-	install ./fw_filesystem.sh ./fw_userhelper.sh $(INSTALL_KSFT_PATH)
-	@echo "$(fw_filesystem)" >> $(KSELFTEST)
-	@echo "$(fw_userhelper)" >> $(KSELFTEST)
-else
-	@echo Run make kselftest_install in top level source directory
-endif
-
-run_tests:
-	@$(fw_filesystem)
-	@$(fw_userhelper)
+include ../kselftest.include
 
 # Nothing to clean up.
 clean:
 
-.PHONY: all clean run_tests fw_filesystem fw_userhelper
+.PHONY: all clean run_tests
diff --git a/tools/testing/selftests/firmware/fw_both.sh b/tools/testing/selftests/firmware/fw_both.sh
new file mode 100755
index 0000000..fb7fa8d
--- /dev/null
+++ b/tools/testing/selftests/firmware/fw_both.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+if /bin/sh ./fw_filesystem.sh ; then
+	echo "fw_filesystem: ok";
+else
+	echo "fw_filesystem: [FAIL]";
+fi
+
+if /bin/sh ./fw_userhelper.sh ; then
+	echo "fw_userhelper: ok";
+else
+	echo "fw_userhelper: [FAIL]";
+fi
diff --git a/tools/testing/selftests/kselftest.include b/tools/testing/selftests/kselftest.include
new file mode 100644
index 0000000..7020a82
--- /dev/null
+++ b/tools/testing/selftests/kselftest.include
@@ -0,0 +1,26 @@
+# this make include script expects the following variables to be set:
+#  TEST_MAIN_PROG - a single program to invoke to run the test
+#  TEST_TITLE - the title of the test
+#  TEST_FILES - other files that should be copied to the testing directory
+#  INSTALL_KSFT_PATH - where to put test programs
+#  KSELFTEST - the name of the generated script (which calls the sub-tests)
+#  the 'all' target, which makes sure everything is built
+
+# this file defines the 'install' and 'run_tests' targets
+
+CC = $(CROSS_COMPILE)gcc
+INSTALL = $(CROSS_INSTALL)install
+export INSTALL
+
+TEST_STR = ./$(TEST_MAIN_PROG) || echo $(TEST_TITLE) selftests: [FAIL]
+
+install:
+ifdef INSTALL_KSFT_PATH
+	$(INSTALL) ./$(TEST_MAIN_PROG) $(TEST_FILES) $(INSTALL_KSFT_PATH)
+	@echo "$(TEST_STR)" >> $(KSELFTEST)
+else
+	@echo Run make kselftest_install in the top level source directory
+endif
+
+run_tests: all
+	@$(TEST_STR)
diff --git a/tools/testing/selftests/size/Makefile b/tools/testing/selftests/size/Makefile
index bb7113b..c88819a 100644
--- a/tools/testing/selftests/size/Makefile
+++ b/tools/testing/selftests/size/Makefile
@@ -1,22 +1,11 @@
-CC = $(CROSS_COMPILE)gcc
+TEST_MAIN_PROG = get_size
 
-TEST_STR = ./get_size || echo get_size selftests: [FAIL]
+all: $(TEST_MAIN_PROG)
 
-all: get_size
+include ../kselftest.include
 
 get_size: get_size.c
 	$(CC) -static -ffreestanding -nostartfiles -s $< -o $@
 
-install:
-ifdef INSTALL_KSFT_PATH
-	install ./get_size $(INSTALL_KSFT_PATH)
-	@echo "$(TEST_STR)" >> $(KSELFTEST)
-else
-	@echo Run make kselftest_install in top level source directory
-endif
-
-run_tests: all
-	@$(TEST_STR)
-
 clean:
-	$(RM) get_size
+	$(RM) $(TEST_MAIN_PROG)
-- 
1.8.2.2



^ 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