Netdev List
 help / color / mirror / Atom feed
* [PATCH v7 6/8] net: can: c_can: Disable pins when CAN interface is down
From: Roger Quadros @ 2014-11-14 15:40 UTC (permalink / raw)
  To: wg, mkl
  Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
	sergei.shtylyov, linux-omap, linux-can, netdev, Roger Quadros
In-Reply-To: <5464CCFF.5010004@ti.com>

DRA7 CAN IP suffers from a problem which causes it to be prevented
from fully turning OFF (i.e. stuck in transition) if the module was
disabled while there was traffic on the CAN_RX line.

To work around this issue we select the SLEEP pin state by default
on probe and use the DEFAULT pin state on CAN up and back to the
SLEEP pin state on CAN down.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/net/can/c_can/c_can.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 8e78bb4..f94a9fa 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -35,6 +35,7 @@
 #include <linux/list.h>
 #include <linux/io.h>
 #include <linux/pm_runtime.h>
+#include <linux/pinctrl/consumer.h>
 
 #include <linux/can.h>
 #include <linux/can/dev.h>
@@ -603,6 +604,8 @@ static int c_can_start(struct net_device *dev)
 
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
+	/* activate pins */
+	pinctrl_pm_select_default_state(dev->dev.parent);
 	return 0;
 }
 
@@ -611,6 +614,9 @@ static void c_can_stop(struct net_device *dev)
 	struct c_can_priv *priv = netdev_priv(dev);
 
 	c_can_irq_control(priv, false);
+
+	/* deactivate pins */
+	pinctrl_pm_select_sleep_state(dev->dev.parent);
 	priv->can.state = CAN_STATE_STOPPED;
 }
 
@@ -1244,6 +1250,13 @@ int register_c_can_dev(struct net_device *dev)
 	struct c_can_priv *priv = netdev_priv(dev);
 	int err;
 
+	/* Deactivate pins to prevent DRA7 DCAN IP from being
+	 * stuck in transition when module is disabled.
+	 * Pins are activated in c_can_start() and deactivated
+	 * in c_can_stop()
+	 */
+	pinctrl_pm_select_sleep_state(dev->dev.parent);
+
 	c_can_pm_runtime_enable(priv);
 
 	dev->flags |= IFF_ECHO;	/* we support local echo */
-- 
1.8.3.2



^ permalink raw reply related

* Re: [PATCH] rtlwifi: Add more checks for get_btc_status callback
From: Mike Galbraith @ 2014-11-14 15:41 UTC (permalink / raw)
  To: Murilo Opsfelder Araújo
  Cc: Larry Finger, linux-kernel, linux-wireless, netdev, Chaoming Li,
	John W. Linville, Thadeu Cascardo, 谭杭波
In-Reply-To: <CA+LKeY6xca=f-qFJoVFaqywRpfHPmMAWDrMmbkUiDf9atA_jVQ@mail.gmail.com>

On Wed, 2014-11-12 at 21:03 -0200, Murilo Opsfelder Araújo wrote:

> Hello, Larry.
> 
> I'd like to let you know that next-20141112 brought my rtl8192se
> device back to life.  I didn't need to build rtlwifi_new to be able to
> connect to my wifi network.  It worked just fine.

Yeah, unsurprisingly, merging those fixed up my lappy as well.

	-Mike

^ permalink raw reply

* Re: [PATCH net-next] fast_hash: clobber registers correctly for inline function use
From: Hannes Frederic Sowa @ 2014-11-14 15:46 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, ogerlitz, pshelar, jesse, jay.vosburgh, discuss
In-Reply-To: <1415979181.17262.45.camel@edumazet-glaptop2.roam.corp.google.com>

On Fr, 2014-11-14 at 07:33 -0800, Eric Dumazet wrote:
> On Fri, 2014-11-14 at 16:13 +0100, Hannes Frederic Sowa wrote:
> > > 
> > > 
> > > Thats a lot of clobbers.
> > 
> > Yes, those are basically all callee-clobbered registers for the
> > particular architecture. I didn't look at the generated code for jhash
> > and crc_hash because I want this code to always be safe, independent of
> > the version and optimization levels of gcc.
> > 
> > > Alternative would be to use an assembly trampoline to save/restore them
> > > before calling __jhash2
> > 
> > This version provides the best hints on how to allocate registers to the
> > optimizers. E.g. it could avoid using callee-clobbered registers but use
> > callee-saved ones. If we build a trampoline, we need to save and reload
> > all registers all the time. This version just lets gcc decide how to do
> > that.
> > 
> > > __intel_crc4_2_hash2 can probably be written in assembly, it is quite
> > > simple.
> > 
> > Sure, but all the pre and postconditions must hold for both, jhash and
> > intel_crc4_2_hash and I don't want to rewrite jhash in assembler.
> 
> We write optimized code for current cpus.
> 
> With current generation of cpus, we have crc32 support.

__intel_crc4_2_hash(2) does already make use of crc32 instruction. I'll
have a closer look at what gcc generates.

> The fallback having to save/restore few registers, we don't care, as the
> fallback has huge cost anyway.
> 
> You don't have to write jhash() in assembler, you misunderstood me.

Ok, understood, so we only clobber the registers needed in the
crc32_hash implementation and only if we branch to jhash we save all the
other ones in a trampoline directly before jhash.

> We only have to provide a trampoline in assembler, with maybe 10
> instructions.
> 
> Then gcc will know that we do not clobber registers for the optimized
> case.

Yes, makes sense.

I would still like to see the current proposed fix getting applied and
we can do this on-top. The inline call after this patch reassembles a
direct function call, so besides the long list of clobbers, it should
still be pretty fast.

Thanks,
Hannes

^ permalink raw reply

* Re: [PATCH v7 6/8] net: can: c_can: Disable pins when CAN interface is down
From: Marc Kleine-Budde @ 2014-11-14 15:49 UTC (permalink / raw)
  To: Roger Quadros, wg
  Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
	sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <5466225D.2070202@ti.com>

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

On 11/14/2014 04:40 PM, Roger Quadros wrote:
> DRA7 CAN IP suffers from a problem which causes it to be prevented
> from fully turning OFF (i.e. stuck in transition) if the module was
> disabled while there was traffic on the CAN_RX line.
> 
> To work around this issue we select the SLEEP pin state by default
> on probe and use the DEFAULT pin state on CAN up and back to the
> SLEEP pin state on CAN down.
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>

Wow! Some interations later this patch got quite nice and shiny :)
Applied all to can-next/master.

Thanks,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


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

^ permalink raw reply

* Re: [PATCH v4 7/8] net: can: c_can: Add support for TI DRA7 DCAN
From: Marc Kleine-Budde @ 2014-11-14 15:56 UTC (permalink / raw)
  To: Roger Quadros, wg
  Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
	sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <1415371762-29885-8-git-send-email-rogerq@ti.com>

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

On 11/07/2014 03:49 PM, Roger Quadros wrote:
> DRA7 SoC has 2 CAN IPs. Provide compatible IDs and RAMINIT
> register data for both.
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  Documentation/devicetree/bindings/net/can/c_can.txt |  1 +
>  drivers/net/can/c_can/c_can_platform.c              | 11 +++++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt
> index a3ca3ee..f682fdb 100644
> --- a/Documentation/devicetree/bindings/net/can/c_can.txt
> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt
> @@ -4,6 +4,7 @@ Bosch C_CAN/D_CAN controller Device Tree Bindings
>  Required properties:
>  - compatible		: Should be "bosch,c_can" for C_CAN controllers and
>  			  "bosch,d_can" for D_CAN controllers.
> +			  Can be "ti,dra7-d_can".
>  - reg			: physical base address and size of the C_CAN/D_CAN
>  			  registers map
>  - interrupts		: property with a value describing the interrupt
> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> index 71b9063..7a81db4 100644
> --- a/drivers/net/can/c_can/c_can_platform.c
> +++ b/drivers/net/can/c_can/c_can_platform.c
> @@ -195,6 +195,16 @@ static struct c_can_driver_data d_can_drvdata = {
>  	.id = BOSCH_D_CAN,
>  };
>  
> +static u8 dra7_raminit_start_bits[] = {3, 5};
> +static u8 dra7_raminit_done_bits[] = {1, 2};
> +static struct c_can_driver_data dra7_dcan_drvdata = {
> +	.id = BOSCH_D_CAN,
> +	.num_can = 2,
                   ^
Replaced by ARRAY_SIZE(dra7_raminit_start_bits)

Same for the am3352_dcan_drvdata in the next patch.

> +	.raminit_start_bits = dra7_raminit_start_bits,
> +	.raminit_done_bits = dra7_raminit_done_bits,
> +	.raminit_pulse = true,
> +};

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


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

^ permalink raw reply

* Re: [PATCH 1/3] arch: Introduce load_acquire() and store_release()
From: Alexander Duyck @ 2014-11-14 16:00 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-arch@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, mikey@neuling.org,
	tony.luck@intel.com, mathieu.desnoyers@polymtl.ca,
	donald.c.skidmore@intel.com, peterz@infradead.org,
	benh@kernel.crashing.org, heiko.carstens@de.ibm.com,
	oleg@redhat.com, davem@davemloft.net, michael@ellerman.id.au,
	matthew.vick@intel.com, nic_swsd@realtek.com,
	geert@linux-m68k.org, jeffrey.t.kirsher@intel.com
In-Reply-To: <20141114101902.GA27963@arm.com>


On 11/14/2014 02:19 AM, Will Deacon wrote:
> Hi Alex,
>
> On Thu, Nov 13, 2014 at 07:27:23PM +0000, Alexander Duyck wrote:
>> It is common for device drivers to make use of acquire/release semantics
>> when dealing with descriptors stored in device memory.  On reviewing the
>> documentation and code for smp_load_acquire() and smp_store_release() as
>> well as reviewing an IBM website that goes over the use of PowerPC barriers
>> at http://www.ibm.com/developerworks/systems/articles/powerpc.html it
>> occurred to me that the same code could likely be applied to device drivers.
>>
>> As a result this patch introduces load_acquire() and store_release().  The
>> load_acquire() function can be used in the place of situations where a test
>> for ownership must be followed by a memory barrier.  The below example is
>> from ixgbe:
>>
>>          if (!rx_desc->wb.upper.status_error)
>>                  break;
>>
>>          /* This memory barrier is needed to keep us from reading
>>           * any other fields out of the rx_desc until we know the
>>           * descriptor has been written back
>>           */
>>          rmb();
>>
>> With load_acquire() this can be changed to:
>>
>>          if (!load_acquire(&rx_desc->wb.upper.status_error))
>>                  break;
> I still don't think this is a good idea for the specific use-case you're
> highlighting.
>
> On ARM, an mb() can be *significantly* more expensive than an rmb() (since
> we may have to drain store buffers on an outer L2 cache) and on arm64 it's
> not at all clear that an LDAR is more efficient than an LDR; DMB LD
> sequence. I can certainly imagine implementations where the latter would
> be preferred.

Yeah, I am pretty sure I overdid it in using a mb() for arm.  I think 
what I should probably be using is something like dmb(ish) which is used 
for smp_mb() instead.  The general idea is to enforce memory-memory 
accesses.  The memory-mmio accesses still should be using a full 
rmb()/wmb() barrier.

The alternative I am mulling over is creating something like a 
lightweight set of memory barriers named lw_mb(), lw_rmb(), lw_wmb(), 
that could be used instead.  The general idea is that on many 
architectures a full mb/rmb/wmb is far too much for just guaranteeing 
ordering for system memory only writes or reads.  I'm thinking I could 
probably use the smp_ varieties as a template for them since I'm 
thinking that in most cases this should be correct.

Also, just to be clear I am not advocating replacing the wmb() in most 
I/O setups where we have to sync the system memory before doing the MMIO 
write.  This is for the case where the device descriptor ring has some 
bit indicating ownership by either the device or the CPU.  So for 
example on the r8169 they have to do a wmb() before writing the DescOwn 
bit in the first descriptor of a given set of Tx descriptors to 
guarantee the rest are written, then they set the DescOwn bit, then they 
call wmb() again to flush that last bit before notifying the device it 
can start fetching the descriptors. My goal is to deal with that first 
wmb() and leave the second as it since it is correct.

> So, whilst I'm perfectly fine to go along with mandatory acquire/release
> macros (we should probably add a check to barf on __iomem pointers), I
> don't agree with using them in preference to finer-grained read/write
> barriers. Doing so will have a real impact on I/O performance.

Couldn't that type of check be added to compiletime_assert_atomic_type?  
That seems like that would be the best place for something like that.


> Finally, do you know of any architectures where load_acquire/store_release
> aren't implemented the same way as the smp_* variants on SMP kernels?
>
> Will

I should probably go back through and sort out the cases where mb() and 
smp_mb() are not the same thing.  I think I probably went with too harsh 
of a barrier in probably a couple of other cases.

Thanks,

Alex

^ permalink raw reply

* [PATCH] e100: fix typo in MDI/MDI-X eeprom check in e100_phy_init
From: John W. Linville @ 2014-11-14 15:59 UTC (permalink / raw)
  To: netdev
  Cc: Dave Miller, Jeff Kirsher, Auke Kok, Malli Chilakala,
	John W. Linville

Although it doesn't explicitly say so, commit 60ffa478759f39a2 ("e100:
Fix MDIO/MDIO-X") appears to be intended to revert the earlier commit
648951451e6d2d53 ("e100: fixed e100 MDI/MDI-X issues").  However,
careful examination reveals that the attempted revert actually
_inverted_ the test for eeprom_mdix_enabled.  That is bound to program
a few PHYs incorrectly...

https://bugzilla.redhat.com/show_bug.cgi?id=1156417

Signed-off-by: John W. Linville <linville@tuxdriver.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Auke Kok <auke-jan.h.kok@intel.com>
Cc: Malli Chilakala <mallikarjuna.chilakala@intel.com>
---
Wow, an 8 year old bug in e100 -- woohoo!! :-)

This was causing some serious flakiness for a large cash register
deployment in Europe.  Testing with this one-line (really,
one-character) patch seems to have resolved the issue.

 drivers/net/ethernet/intel/e100.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
index 781065eb5431..e9c3a87e5b11 100644
--- a/drivers/net/ethernet/intel/e100.c
+++ b/drivers/net/ethernet/intel/e100.c
@@ -1543,7 +1543,7 @@ static int e100_phy_init(struct nic *nic)
 		mdio_write(netdev, nic->mii.phy_id, MII_BMCR, bmcr);
 	} else if ((nic->mac >= mac_82550_D102) || ((nic->flags & ich) &&
 	   (mdio_read(netdev, nic->mii.phy_id, MII_TPISTATUS) & 0x8000) &&
-		!(nic->eeprom[eeprom_cnfg_mdix] & eeprom_mdix_enabled))) {
+		(nic->eeprom[eeprom_cnfg_mdix] & eeprom_mdix_enabled))) {
 		/* enable/disable MDI/MDI-X auto-switching. */
 		mdio_write(netdev, nic->mii.phy_id, MII_NCONFIG,
 				nic->mii.force_media ? 0 : NCONFIG_AUTO_SWITCH);
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH v7 6/8] net: can: c_can: Disable pins when CAN interface is down
From: Roger Quadros @ 2014-11-14 16:04 UTC (permalink / raw)
  To: Marc Kleine-Budde, wg
  Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
	sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <54662493.5040002@pengutronix.de>

On 11/14/2014 05:49 PM, Marc Kleine-Budde wrote:
> On 11/14/2014 04:40 PM, Roger Quadros wrote:
>> DRA7 CAN IP suffers from a problem which causes it to be prevented
>> from fully turning OFF (i.e. stuck in transition) if the module was
>> disabled while there was traffic on the CAN_RX line.
>>
>> To work around this issue we select the SLEEP pin state by default
>> on probe and use the DEFAULT pin state on CAN up and back to the
>> SLEEP pin state on CAN down.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
> 
> Wow! Some interations later this patch got quite nice and shiny :)
> Applied all to can-next/master.

Thanks Marc :)

cheers,
-roger


^ permalink raw reply

* Re: [PATCH v2 net-next 1/7] bpf: add 'flags' attribute to BPF_MAP_UPDATE_ELEM command
From: Hannes Frederic Sowa @ 2014-11-14 16:06 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David S. Miller, Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Eric Dumazet, Linux API, Network Development, LKML
In-Reply-To: <CAMEtUux2vuN3iNaAsxybyFwzfMRVe1+BOW-JvnvYYh+=-vpELw@mail.gmail.com>

On Fr, 2014-11-14 at 07:33 -0800, Alexei Starovoitov wrote:
> On Fri, Nov 14, 2014 at 4:11 AM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
> > On Do, 2014-11-13 at 17:36 -0800, Alexei Starovoitov wrote:
> >> the current meaning of BPF_MAP_UPDATE_ELEM syscall command is:
> >> either update existing map element or create a new one.
> >> Initially the plan was to add a new command to handle the case of
> >> 'create new element if it didn't exist', but 'flags' style looks
> >> cleaner and overall diff is much smaller (more code reused), so add 'flags'
> >> attribute to BPF_MAP_UPDATE_ELEM command with the following meaning:
> >>  #define BPF_ANY      0 /* create new element or update existing */
> >>  #define BPF_NOEXIST  1 /* create new element if it didn't exist */
> >>  #define BPF_EXIST    2 /* update existing element */
> >
> > Would a cmpxchg-alike function be handy here?
> 
> you mean cmpxchg command in addition to
> update() command ?
> May be... it will have an extra 'value' argument
> (key, old_value, new_value)
> I don't have a use case for it yet though.

I don't neither. ;)

I just wanted to bring this up before user space api might get public
and the additional argument might make problems.

Bye,
Hannes

^ permalink raw reply

* Re: [PATCH net-next] icmp: Remove some spurious dropped packet profile hits from the ICMP path
From: Rick Jones @ 2014-11-14 16:16 UTC (permalink / raw)
  To: Eric Dumazet, Rick Jones; +Cc: netdev, davem
In-Reply-To: <1415931471.17262.27.camel@edumazet-glaptop2.roam.corp.google.com>

>
> This looks quite complicated to me.
>
> Why are you adding kfree_skb() everywhere instead of :
>
> 	bool to_consume = icmp_pointers[icmph->type].handler(skb);
> 	if (ro_consume)
> 		consume_skb(skb);
> 	else
> 		kfree_skb(skb);

I thought the point of the drop profiling was to show where the drops 
were happening.  Leaving the kfree_skb() up in icmp_rcv() does not 
improve showing where the drops happened.  That is why I've pushed it 
down into the routines called by icmp_rcv().

rick

^ permalink raw reply

* Re: Understanding what's going on when using a Huawei E173 USB 3G web-stick (UMTS/HSPA)
From: Dan Williams @ 2014-11-14 16:18 UTC (permalink / raw)
  To: sedat.dilek
  Cc: Greg KH, David S. Miller, netdev@vger.kernel.org, linux-usb,
	Aleksander Morgado
In-Reply-To: <CA+icZUX6caBKnjMv6V_26y_-VNfDqkeXFXKJhDdQpZy7W4oofw@mail.gmail.com>

On Fri, 2014-11-14 at 11:56 +0100, Sedat Dilek wrote:
> On Wed, Nov 12, 2014 at 2:21 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> > On Tue, Nov 4, 2014 at 5:55 PM, Dan Williams <dcbw@redhat.com> wrote:
> >> On Tue, 2014-11-04 at 16:11 +0100, Sedat Dilek wrote:
> >>> Hi,
> >>>
> >>> I wanted to understand what is going on the kernel-side when
> >>> connecting to the Internet via a Huawei E173 USB web-stick (3rd
> >>> Generation: UMTS / HSPA).
> >>>
> >>> Especially the correlation between the diverse USB/NET kernel-drivers
> >>> and how the networking is setup.
> >>
> >
> > [ Sitting in front of a foreign Windows machine ]
> >
> > [ CC Aleksander ]
> >
> > Hi Dan,
> >
> > sorry for the late (and short) response.
> >
> > AFAICS you have given a "skeleton" for a "usb-wwan-networking"
> > documentation :-).
> >
> > Personally, I would like to take into account some kernel-config
> > options and some more things.
> >
> 
> I started with documenting...
> 
> I have still some difficulties in understanding USB WWAN Networking.
> So, this is what I revealed...
> 
> ##### USB: HUAWEI E173 3G/UMTS/HSPA INTERNET STICK
> 
> ### USB-NETWORKING AND WWAN SETUP
> CONFIG_USB_USBNET=m        <--- usb networking
> CONFIG_USB_NET_CDCETHER=m  <--- usb-wwan (net) configuration
> CONFIG_USB_SERIAL_WWAN=m   <--- usb-wwan (serial) configuration
> CONFIG_USB_SERIAL_OPTION=m <--- usb-serial driver called "option"

Most WWAN devices actually require option, because most WWAN devices
have "serial" ports (even if they aren't used for PPP), and 'option' is
the driver that handles this.  The 'option' name is historic, but the
driver should really be called something like 'wwan-serial-generic' or
something like that.

You're missing a few other "net" type drivers:

CONFIG_USB_NET_QMI_WWAN = Qualcomm QMI capable devices (net)
CONFIG_USB_HSO = "Option High-Speed" (net) devices
CONFIG_USB_NET_KALMIA = Samsung LTE dongle (net)
CONFIG_USB_SIERRA_NET = Sierra devices (net)
CONFIG_USB_NET_CDC_NCM = Ericsson F5321 (?) and some others (net)
CONFIG_USB_NET_HUAWEI_CDC_NCM = Huawei NCM variant (net)
CONFIG_USB_VL600 = LG VL600 / AD600 LTE device (net)
CONFIG_USB_NET_CDC_MBIM = MBIM (net) devices (popular currently)

and maybe even:

CONFIG_USB_CDC_PHONET = Nokia phones and USB sticks, not "net" but
proprietary protocol that handles both data/control channels

For the "serial" side:

CONFIG_USB_ACM = generic "serial" devices, many are *not* WWAN but many
WWAN devices use this class/driver
CONFIG_USB_SERIAL_QCAUX = Various Qualcomm-based devices' "auxiliary"
ports (DIAG, GPS, etc)
CONFIG_USB_SERIAL_QUALCOMM = Firmware loading and "auxiliary" ports on
various Qualcomm Gobi devices
CONFIG_USB_SERIAL_SIERRAWIRELESS = Older Sierra device serial ports for
PPP/control and "auxiliary" functions

WWAN devices basically mix & match these drivers depending on what
interfaces the firmware exposes.

For example, some Sierra devices may require both
CONFIG_USB_SERIAL_SIERRAWIRELESS and CONFIG_USB_SIERRA_NET.

Older Sierra devices may use only CONFIG_USB_SERIAL_SIERRAWIRELESS and
do not provide a 'net' port at all, but use only PPP.

Sierra devices based on Icera chips may use CONFIG_USB_ACM and either
CONFIG_USB_SIERRA_NET or CONFIG_USB_NET_CDCETHER.

Some Huawei devices may use CONFIG_USB_NET_CDCETHER and either
CONFIG_USB_SERIAL_OPTION or CONFIG_USB_ACM.

Other Huawei devices may use CONFIG_USB_NET_QMI_WWAN and
CONFIG_USB_SERIAL_OPTION.

Even other Huawei devices may be Qualcomm Gobi type and use
CONFIG_USB_NET_QMI_WWAN and CONFIG_USB_SERIAL_QUALCOMM.

So you see it really depends on exactly how the firmware is implemented.
But in general, devices still fall into the categories I originally
listed, and the drivers fall into specific categories too ("net",
"serial", "proprietary"), and devices mix & match drivers to achieve the
end result.

Dan

> ### PPP OPTIONS
> CONFIG_PPP=y
> CONFIG_PPP_BSDCOMP=m
> CONFIG_PPP_DEFLATE=m
> CONFIG_PPP_FILTER=y
> CONFIG_PPP_MULTILINK=y
> CONFIG_PPP_ASYNC=m
> 
> Beyond the PPP options, I wanted to understand what
> CONFIG_USB_NET_CDCETHER does and why I need it.
> Can someone help?




> Thanks.
> 
> - Sedat -
> 
> [1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/usb/Kconfig#n189
> 
> P.S.: cdc_ether Kconfig option and checking my logs
> 
> From [1]...
> ...
> config USB_NET_CDCETHER
> tristate "CDC Ethernet support (smart devices such as cable modems)"
> depends on USB_USBNET
> default y
> help
>  This option supports devices conforming to the Communication Device
>  Class (CDC) Ethernet Control Model, a specification that's easy to
>  implement in device firmware.  The CDC specifications are available
>  from <http://www.usb.org/>.
> 
>  CDC Ethernet is an implementation option for DOCSIS cable modems
>  that support USB connectivity, used for non-Microsoft USB hosts.
>  The Linux-USB CDC Ethernet Gadget driver is an open implementation.
>    This driver should work with at least the following devices:
> 
>    * Dell Wireless 5530 HSPA
>      * Ericsson PipeRider (all variants)
>    * Ericsson Mobile Broadband Module (all variants)
>      * Motorola (DM100 and SB4100)
>      * Broadcom Cable Modem (reference design)
>    * Toshiba (PCX1100U and F3507g/F3607gw)
>    * ...
> 
>  This driver creates an interface named "ethX", where X depends on
>  what other networking devices you have in use.  However, if the
>  IEEE 802 "local assignment" bit is set in the address, a "usbX"
>  name is used instead.
> ...
> 
> From my logs...
> 
> $ dmesg | egrep -i 'option|wwan|ppp|3-1.2|huawei|gsm|modem'
> [    0.000000] please try 'cgroup_disable=memory' option if you don't
> want memory cgroups
> [    0.549498] PPP generic driver version 2.4.2
> [    1.299059] usb 3-1.2: new high-speed USB device number 3 using ehci-pci
> [    1.394084] usb 3-1.2: New USB device found, idVendor=12d1, idProduct=1436
> [    1.394095] usb 3-1.2: New USB device strings: Mfr=4, Product=3,
> SerialNumber=0
> [    1.394100] usb 3-1.2: Product: HUAWEI Mobile
> [    1.394103] usb 3-1.2: Manufacturer: HUAWEI Technology
> [    2.115424] usb-storage 3-1.2:1.0: USB Mass Storage device detected
> [    2.125026] usb-storage 3-1.2:1.1: USB Mass Storage device detected
> [    2.125607] usb-storage 3-1.2:1.2: USB Mass Storage device detected
> [    2.125888] usb-storage 3-1.2:1.3: USB Mass Storage device detected
> [    2.126187] usb-storage 3-1.2:1.4: USB Mass Storage device detected
> [    2.126461] usb-storage 3-1.2:1.5: USB Mass Storage device detected
> [    2.127098] scsi host11: usb-storage 3-1.2:1.5
> [    2.129370] usb-storage 3-1.2:1.6: USB Mass Storage device detected
> [    2.131685] scsi host12: usb-storage 3-1.2:1.6
> [    3.127317] scsi 11:0:0:0: CD-ROM            HUAWEI   Mass Storage
>    2.31 PQ: 0 ANSI: 2
> [    3.137589] scsi 12:0:0:0: Direct-Access     HUAWEI   SD Storage
>    2.31 PQ: 0 ANSI: 2
> [   13.500302] cdc_ether 3-1.2:1.1 wwan0: register 'cdc_ether' at
> usb-0000:00:1a.0-1.2, Mobile Broadband Network Device,
> 02:50:f3:00:00:00
> [   14.160221] usbcore: registered new interface driver option
> [   14.160820] usbserial: USB Serial support registered for GSM modem (1-port)
> [   14.160940] option 3-1.2:1.0: GSM modem (1-port) converter detected
> [   14.163032] usb 3-1.2: GSM modem (1-port) converter now attached to ttyUSB0
> [   14.163305] option 3-1.2:1.3: GSM modem (1-port) converter detected
> [   14.163676] usb 3-1.2: GSM modem (1-port) converter now attached to ttyUSB1
> [   14.163742] option 3-1.2:1.4: GSM modem (1-port) converter detected
> [   14.165227] usb 3-1.2: GSM modem (1-port) converter now attached to ttyUSB2
> [   72.877065] PPP BSD Compression module registered
> [   72.881701] PPP Deflate Compression module registered
> - EOT -
> --
> 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 v2 net-next 1/7] bpf: add 'flags' attribute to BPF_MAP_UPDATE_ELEM command
From: Alexei Starovoitov @ 2014-11-14 16:31 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: David S. Miller, Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Eric Dumazet, Linux API, Network Development, LKML
In-Reply-To: <1415981203.15154.45.camel@localhost>

On Fri, Nov 14, 2014 at 8:06 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> On Fr, 2014-11-14 at 07:33 -0800, Alexei Starovoitov wrote:
>> On Fri, Nov 14, 2014 at 4:11 AM, Hannes Frederic Sowa
>> <hannes@stressinduktion.org> wrote:
>> > On Do, 2014-11-13 at 17:36 -0800, Alexei Starovoitov wrote:
>> >> the current meaning of BPF_MAP_UPDATE_ELEM syscall command is:
>> >> either update existing map element or create a new one.
>> >> Initially the plan was to add a new command to handle the case of
>> >> 'create new element if it didn't exist', but 'flags' style looks
>> >> cleaner and overall diff is much smaller (more code reused), so add 'flags'
>> >> attribute to BPF_MAP_UPDATE_ELEM command with the following meaning:
>> >>  #define BPF_ANY      0 /* create new element or update existing */
>> >>  #define BPF_NOEXIST  1 /* create new element if it didn't exist */
>> >>  #define BPF_EXIST    2 /* update existing element */
>> >
>> > Would a cmpxchg-alike function be handy here?
>>
>> you mean cmpxchg command in addition to
>> update() command ?
>> May be... it will have an extra 'value' argument
>> (key, old_value, new_value)
>> I don't have a use case for it yet though.
>
> I don't neither. ;)
>
> I just wanted to bring this up before user space api might get public
> and the additional argument might make problems.

addition of cmpxchg command won't be a problem obviously.
(just another 'new_value' field in existing struct inside bpf_attr union).

^ permalink raw reply

* Re: [PATCH v5 4/8] net: can: c_can: Add syscon/regmap RAMINIT mechanism
From: Marc Kleine-Budde @ 2014-11-14 16:32 UTC (permalink / raw)
  To: Roger Quadros, wg
  Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
	sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <546621C3.3010804@ti.com>

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

On 11/14/2014 04:37 PM, Roger Quadros wrote:
> Some TI SoCs like DRA7 have a RAMINIT register specification
> different from the other AMxx SoCs and as expected by the
> existing driver.
> 
> To add more insanity, this register is shared with other
> IPs like DSS, PCIe and PWM.
> 
> Provides a more generic mechanism to specify the RAMINIT
> register location and START/DONE bit position and use the
> syscon/regmap framework to access the register.
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  .../devicetree/bindings/net/can/c_can.txt          |   3 +
>  drivers/net/can/c_can/c_can.h                      |  11 +-
>  drivers/net/can/c_can/c_can_platform.c             | 113 ++++++++++++++-------
>  3 files changed, 87 insertions(+), 40 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt
> index 8f1ae81..a3ca3ee 100644
> --- a/Documentation/devicetree/bindings/net/can/c_can.txt
> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt
> @@ -12,6 +12,9 @@ Required properties:
>  Optional properties:
>  - ti,hwmods		: Must be "d_can<n>" or "c_can<n>", n being the
>  			  instance number
> +- syscon-raminit	: Handle to system control region that contains the
> +			  RAMINIT register, register offset to the RAMINIT
> +			  register and the CAN instance number (0 offset).
>  
>  Note: "ti,hwmods" field is used to fetch the base address and irq
>  resources from TI, omap hwmod data base during device registration.
> diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> index 3c305a1..0e17c7b 100644
> --- a/drivers/net/can/c_can/c_can.h
> +++ b/drivers/net/can/c_can/c_can.h
> @@ -179,6 +179,14 @@ struct c_can_driver_data {
>  	bool raminit_pulse;	/* If set, sets and clears START bit (pulse) */
>  };
>  
> +/* Out of band RAMINIT register access via syscon regmap */
> +struct c_can_raminit {
> +	struct regmap *syscon;	/* for raminit ctrl. reg. access */
> +	unsigned int reg;	/* register index within syscon */
> +	u8 start_bit;
> +	u8 done_bit;
> +};
> +
>  /* c_can private data structure */
>  struct c_can_priv {
>  	struct can_priv can;	/* must be the first member */
> @@ -196,8 +204,7 @@ struct c_can_priv {
>  	const u16 *regs;
>  	void *priv;		/* for board-specific data */
>  	enum c_can_dev_id type;
> -	u32 __iomem *raminit_ctrlreg;
> -	int instance;
> +	struct c_can_raminit raminit_sys;	/* RAMINIT via syscon regmap */
>  	void (*raminit) (const struct c_can_priv *priv, bool enable);
>  	u32 comm_rcv_high;
>  	u32 rxmasked;
> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> index 1546c2b..89739a1 100644
> --- a/drivers/net/can/c_can/c_can_platform.c
> +++ b/drivers/net/can/c_can/c_can_platform.c
> @@ -32,14 +32,13 @@
>  #include <linux/clk.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
>  
>  #include <linux/can/dev.h>
>  
>  #include "c_can.h"
>  
> -#define CAN_RAMINIT_START_MASK(i)	(0x001 << (i))
> -#define CAN_RAMINIT_DONE_MASK(i)	(0x100 << (i))
> -#define CAN_RAMINIT_ALL_MASK(i)		(0x101 << (i))
>  #define DCAN_RAM_INIT_BIT		(1 << 3)
>  static DEFINE_SPINLOCK(raminit_lock);
>  /*
> @@ -72,47 +71,61 @@ static void c_can_plat_write_reg_aligned_to_32bit(const struct c_can_priv *priv,
>  	writew(val, priv->base + 2 * priv->regs[index]);
>  }
>  
> -static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask,
> -				  u32 val)
> +static void c_can_hw_raminit_wait_syscon(const struct c_can_priv *priv,
> +					 u32 mask, u32 val)
>  {
>  	int timeout = 0;
> +	const struct c_can_raminit *raminit = &priv->raminit_sys;
> +	u32 ctrl;
> +
>  	/* We look only at the bits of our instance. */
>  	val &= mask;
> -	while ((readl(priv->raminit_ctrlreg) & mask) != val) {
> +	do {
>  		udelay(1);
>  		timeout++;
>  
> +		regmap_read(raminit->syscon, raminit->reg, &ctrl);
>  		if (timeout == 1000) {
>  			dev_err(&priv->dev->dev, "%s: time out\n", __func__);
>  			break;
>  		}
> -	}
> +	} while ((ctrl & mask) != val);
>  }
>  
> -static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable)
> +static void c_can_hw_raminit_syscon(const struct c_can_priv *priv, bool enable)
>  {
> -	u32 mask = CAN_RAMINIT_ALL_MASK(priv->instance);
> +	u32 mask;
>  	u32 ctrl;
> +	const struct c_can_raminit *raminit = &priv->raminit_sys;
> +	u8 start_bit, done_bit;
> +
> +	start_bit = raminit->start_bit;
> +	done_bit = raminit->done_bit;
>  
>  	spin_lock(&raminit_lock);
>  
> -	ctrl = readl(priv->raminit_ctrlreg);
> +	mask = 1 << start_bit | 1 << done_bit;
> +	regmap_read(raminit->syscon, raminit->reg, &ctrl);
> +
>  	/* We clear the done and start bit first. The start bit is
>  	 * looking at the 0 -> transition, but is not self clearing;
>  	 * And we clear the init done bit as well.
> +	 * NOTE: DONE must be written with 1 to clear it.
>  	 */
> -	ctrl &= ~CAN_RAMINIT_START_MASK(priv->instance);
> -	ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
> -	writel(ctrl, priv->raminit_ctrlreg);
> -	ctrl &= ~CAN_RAMINIT_DONE_MASK(priv->instance);
> -	c_can_hw_raminit_wait_ti(priv, mask, ctrl);
> +	ctrl &= ~(1 << start_bit);
> +	ctrl |= 1 << done_bit;
> +	regmap_write(raminit->syscon, raminit->reg, ctrl);
> +
> +	ctrl &= ~(1 << done_bit);
> +	c_can_hw_raminit_wait_syscon(priv, mask, ctrl);
>  
>  	if (enable) {
>  		/* Set start bit and wait for the done bit. */
> -		ctrl |= CAN_RAMINIT_START_MASK(priv->instance);
> -		writel(ctrl, priv->raminit_ctrlreg);
> -		ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
> -		c_can_hw_raminit_wait_ti(priv, mask, ctrl);
> +		ctrl |= 1 << start_bit;
> +		regmap_write(raminit->syscon, raminit->reg, ctrl);
> +
> +		ctrl |= 1 << done_bit;
> +		c_can_hw_raminit_wait_syscon(priv, mask, ctrl);
>  	}
>  	spin_unlock(&raminit_lock);
>  }

My arm gcc-4.7.2 spits this warnings, I'll initialize ctrl to 0.

> drivers/net/can/c_can/c_can_platform.c: In function 'c_can_hw_raminit_wait_syscon':
> drivers/net/can/c_can/c_can_platform.c:92:17: warning: 'ctrl' may be used uninitialized in this function [-Wuninitialized]
> drivers/net/can/c_can/c_can_platform.c: In function 'c_can_hw_raminit_syscon':
> drivers/net/can/c_can/c_can_platform.c:115:7: warning: 'ctrl' is used uninitialized in this function [-Wuninitialized]

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


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

^ permalink raw reply

* Re: [PATCH nf] netfilter: conntrack: fix race in __nf_conntrack_confirm against get_next_corpse
From: Pablo Neira Ayuso @ 2014-11-14 16:40 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: programme110, netfilter-devel, Florian Westphal, netdev,
	Patrick McHardy, Joerg Marx
In-Reply-To: <20141112083500.5404e5f4@redhat.com>

On Wed, Nov 12, 2014 at 08:35:00AM +0100, Jesper Dangaard Brouer wrote:
> > > -	/* We have to check the DYING flag inside the lock to prevent
> > > +
> > > +	/* We have to check the DYING flag after unlink to prevent
> > >  	   a race against nf_ct_get_next_corpse() possibly called from
> > >  	   user context, else we insert an already 'dead' hash, blocking
> > >  	   further use of that particular connection -JM */
> > 
> > While at this, I think it would be good to fix comment style to:
> > 
> >         /* We have ...
> >          * ...
> >          */
> > 
> > I can fix this here, no need to resend, just let me know.
> 
> Okay, I was just trying to keep the changes as minimal as possible, if
> this should go into a stable-kernel.  Your choice.

I'm going to take this patch including the comment style fix, I would
like to avoid specific patches to fix coding style issues, and the
first line of this comment is updated. I think the patch will be still
small to fulfill -stable rules.

I'll send a follow a patch to change the return verdict to NF_DROP to
not mix up different things.

Thanks!

^ permalink raw reply

* Re: [PATCH v5 4/8] net: can: c_can: Add syscon/regmap RAMINIT mechanism
From: Roger Quadros @ 2014-11-14 16:42 UTC (permalink / raw)
  To: Marc Kleine-Budde, wg
  Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
	sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <54662E9B.9030104@pengutronix.de>

On 11/14/2014 06:32 PM, Marc Kleine-Budde wrote:
> On 11/14/2014 04:37 PM, Roger Quadros wrote:
>> Some TI SoCs like DRA7 have a RAMINIT register specification
>> different from the other AMxx SoCs and as expected by the
>> existing driver.
>>
>> To add more insanity, this register is shared with other
>> IPs like DSS, PCIe and PWM.
>>
>> Provides a more generic mechanism to specify the RAMINIT
>> register location and START/DONE bit position and use the
>> syscon/regmap framework to access the register.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  .../devicetree/bindings/net/can/c_can.txt          |   3 +
>>  drivers/net/can/c_can/c_can.h                      |  11 +-
>>  drivers/net/can/c_can/c_can_platform.c             | 113 ++++++++++++++-------
>>  3 files changed, 87 insertions(+), 40 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt
>> index 8f1ae81..a3ca3ee 100644
>> --- a/Documentation/devicetree/bindings/net/can/c_can.txt
>> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt
>> @@ -12,6 +12,9 @@ Required properties:
>>  Optional properties:
>>  - ti,hwmods		: Must be "d_can<n>" or "c_can<n>", n being the
>>  			  instance number
>> +- syscon-raminit	: Handle to system control region that contains the
>> +			  RAMINIT register, register offset to the RAMINIT
>> +			  register and the CAN instance number (0 offset).
>>  
>>  Note: "ti,hwmods" field is used to fetch the base address and irq
>>  resources from TI, omap hwmod data base during device registration.
>> diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
>> index 3c305a1..0e17c7b 100644
>> --- a/drivers/net/can/c_can/c_can.h
>> +++ b/drivers/net/can/c_can/c_can.h
>> @@ -179,6 +179,14 @@ struct c_can_driver_data {
>>  	bool raminit_pulse;	/* If set, sets and clears START bit (pulse) */
>>  };
>>  
>> +/* Out of band RAMINIT register access via syscon regmap */
>> +struct c_can_raminit {
>> +	struct regmap *syscon;	/* for raminit ctrl. reg. access */
>> +	unsigned int reg;	/* register index within syscon */
>> +	u8 start_bit;
>> +	u8 done_bit;
>> +};
>> +
>>  /* c_can private data structure */
>>  struct c_can_priv {
>>  	struct can_priv can;	/* must be the first member */
>> @@ -196,8 +204,7 @@ struct c_can_priv {
>>  	const u16 *regs;
>>  	void *priv;		/* for board-specific data */
>>  	enum c_can_dev_id type;
>> -	u32 __iomem *raminit_ctrlreg;
>> -	int instance;
>> +	struct c_can_raminit raminit_sys;	/* RAMINIT via syscon regmap */
>>  	void (*raminit) (const struct c_can_priv *priv, bool enable);
>>  	u32 comm_rcv_high;
>>  	u32 rxmasked;
>> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
>> index 1546c2b..89739a1 100644
>> --- a/drivers/net/can/c_can/c_can_platform.c
>> +++ b/drivers/net/can/c_can/c_can_platform.c
>> @@ -32,14 +32,13 @@
>>  #include <linux/clk.h>
>>  #include <linux/of.h>
>>  #include <linux/of_device.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/regmap.h>
>>  
>>  #include <linux/can/dev.h>
>>  
>>  #include "c_can.h"
>>  
>> -#define CAN_RAMINIT_START_MASK(i)	(0x001 << (i))
>> -#define CAN_RAMINIT_DONE_MASK(i)	(0x100 << (i))
>> -#define CAN_RAMINIT_ALL_MASK(i)		(0x101 << (i))
>>  #define DCAN_RAM_INIT_BIT		(1 << 3)
>>  static DEFINE_SPINLOCK(raminit_lock);
>>  /*
>> @@ -72,47 +71,61 @@ static void c_can_plat_write_reg_aligned_to_32bit(const struct c_can_priv *priv,
>>  	writew(val, priv->base + 2 * priv->regs[index]);
>>  }
>>  
>> -static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask,
>> -				  u32 val)
>> +static void c_can_hw_raminit_wait_syscon(const struct c_can_priv *priv,
>> +					 u32 mask, u32 val)
>>  {
>>  	int timeout = 0;
>> +	const struct c_can_raminit *raminit = &priv->raminit_sys;
>> +	u32 ctrl;
>> +
>>  	/* We look only at the bits of our instance. */
>>  	val &= mask;
>> -	while ((readl(priv->raminit_ctrlreg) & mask) != val) {
>> +	do {
>>  		udelay(1);
>>  		timeout++;
>>  
>> +		regmap_read(raminit->syscon, raminit->reg, &ctrl);
>>  		if (timeout == 1000) {
>>  			dev_err(&priv->dev->dev, "%s: time out\n", __func__);
>>  			break;
>>  		}
>> -	}
>> +	} while ((ctrl & mask) != val);
>>  }
>>  
>> -static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable)
>> +static void c_can_hw_raminit_syscon(const struct c_can_priv *priv, bool enable)
>>  {
>> -	u32 mask = CAN_RAMINIT_ALL_MASK(priv->instance);
>> +	u32 mask;
>>  	u32 ctrl;
>> +	const struct c_can_raminit *raminit = &priv->raminit_sys;
>> +	u8 start_bit, done_bit;
>> +
>> +	start_bit = raminit->start_bit;
>> +	done_bit = raminit->done_bit;
>>  
>>  	spin_lock(&raminit_lock);
>>  
>> -	ctrl = readl(priv->raminit_ctrlreg);
>> +	mask = 1 << start_bit | 1 << done_bit;
>> +	regmap_read(raminit->syscon, raminit->reg, &ctrl);
>> +
>>  	/* We clear the done and start bit first. The start bit is
>>  	 * looking at the 0 -> transition, but is not self clearing;
>>  	 * And we clear the init done bit as well.
>> +	 * NOTE: DONE must be written with 1 to clear it.
>>  	 */
>> -	ctrl &= ~CAN_RAMINIT_START_MASK(priv->instance);
>> -	ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
>> -	writel(ctrl, priv->raminit_ctrlreg);
>> -	ctrl &= ~CAN_RAMINIT_DONE_MASK(priv->instance);
>> -	c_can_hw_raminit_wait_ti(priv, mask, ctrl);
>> +	ctrl &= ~(1 << start_bit);
>> +	ctrl |= 1 << done_bit;
>> +	regmap_write(raminit->syscon, raminit->reg, ctrl);
>> +
>> +	ctrl &= ~(1 << done_bit);
>> +	c_can_hw_raminit_wait_syscon(priv, mask, ctrl);
>>  
>>  	if (enable) {
>>  		/* Set start bit and wait for the done bit. */
>> -		ctrl |= CAN_RAMINIT_START_MASK(priv->instance);
>> -		writel(ctrl, priv->raminit_ctrlreg);
>> -		ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
>> -		c_can_hw_raminit_wait_ti(priv, mask, ctrl);
>> +		ctrl |= 1 << start_bit;
>> +		regmap_write(raminit->syscon, raminit->reg, ctrl);
>> +
>> +		ctrl |= 1 << done_bit;
>> +		c_can_hw_raminit_wait_syscon(priv, mask, ctrl);
>>  	}
>>  	spin_unlock(&raminit_lock);
>>  }
> 
> My arm gcc-4.7.2 spits this warnings, I'll initialize ctrl to 0.

My 4.7.3 doesn't. Initializing to 0 is fine as well.

cheers,
-roger

> 
>> drivers/net/can/c_can/c_can_platform.c: In function 'c_can_hw_raminit_wait_syscon':
>> drivers/net/can/c_can/c_can_platform.c:92:17: warning: 'ctrl' may be used uninitialized in this function [-Wuninitialized]
>> drivers/net/can/c_can/c_can_platform.c: In function 'c_can_hw_raminit_syscon':
>> drivers/net/can/c_can/c_can_platform.c:115:7: warning: 'ctrl' is used uninitialized in this function [-Wuninitialized]
> 
> Marc
> 


^ permalink raw reply

* [PATCH 0/8] Netfilter/IPVS fixes for net
From: Pablo Neira Ayuso @ 2014-11-14 16:58 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1415984329-5569-1-git-send-email-pablo@netfilter.org>

Hi David,

The following patchset contains Netfilter updates for your net tree,
they are:

1) Fix missing initialization of the range structure (allocated in the
   stack) in nft_masq_{ipv4, ipv6}_eval, from Daniel Borkmann.

2) Make sure the data we receive from userspace contains the req_version
   structure, otherwise return an error incomplete on truncated input.
   From Dan Carpenter.

3) Fix handling og skb->sk which may cause incorrect handling
   of connections from a local process. Via Simon Horman, patch from
   Calvin Owens.

4) Fix wrong netns in nft_compat when setting target and match params
   structure.

5) Relax chain type validation in nft_compat that was recently included,
   this broke the matches that need to be run from the route chain type.
   Now iptables-test.py automated regression tests report success again
   and we avoid the only possible problematic case, which is the use of
   nat targets out of nat chain type.

6) Use match->table to validate the tablename, instead of the match->name.
   Again patch for nft_compat.

7) Restore the synchronous release of objects from the commit and abort
   path in nf_tables. This is causing two major problems: splats when using
   nft_compat, given that matches and targets may sleep and call_rcu is
   invoked from softirq context. Moreover Patrick reported possible event
   notification reordering when rules refer to anonymous sets.

8) Fix race condition in between packets that are being confirmed by
   conntrack and the ctnetlink flush operation. This happens since the
   removal of the central spinlock. Thanks to Jesper D. Brouer to looking
   into this.

You can pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git

Thanks!

----------------------------------------------------------------

The following changes since commit d52fdbb735c36a209f36a628d40ca9185b349ba7:

  smc91x: retrieve IRQ and trigger flags in a modern way (2014-11-01 17:04:20 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master

for you to fetch changes up to 5195c14c8b27cc0b18220ddbf0e5ad3328a04187:

  netfilter: conntrack: fix race in __nf_conntrack_confirm against get_next_corpse (2014-11-14 17:43:05 +0100)

----------------------------------------------------------------
Calvin Owens (1):
      ipvs: Keep skb->sk when allocating headroom on tunnel xmit

Dan Carpenter (1):
      netfilter: ipset: small potential read beyond the end of buffer

Daniel Borkmann (1):
      netfilter: nft_masq: fix uninitialized range in nft_masq_{ipv4, ipv6}_eval

Pablo Neira Ayuso (4):
      netfilter: nft_compat: use current net namespace
      netfilter: nft_compat: relax chain type validation
      netfilter: nft_compat: use the match->table to validate dependencies
      netfilter: nf_tables: restore synchronous object release from commit/abort

bill bonaparte (1):
      netfilter: conntrack: fix race in __nf_conntrack_confirm against get_next_corpse

 include/net/netfilter/nf_tables.h  |    2 --
 net/ipv4/netfilter/nft_masq_ipv4.c |    1 +
 net/ipv6/netfilter/nft_masq_ipv6.c |    1 +
 net/netfilter/ipset/ip_set_core.c  |    6 ++++++
 net/netfilter/ipvs/ip_vs_xmit.c    |    2 ++
 net/netfilter/nf_conntrack_core.c  |   14 +++++++------
 net/netfilter/nf_tables_api.c      |   24 ++++++++--------------
 net/netfilter/nft_compat.c         |   40 ++++++------------------------------
 8 files changed, 32 insertions(+), 58 deletions(-)

^ permalink raw reply

* [PATCH 1/8] netfilter: nft_masq: fix uninitialized range in nft_masq_{ipv4, ipv6}_eval
From: Pablo Neira Ayuso @ 2014-11-14 16:58 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1415984329-5569-1-git-send-email-pablo@netfilter.org>

From: Daniel Borkmann <dborkman@redhat.com>

When transferring from the original range in nf_nat_masquerade_{ipv4,ipv6}()
we copy over values from stack in from min_proto/max_proto due to uninitialized
range variable in both, nft_masq_{ipv4,ipv6}_eval. As we only initialize
flags at this time from nft_masq struct, just zero out the rest.

Fixes: 9ba1f726bec09 ("netfilter: nf_tables: add new nft_masq expression")
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Acked-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv4/netfilter/nft_masq_ipv4.c |    1 +
 net/ipv6/netfilter/nft_masq_ipv6.c |    1 +
 2 files changed, 2 insertions(+)

diff --git a/net/ipv4/netfilter/nft_masq_ipv4.c b/net/ipv4/netfilter/nft_masq_ipv4.c
index c1023c4..665de06 100644
--- a/net/ipv4/netfilter/nft_masq_ipv4.c
+++ b/net/ipv4/netfilter/nft_masq_ipv4.c
@@ -24,6 +24,7 @@ static void nft_masq_ipv4_eval(const struct nft_expr *expr,
 	struct nf_nat_range range;
 	unsigned int verdict;
 
+	memset(&range, 0, sizeof(range));
 	range.flags = priv->flags;
 
 	verdict = nf_nat_masquerade_ipv4(pkt->skb, pkt->ops->hooknum,
diff --git a/net/ipv6/netfilter/nft_masq_ipv6.c b/net/ipv6/netfilter/nft_masq_ipv6.c
index 8a7ac68..529c119 100644
--- a/net/ipv6/netfilter/nft_masq_ipv6.c
+++ b/net/ipv6/netfilter/nft_masq_ipv6.c
@@ -25,6 +25,7 @@ static void nft_masq_ipv6_eval(const struct nft_expr *expr,
 	struct nf_nat_range range;
 	unsigned int verdict;
 
+	memset(&range, 0, sizeof(range));
 	range.flags = priv->flags;
 
 	verdict = nf_nat_masquerade_ipv6(pkt->skb, &range, pkt->out);
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 3/8] ipvs: Keep skb->sk when allocating headroom on tunnel xmit
From: Pablo Neira Ayuso @ 2014-11-14 16:58 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1415984329-5569-1-git-send-email-pablo@netfilter.org>

From: Calvin Owens <calvinowens@fb.com>

ip_vs_prepare_tunneled_skb() ignores ->sk when allocating a new
skb, either unconditionally setting ->sk to NULL or allowing
the uninitialized ->sk from a newly allocated skb to leak through
to the caller.

This patch properly copies ->sk and increments its reference count.

Signed-off-by: Calvin Owens <calvinowens@fb.com>
Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_xmit.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 437a366..bd90bf8 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -846,6 +846,8 @@ ip_vs_prepare_tunneled_skb(struct sk_buff *skb, int skb_af,
 		new_skb = skb_realloc_headroom(skb, max_headroom);
 		if (!new_skb)
 			goto error;
+		if (skb->sk)
+			skb_set_owner_w(new_skb, skb->sk);
 		consume_skb(skb);
 		skb = new_skb;
 	}
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 7/8] netfilter: nf_tables: restore synchronous object release from commit/abort
From: Pablo Neira Ayuso @ 2014-11-14 16:58 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1415984329-5569-1-git-send-email-pablo@netfilter.org>

The existing xtables matches and targets, when used from nft_compat, may
sleep from the destroy path, ie. when removing rules. Since the objects
are released via call_rcu from softirq context, this results in lockdep
splats and possible lockups that may be hard to reproduce.

Patrick also indicated that delayed object release via call_rcu can
cause us problems in the ordering of event notifications when anonymous
sets are in place.

So, this patch restores the synchronous object release from the commit
and abort paths. This includes a call to synchronize_rcu() to make sure
that no packets are walking on the objects that are going to be
released. This is slowier though, but it's simple and it resolves the
aforementioned problems.

This is a partial revert of c7c32e7 ("netfilter: nf_tables: defer all
object release via rcu") that was introduced in 3.16 to speed up
interaction with userspace.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_tables.h |    2 --
 net/netfilter/nf_tables_api.c     |   24 ++++++++----------------
 2 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 845c596..3ae969e 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -396,14 +396,12 @@ struct nft_rule {
 /**
  *	struct nft_trans - nf_tables object update in transaction
  *
- *	@rcu_head: rcu head to defer release of transaction data
  *	@list: used internally
  *	@msg_type: message type
  *	@ctx: transaction context
  *	@data: internal information related to the transaction
  */
 struct nft_trans {
-	struct rcu_head			rcu_head;
 	struct list_head		list;
 	int				msg_type;
 	struct nft_ctx			ctx;
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 11ab4b0..66e8425 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -3484,13 +3484,8 @@ static void nft_chain_commit_update(struct nft_trans *trans)
 	}
 }
 
-/* Schedule objects for release via rcu to make sure no packets are accesing
- * removed rules.
- */
-static void nf_tables_commit_release_rcu(struct rcu_head *rt)
+static void nf_tables_commit_release(struct nft_trans *trans)
 {
-	struct nft_trans *trans = container_of(rt, struct nft_trans, rcu_head);
-
 	switch (trans->msg_type) {
 	case NFT_MSG_DELTABLE:
 		nf_tables_table_destroy(&trans->ctx);
@@ -3612,10 +3607,11 @@ static int nf_tables_commit(struct sk_buff *skb)
 		}
 	}
 
+	synchronize_rcu();
+
 	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
 		list_del(&trans->list);
-		trans->ctx.nla = NULL;
-		call_rcu(&trans->rcu_head, nf_tables_commit_release_rcu);
+		nf_tables_commit_release(trans);
 	}
 
 	nf_tables_gen_notify(net, skb, NFT_MSG_NEWGEN);
@@ -3623,13 +3619,8 @@ static int nf_tables_commit(struct sk_buff *skb)
 	return 0;
 }
 
-/* Schedule objects for release via rcu to make sure no packets are accesing
- * aborted rules.
- */
-static void nf_tables_abort_release_rcu(struct rcu_head *rt)
+static void nf_tables_abort_release(struct nft_trans *trans)
 {
-	struct nft_trans *trans = container_of(rt, struct nft_trans, rcu_head);
-
 	switch (trans->msg_type) {
 	case NFT_MSG_NEWTABLE:
 		nf_tables_table_destroy(&trans->ctx);
@@ -3725,11 +3716,12 @@ static int nf_tables_abort(struct sk_buff *skb)
 		}
 	}
 
+	synchronize_rcu();
+
 	list_for_each_entry_safe_reverse(trans, next,
 					 &net->nft.commit_list, list) {
 		list_del(&trans->list);
-		trans->ctx.nla = NULL;
-		call_rcu(&trans->rcu_head, nf_tables_abort_release_rcu);
+		nf_tables_abort_release(trans);
 	}
 
 	return 0;
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH 1/3] arch: Introduce load_acquire() and store_release()
From: Alexander Duyck @ 2014-11-14 16:58 UTC (permalink / raw)
  To: David Laight, linux-arch@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
  Cc: mikey@neuling.org, tony.luck@intel.com,
	mathieu.desnoyers@polymtl.ca, donald.c.skidmore@intel.com,
	peterz@infradead.org, benh@kernel.crashing.org,
	heiko.carstens@de.ibm.com, oleg@redhat.com, will.deacon@arm.com,
	davem@davemloft.net, michael@ellerman.id.au,
	matthew.vick@intel.com, nic_swsd@realtek.com,
	geert@linux-m68k.org, jeffrey.t.kirsher@intel.com,
	fweisbec@gmail.com, schwidefsky@de.ibm.com,
	"linux@arm.linux.org.uk" <linux@
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1C9F0780@AcuExch.aculab.com>


On 11/14/2014 02:45 AM, David Laight wrote:
> From: Alexander Duyck
>> It is common for device drivers to make use of acquire/release semantics
>> when dealing with descriptors stored in device memory.  On reviewing the
>> documentation and code for smp_load_acquire() and smp_store_release() as
>> well as reviewing an IBM website that goes over the use of PowerPC barriers
>> at http://www.ibm.com/developerworks/systems/articles/powerpc.html it
>> occurred to me that the same code could likely be applied to device drivers.
>>
>> As a result this patch introduces load_acquire() and store_release().  The
>> load_acquire() function can be used in the place of situations where a test
>> for ownership must be followed by a memory barrier.  The below example is
>> from ixgbe:
>>
>> 	if (!rx_desc->wb.upper.status_error)
>> 		break;
>>
>> 	/* This memory barrier is needed to keep us from reading
>> 	 * any other fields out of the rx_desc until we know the
>> 	 * descriptor has been written back
>> 	 */
>> 	rmb();
>>
>> With load_acquire() this can be changed to:
>>
>> 	if (!load_acquire(&rx_desc->wb.upper.status_error))
>> 		break;
> If I'm quickly reading the 'new' code I need to look up yet another
> function, with the 'old' code I can easily see the logic.
>
> You've also added a memory barrier to the 'break' path - which isn't needed.
>
> The driver might also have additional code that can be added before the barrier
> so reducing the cost of the barrier.
>
> The driver may also be able to perform multiple actions before a barrier is needed.
>
> Hiding barriers isn't necessarily a good idea anyway.
> If you are writing a driver you need to understand when and where they are needed.
>
> Maybe you need a new (weaker) barrier to replace rmb() on some architectures.
>
> ...
>
>
> 	David

Yeah, I think I might explore creating some lightweight barriers. The 
load/acquire stuff is a bit overkill for what is needed.

Thanks,

Alex

^ permalink raw reply

* [PATCH 0/8] Netfilter/IPVS fixes for net
From: Pablo Neira Ayuso @ 2014-11-14 16:58 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

Hi David,

The following patchset contains Netfilter updates for your net tree,
they are:

1) Fix missing initialization of the range structure (allocated in the
   stack) in nft_masq_{ipv4, ipv6}_eval, from Daniel Borkmann.

2) Make sure the data we receive from userspace contains the req_version
   structure, otherwise return an error incomplete on truncated input.
   From Dan Carpenter.

3) Fix handling og skb->sk which may cause incorrect handling
   of connections from a local process. Via Simon Horman, patch from
   Calvin Owens.

4) Fix wrong netns in nft_compat when setting target and match params
   structure.

5) Relax chain type validation in nft_compat that was recently included,
   this broke the matches that need to be run from the route chain type.
   Now iptables-test.py automated regression tests report success again
   and we avoid the only possible problematic case, which is the use of
   nat targets out of nat chain type.

6) Use match->table to validate the tablename, instead of the match->name.
   Again patch for nft_compat.

7) Restore the synchronous release of objects from the commit and abort
   path in nf_tables. This is causing two major problems: splats when using
   nft_compat, given that matches and targets may sleep and call_rcu is
   invoked from softirq context. Moreover Patrick reported possible event
   notification reordering when rules refer to anonymous sets.

8) Fix race condition in between packets that are being confirmed by
   conntrack and the ctnetlink flush operation. This happens since the
   removal of the central spinlock. Thanks to Jesper D. Brouer to looking
   into this.

You can pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git

Thanks!

----------------------------------------------------------------

The following changes since commit d52fdbb735c36a209f36a628d40ca9185b349ba7:

  smc91x: retrieve IRQ and trigger flags in a modern way (2014-11-01 17:04:20 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master

for you to fetch changes up to 5195c14c8b27cc0b18220ddbf0e5ad3328a04187:

  netfilter: conntrack: fix race in __nf_conntrack_confirm against get_next_corpse (2014-11-14 17:43:05 +0100)

----------------------------------------------------------------
Calvin Owens (1):
      ipvs: Keep skb->sk when allocating headroom on tunnel xmit

Dan Carpenter (1):
      netfilter: ipset: small potential read beyond the end of buffer

Daniel Borkmann (1):
      netfilter: nft_masq: fix uninitialized range in nft_masq_{ipv4, ipv6}_eval

Pablo Neira Ayuso (4):
      netfilter: nft_compat: use current net namespace
      netfilter: nft_compat: relax chain type validation
      netfilter: nft_compat: use the match->table to validate dependencies
      netfilter: nf_tables: restore synchronous object release from commit/abort

bill bonaparte (1):
      netfilter: conntrack: fix race in __nf_conntrack_confirm against get_next_corpse

 include/net/netfilter/nf_tables.h  |    2 --
 net/ipv4/netfilter/nft_masq_ipv4.c |    1 +
 net/ipv6/netfilter/nft_masq_ipv6.c |    1 +
 net/netfilter/ipset/ip_set_core.c  |    6 ++++++
 net/netfilter/ipvs/ip_vs_xmit.c    |    2 ++
 net/netfilter/nf_conntrack_core.c  |   14 +++++++------
 net/netfilter/nf_tables_api.c      |   24 ++++++++--------------
 net/netfilter/nft_compat.c         |   40 ++++++------------------------------
 8 files changed, 32 insertions(+), 58 deletions(-)

^ permalink raw reply

* [PATCH 2/8] netfilter: ipset: small potential read beyond the end of buffer
From: Pablo Neira Ayuso @ 2014-11-14 16:58 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1415984329-5569-1-git-send-email-pablo@netfilter.org>

From: Dan Carpenter <dan.carpenter@oracle.com>

We could be reading 8 bytes into a 4 byte buffer here.  It seems
harmless but adding a check is the right thing to do and it silences a
static checker warning.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/ipset/ip_set_core.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index 86f9d76..d259da3 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -1863,6 +1863,12 @@ ip_set_sockfn_get(struct sock *sk, int optval, void __user *user, int *len)
 	if (*op < IP_SET_OP_VERSION) {
 		/* Check the version at the beginning of operations */
 		struct ip_set_req_version *req_version = data;
+
+		if (*len < sizeof(struct ip_set_req_version)) {
+			ret = -EINVAL;
+			goto done;
+		}
+
 		if (req_version->version != IPSET_PROTOCOL) {
 			ret = -EPROTO;
 			goto done;
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 4/8] netfilter: nft_compat: use current net namespace
From: Pablo Neira Ayuso @ 2014-11-14 16:58 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1415984329-5569-1-git-send-email-pablo@netfilter.org>

Instead of init_net when using xtables over nftables compat.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_compat.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
index 9d6d6f6..b92f129 100644
--- a/net/netfilter/nft_compat.c
+++ b/net/netfilter/nft_compat.c
@@ -117,7 +117,7 @@ nft_target_set_tgchk_param(struct xt_tgchk_param *par,
 			   struct xt_target *target, void *info,
 			   union nft_entry *entry, u8 proto, bool inv)
 {
-	par->net	= &init_net;
+	par->net	= ctx->net;
 	par->table	= ctx->table->name;
 	switch (ctx->afi->family) {
 	case AF_INET:
@@ -324,7 +324,7 @@ nft_match_set_mtchk_param(struct xt_mtchk_param *par, const struct nft_ctx *ctx,
 			  struct xt_match *match, void *info,
 			  union nft_entry *entry, u8 proto, bool inv)
 {
-	par->net	= &init_net;
+	par->net	= ctx->net;
 	par->table	= ctx->table->name;
 	switch (ctx->afi->family) {
 	case AF_INET:
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 5/8] netfilter: nft_compat: relax chain type validation
From: Pablo Neira Ayuso @ 2014-11-14 16:58 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1415984329-5569-1-git-send-email-pablo@netfilter.org>

Check for nat chain dependency only, which is the one that can
actually crash the kernel. Don't care if mangle, filter and security
specific match and targets are used out of their scope, they are
harmless.

This restores iptables-compat with mangle specific match/target when
used out of the OUTPUT chain, that are actually emulated through filter
chains, which broke when performing strict validation.

Fixes: f3f5dde ("netfilter: nft_compat: validate chain type in match/target")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_compat.c |   32 ++------------------------------
 1 file changed, 2 insertions(+), 30 deletions(-)

diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
index b92f129..70dc965 100644
--- a/net/netfilter/nft_compat.c
+++ b/net/netfilter/nft_compat.c
@@ -21,45 +21,17 @@
 #include <linux/netfilter_ipv6/ip6_tables.h>
 #include <net/netfilter/nf_tables.h>
 
-static const struct {
-       const char	*name;
-       u8		type;
-} table_to_chaintype[] = {
-       { "filter",     NFT_CHAIN_T_DEFAULT },
-       { "raw",        NFT_CHAIN_T_DEFAULT },
-       { "security",   NFT_CHAIN_T_DEFAULT },
-       { "mangle",     NFT_CHAIN_T_ROUTE },
-       { "nat",        NFT_CHAIN_T_NAT },
-       { },
-};
-
-static int nft_compat_table_to_chaintype(const char *table)
-{
-	int i;
-
-	for (i = 0; table_to_chaintype[i].name != NULL; i++) {
-		if (strcmp(table_to_chaintype[i].name, table) == 0)
-			return table_to_chaintype[i].type;
-	}
-
-	return -1;
-}
-
 static int nft_compat_chain_validate_dependency(const char *tablename,
 						const struct nft_chain *chain)
 {
-	enum nft_chain_type type;
 	const struct nft_base_chain *basechain;
 
 	if (!tablename || !(chain->flags & NFT_BASE_CHAIN))
 		return 0;
 
-	type = nft_compat_table_to_chaintype(tablename);
-	if (type < 0)
-		return -EINVAL;
-
 	basechain = nft_base_chain(chain);
-	if (basechain->type->type != type)
+	if (strcmp(tablename, "nat") == 0 &&
+	    basechain->type->type != NFT_CHAIN_T_NAT)
 		return -EINVAL;
 
 	return 0;
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 6/8] netfilter: nft_compat: use the match->table to validate dependencies
From: Pablo Neira Ayuso @ 2014-11-14 16:58 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1415984329-5569-1-git-send-email-pablo@netfilter.org>

Instead of the match->name, which is of course not relevant.

Fixes: f3f5dde ("netfilter: nft_compat: validate chain type in match/target")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_compat.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
index 70dc965..265e190 100644
--- a/net/netfilter/nft_compat.c
+++ b/net/netfilter/nft_compat.c
@@ -346,7 +346,7 @@ nft_match_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
 	union nft_entry e = {};
 	int ret;
 
-	ret = nft_compat_chain_validate_dependency(match->name, ctx->chain);
+	ret = nft_compat_chain_validate_dependency(match->table, ctx->chain);
 	if (ret < 0)
 		goto err;
 
@@ -420,7 +420,7 @@ static int nft_match_validate(const struct nft_ctx *ctx,
 		if (!(hook_mask & match->hooks))
 			return -EINVAL;
 
-		ret = nft_compat_chain_validate_dependency(match->name,
+		ret = nft_compat_chain_validate_dependency(match->table,
 							   ctx->chain);
 		if (ret < 0)
 			return ret;
-- 
1.7.10.4


^ 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