netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [iproute PATCH v2 2/4] tc: pedit: Fix for big-endian systems
From: Phil Sutter @ 2016-03-22 14:16 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jamal Hadi Salim, netdev
In-Reply-To: <1458656184-22154-1-git-send-email-phil@nwl.cc>

This was tricky to get right:
- The 'stride' value used for 8 and 16 bit values must behave inverse to
  the value's intra word offset to work correctly with big-endian data
  act_pedit is editing.
- The 'm' array's values are in host byte order, so they have to be
  converted as well (and the ordering was just inverse, for some
  reason).
- The only sane way of getting this right is to manipulate value/mask in
  host byte order and convert the output.
- TIPV4 (i.e. 'munge ip src/dst') had it's own pitfall: the address
  parser converts to network byte order automatically. This patch fixes
  this by converting it back before calling pack_key32, which is a hack
  but at least does not require to implement a completely separate code
  flow.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tc/m_pedit.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/tc/m_pedit.c b/tc/m_pedit.c
index ca78a83dd9d9d..30a6f3673e896 100644
--- a/tc/m_pedit.c
+++ b/tc/m_pedit.c
@@ -156,7 +156,7 @@ int
 pack_key16(__u32 retain, struct tc_pedit_sel *sel, struct tc_pedit_key *tkey)
 {
 	int ind, stride;
-	__u32 m[4] = {0xFFFF0000, 0xFF0000FF, 0x0000FFFF};
+	__u32 m[4] = {0x0000FFFF, 0xFF0000FF, 0xFFFF0000};
 
 	if (tkey->val > 0xFFFF || tkey->mask > 0xFFFF) {
 		fprintf(stderr, "pack_key16 bad value\n");
@@ -170,9 +170,9 @@ pack_key16(__u32 retain, struct tc_pedit_sel *sel, struct tc_pedit_key *tkey)
 		return -1;
 	}
 
-	stride = 8 * ind;
-	tkey->val = htons(tkey->val & retain) << stride;
-	tkey->mask = (htons(tkey->mask | ~retain) << stride) | m[ind];
+	stride = 8 * (2 - ind);
+	tkey->val = htonl((tkey->val & retain) << stride);
+	tkey->mask = htonl(((tkey->mask | ~retain) << stride) | m[ind]);
 
 	tkey->off &= ~3;
 
@@ -186,7 +186,7 @@ int
 pack_key8(__u32 retain, struct tc_pedit_sel *sel, struct tc_pedit_key *tkey)
 {
 	int ind, stride;
-	__u32 m[4] = {0xFFFFFF00, 0xFFFF00FF, 0xFF00FFFF, 0x00FFFFFF};
+	__u32 m[4] = {0x00FFFFFF, 0xFF00FFFF, 0xFFFF00FF, 0xFFFFFF00};
 
 	if (tkey->val > 0xFF || tkey->mask > 0xFF) {
 		fprintf(stderr, "pack_key8 bad value (val %x mask %x\n", tkey->val, tkey->mask);
@@ -195,9 +195,9 @@ pack_key8(__u32 retain, struct tc_pedit_sel *sel, struct tc_pedit_key *tkey)
 
 	ind = tkey->off & 3;
 
-	stride = 8 * ind;
-	tkey->val = (tkey->val & retain) << stride;
-	tkey->mask = ((tkey->mask | ~retain) << stride) | m[ind];
+	stride = 8 * (3 - ind);
+	tkey->val = htonl((tkey->val & retain) << stride);
+	tkey->mask = htonl(((tkey->mask | ~retain) << stride) | m[ind]);
 
 	tkey->off &= ~3;
 
@@ -283,6 +283,9 @@ parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain, struct
 	tkey->val = val;
 	tkey->mask = mask;
 
+	if (type == TIPV4)
+		tkey->val = ntohl(tkey->val);
+
 	if (len == 1) {
 		res = pack_key8(retain, sel, tkey);
 		goto done;
-- 
2.7.2

^ permalink raw reply related

* [iproute PATCH v2 0/4] tc: pedit: further fixes
From: Phil Sutter @ 2016-03-22 14:16 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jamal Hadi Salim, netdev

The following series was created after testing pedit on a big-endian
system. It starts with a minor patch fixing coding style in tc/p_ip.c,
then the actual big-endian fixup, a fix for raw op discovered when
writing the pedit test for testsuite and finally a testsuite addon to
verify pedit functionality.

Changes since v1:
- Rebased whole series onto current master branch.
- Replaced patch 1/4 with a smaller one containing only the rebase
  leftovers.

Phil Sutter (4):
  tc/p_ip.c: Minor coding style cleanup
  tc: pedit: Fix for big-endian systems
  tc: pedit: Fix raw op
  testsuite: add a test for tc pedit action

 tc/m_pedit.c               |  23 ++---
 tc/p_ip.c                  |   8 +-
 testsuite/tests/tc/pedit.t | 217 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 235 insertions(+), 13 deletions(-)
 create mode 100755 testsuite/tests/tc/pedit.t

-- 
2.7.2

^ permalink raw reply

* [iproute PATCH v2 3/4] tc: pedit: Fix raw op
From: Phil Sutter @ 2016-03-22 14:16 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jamal Hadi Salim, netdev
In-Reply-To: <1458656184-22154-1-git-send-email-phil@nwl.cc>

The retain value was wrong for u16 and u8 types.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tc/m_pedit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tc/m_pedit.c b/tc/m_pedit.c
index 30a6f3673e896..8ccec8bc4a99f 100644
--- a/tc/m_pedit.c
+++ b/tc/m_pedit.c
@@ -339,12 +339,12 @@ parse_offset(int *argc_p, char ***argv_p, struct tc_pedit_sel *sel, struct tc_pe
 	}
 	if (matches(*argv, "u16") == 0) {
 		len = 2;
-		retain = 0x0;
+		retain = 0xffff;
 		goto done;
 	}
 	if (matches(*argv, "u8") == 0) {
 		len = 1;
-		retain = 0x0;
+		retain = 0xff;
 		goto done;
 	}
 
-- 
2.7.2

^ permalink raw reply related

* Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
From: Alan Stern @ 2016-03-22 14:21 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: David S. Miller, Geert Uytterhoeven,
	Microchip Linux Driver Support, Woojung Huh, Rafael J. Wysocki,
	Guenter Roeck, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1458640209.1990.8.camel-IBi9RG/b67k@public.gmane.org>

On Tue, 22 Mar 2016, Oliver Neukum wrote:

> On Mon, 2016-03-21 at 15:30 -0400, Alan Stern wrote:
> > On Mon, 21 Mar 2016, Oliver Neukum wrote:
> > 
> 
> > > We have an autosuspend timeout because we think that IO, if it will
> > > come at all, is likeliest to come soon. If, however, the IO is
> > > periodic that heuristics is false.
> > > To save most power the driver must either decide that the interval
> > > is too short or suspend immediately. So if we are lucky enough
> > > to have the frequency in the kernel, we should use that information.
> > 
> > The autosuspend timeout is set by userspace.  The kernel may assign a
> 
> Thus it should apply to all IO originating in user space.
> But only to that IO.

Fair enough.

> > default value, but the user can always override it.  Given this, I 
> > don't see how the kernel can use frequency information (and I'm not 
> > sure where that information would come from in the first place).
> 
> It can ignore internal IO for the purpose of the timeout.
> If such IO is performed while the device is active, don't
> alter the timer.

Come to think of it, we don't.  If pm_runtime_get_sync() and then
pm_runtime_put() are called while the device is already at full power, 
the PM core doesn't update the last_busy time.  So if the driver 
doesn't update it either, the statistics collection won't interfere 
with autosuspend (except when it races with the autosuspend timer 
expiration).

> Otherwise resume the device and look at
> the provided hint and suspend again immediately if the period is long
> enough.

I don't see any point in resuming the device just in order to collect 
operating statistics.  If it was already suspended then it wasn't 
operating, so there will be no statistics to collect.

> If IO is generated periodically in the kernel, the kernel must know that
> period.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] fsl/fman: Workaround for Errata A-007273
From: igal.liberman @ 2016-03-21 21:08 UTC (permalink / raw)
  To: netdev; +Cc: scottwood, madalin.bucur, Igal Liberman

From: Igal Liberman <igal.liberman@freescale.com>

Errata A-007273 (For FMan V3 devices only):
FMan soft reset is not finished properly if one
of the Ethernet MAC clocks is disabled

Workaround:
Re-enable all disabled MAC clocks through the DCFG_CCSR_DEVDISR2
register prior to issuing an FMAN soft reset.
Re-disable the MAC clocks after the FMAN soft reset is done.

Signed-off-by: Igal Liberman <igal.liberman@freescale.com>
---
 drivers/net/ethernet/freescale/fman/fman.c |  104 +++++++++++++++++++++++-----
 1 file changed, 88 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman.c b/drivers/net/ethernet/freescale/fman/fman.c
index 79a210a..ea83712 100644
--- a/drivers/net/ethernet/freescale/fman/fman.c
+++ b/drivers/net/ethernet/freescale/fman/fman.c
@@ -35,6 +35,7 @@
 #include "fman.h"
 #include "fman_muram.h"
 
+#include <linux/fsl/guts.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/module.h>
@@ -1871,6 +1872,90 @@ err_fm_state:
 	return -EINVAL;
 }
 
+static int fman_reset(struct fman *fman)
+{
+	u32 count;
+	int err = 0;
+
+	if (fman->state->rev_info.major < 6) {
+		iowrite32be(FPM_RSTC_FM_RESET, &fman->fpm_regs->fm_rstc);
+		/* Wait for reset completion */
+		count = 100;
+		do {
+			udelay(1);
+		} while (((ioread32be(&fman->fpm_regs->fm_rstc)) &
+			 FPM_RSTC_FM_RESET) && --count);
+		if (count == 0)
+			err = -EBUSY;
+
+		goto _return;
+	} else {
+		struct device_node *guts_node;
+		struct ccsr_guts __iomem *guts_regs;
+		u32 devdisr2, reg;
+
+		/* Errata A007273 */
+		guts_node =
+			of_find_compatible_node(NULL, NULL,
+						"fsl,qoriq-device-config-2.0");
+		if (!guts_node) {
+			dev_err(fman->dev, "%s: Couldn't find guts node\n",
+				__func__);
+			goto guts_node;
+		}
+
+		guts_regs = of_iomap(guts_node, 0);
+		if (!guts_regs) {
+			dev_err(fman->dev, "%s: Couldn't map %s regs\n",
+				__func__, guts_node->full_name);
+			goto guts_regs;
+		}
+#define FMAN1_ALL_MACS_MASK	0xFCC00000
+#define FMAN2_ALL_MACS_MASK	0x000FCC00
+		/* Read current state */
+		devdisr2 = ioread32be(&guts_regs->devdisr2);
+		if (fman->dts_params.id == 0)
+			reg = devdisr2 & ~FMAN1_ALL_MACS_MASK;
+		else
+			reg = devdisr2 & ~FMAN2_ALL_MACS_MASK;
+
+		/* Enable all MACs */
+		iowrite32be(reg, &guts_regs->devdisr2);
+
+		/* Perform FMan reset */
+		iowrite32be(FPM_RSTC_FM_RESET, &fman->fpm_regs->fm_rstc);
+
+		/* Wait for reset completion */
+		count = 100;
+		do {
+			udelay(1);
+		} while (((ioread32be(&fman->fpm_regs->fm_rstc)) &
+			 FPM_RSTC_FM_RESET) && --count);
+		if (count == 0) {
+			iounmap(guts_regs);
+			of_node_put(guts_node);
+			err = -EBUSY;
+			goto _return;
+		}
+
+		/* Restore devdisr2 value */
+		iowrite32be(devdisr2, &guts_regs->devdisr2);
+
+		iounmap(guts_regs);
+		of_node_put(guts_node);
+
+		goto _return;
+
+guts_regs:
+		of_node_put(guts_node);
+guts_node:
+		dev_dbg(fman->dev, "%s: Didn't perform FManV3 reset due to Errata A007273!\n",
+			__func__);
+	}
+_return:
+	return err;
+}
+
 static int fman_init(struct fman *fman)
 {
 	struct fman_cfg *cfg = NULL;
@@ -1914,22 +1999,9 @@ static int fman_init(struct fman *fman)
 		fman->liodn_base[i] = liodn_base;
 	}
 
-	/* FMan Reset (supported only for FMan V2) */
-	if (fman->state->rev_info.major >= 6) {
-		/* Errata A007273 */
-		dev_dbg(fman->dev, "%s: FManV3 reset is not supported!\n",
-			__func__);
-	} else {
-		iowrite32be(FPM_RSTC_FM_RESET, &fman->fpm_regs->fm_rstc);
-		/* Wait for reset completion */
-		count = 100;
-		do {
-			udelay(1);
-		} while (((ioread32be(&fman->fpm_regs->fm_rstc)) &
-			 FPM_RSTC_FM_RESET) && --count);
-		if (count == 0)
-			return -EBUSY;
-	}
+	err = fman_reset(fman);
+	if (err)
+		return err;
 
 	if (ioread32be(&fman->qmi_regs->fmqm_gs) & QMI_GS_HALT_NOT_BUSY) {
 		resume(fman->fpm_regs);
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
From: Oliver Neukum @ 2016-03-22 14:28 UTC (permalink / raw)
  To: Alan Stern
  Cc: David S. Miller, Geert Uytterhoeven,
	Microchip Linux Driver Support, Woojung Huh, Rafael J. Wysocki,
	Guenter Roeck, linux-kernel, linux-pm, linux-usb, netdev
In-Reply-To: <Pine.LNX.4.44L0.1603221014500.1793-100000@iolanthe.rowland.org>

On Tue, 2016-03-22 at 10:21 -0400, Alan Stern wrote:
> I don't see any point in resuming the device just in order to collect 
> operating statistics.  If it was already suspended then it wasn't 
> operating, so there will be no statistics to collect.

Indeed. In that case the point is moot. But it is correct to ask
the core whether the device is autosuspended at that point rather
than keep a private flag if you can.

All that is relevant only if the upper layers ask for information
that the driver cannot provide without resuming the device.
Those are fundamentally different issues.

	Regards
		Oliver



^ permalink raw reply

* Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB
From: Sebastian Frias @ 2016-03-22 14:34 UTC (permalink / raw)
  To: Uwe Kleine-König, Daniel Mack
  Cc: David S. Miller, netdev, lkml, mason, Florian Fainelli,
	Mans Rullgard, Fabio Estevam, Martin Blumenstingl, Linus Walleij
In-Reply-To: <20160321201229.GA6191@pengutronix.de>

Hi Uwe,

I think we are in a deadlock :-)
I'm going to reply inline below, but I will also send a different email
to Daniel with a small recap.
I think he should share the intent of the "reset" mechanism he
introduced, in particular if it is mandatory.


On 03/21/2016 09:12 PM, Uwe Kleine-König wrote:
>>
>> gpiod=NULL (because the key is not there) or gpiod=ERR (because
>> GPIOLIB=n + my patch) will result in the same behaviour, ie: driver
>> binds, but fails to reset.
> 
> Assuming the source for the hardware description is dt (the same
> argument applies if it's ACPI or something else).
> 
> The driver uses devm_gpiod_get_optional(..."reset"...). That means some
> hardware has a reset line, some don't. If a reset gpio specification is
> there, that means the hardware has such a signal and it seems that means
> it must not be ignored. 

The problem is all those "it seems" :-)
"it seems it must not be ignored" is also an hypothesis considering the
driver is not refusing to work even if it detects that the faulty PHY
has not been provided with a reset line.
Also, further down the thread you acknowledge that's a possibility.

>If there is no reset gpio specification that
> means that driver has to assume there is no reset line. (In the real
> world there might be other reasons the reset line isn't in the device
> tree, but it's not in the scope of the driver to guess why it's missing.
> If it's not there the only sensible thing to assume for the driver is:
> There is no reset line.)

But that is not what the current code does, the current code does not
checks if the DT presents the "reset" property directly.
It assumes that GPIOLIB works and checks GPIOLIB return codes, to
indirectly (and incorrectly) guess if a given device has a "reset" property.
It has not been clearly explained anywhere that the "reset" property for
AT8030 is supposed to be mandatory and that the driver should fail if
not provided with one.
The code does not back up such hypothesis either, ie: GPIOLIB=y +
missing "reset" will still allow the driver to work.

> 
> So the conclusions are: If there is a reset line in dt, it must be used.
> If you don't know if there is a reset line (because GPIOLIB=n) the
> driver should not bind. Everything else yields to more problems than
> good.

a) "If there is a reset line in dt, it must be used": that is an
hypothesis (I would say "it can be used");
b) if it were true, then another way of knowing if the "reset" key is
present should be used;
indeed, there should be:
c) a way to unambiguously determine if the "reset" key is in the DT
(regardless of GPIOLIB status);
that in order to know if the DT is specifying that the "reset" mechanism
must be used, then:
d) iff the "reset" is absolutely necessary (ie: the faulty PHY is in use
AND the original intention of the hack was for it *mandatory* for faulty
PHYs), some decision can be taken;

However, right now those conditions are not met.

>>> With your patch and GPIOLIB=n you bind the driver even for the devices
>>> that need the hack. This is broken!
>>
>> It is a degraded mode I agree and had proposed to print a warning.
>> However, I fail to see how is GPIOLIB=n different from just not having
>> "reset" present.
> 
> GPIOLIB=n means "the driver doesn't know if a reset line is
> present/necessary", not having reset means "there is no reset line".

That's the problem, that the driver is relying on GPIOLIB to know if:
- "reset" key is present
- it should fail to bind or not

and it does that based solely on GPIOLIB return codes, thus indirectly.
As opposed to checking directly the presence of the "reset" key
(regardless of GPIOLIB status) and failing immediately.

> 
> And don't do error handling by printk assuming anyone will read it. That
> doesn't work.

I proposed a warning, not error handling.
Also if we assume people won't read the logs, why put any at all?
Some people may not read the logs, some others will, it is their choice
to bang their heads one way or another.
Currently there's no warning.
So I had to debug why the driver was not binding, and it is not obvious
that GPIOLIB must be selected, considering:
- I don't have a faulty PHY
- I don't have a "reset" key on DT
- Even if I wanted, I don't have a GPIO to connect to it

(also, even if this driver does not bind, some generic one would take over)

> 
>> The fact that GPIOLIB=y does not means that the "reset" key will be there.
> 
> Right, but with GPIOLIB=y you know if it's there, and if yes, you can
> control the line.

Exactly, and what I say is that knowing if "reset" is there or not
should not depend on having GPIOLIB=y

To me the problem comes from the aliasing between "GPIOLIB=y" and
"reset" present.
Indeed, the driver should query if the "reset" key is present
(regardless of GPIOLIB status), and then try to acquire the GPIO line.
If it cannot get the reset GPIO, then it can give up saying "you told me
I need a reset line but then you did not give me one".

> 
>> I mean, you assume that "reset" will be present for devices that need
>> the hack, yet there is no such guarantee and the code clearly assumes
>> that it can be missing.
> 
> The driver must assume that the device tree is right. If it's not fix
> the device tree, don't implement clumsy work arounds in the driver.

I doubt it is like that.
I mean, it may sound easy to say "just update the DT and add the 'reset'
key", but the DT is describing a physical connections in this case.
Changing that may not be as easy.

IMHO it would be far better if the driver made the check on the PHY ID
(8030) at probe time and refused to bind if:
- the "reset" is absolutely necessary per design (ie: intended by Daniel)
and
- it did not get a reset line (for whatever reason).

That was not done.
Maybe because it is not possible because the ID may not be known at that
time (strange since it is being probed);
Maybe because Daniel meant the whole thing to be optional;
Or maybe for other reasons.

> 
>> For all we know, the key is actually missing on systems that do use the
>> faulty PHY (all systems designed prior to the hack being implemented)
>> And that, regardless of GPIOLIB status.
> 
> Then fix the device tree.

Changing the device tree does not automagically creates a connection
between a given GPIO (which must be available) and the PHY reset line.
GPIOs and reset lines are physical things.

But maybe I'm missing something and board designers always route GPIOs
to each and every peripheral's reset line.

> 
>> Since the reset line can be missing, it can be missing for whatever reason.
>> Whether it is because the "reset" key is not present or because
>> GPIOLIB=n, it does not matter, it will result on the same outcome.
>>
>> Furthermore, if "reset" is really required for certain devices that need
>> the hack, then both cases:
>> - GPIOLIB=n
>> - "reset" not present
>> should be handled the same way (ie: not bind the driver) for such devices.
> 
> If you can detect by other means that "reset is necessary", then yes,
> the driver should ideally also fail in this case with no reset specified
> in dt.

Well, the problem is that right now we don't know what was the intention
of the person that implemented the "reset" mechanism.
Both of us are arguing about our own hypothesis.
You have yours "reset is necessary and driver should not bind without".
And I have mine "nothing in the code implies it is necessary".

> 
>> By the way, I think not binding the driver is too strong too.
>> Having a GPIO free and being able to route it to the PHY for reset may
>> not even be possible on some systems.
>> Are they supposed to stop working?
> 
> Sometimes no driver is better than an unreliable one.

It appears that if this driver does not binds, then a generic one (which
arguably will also lack the reset behaviour) will take over.
How can that be any better?

>>>
>>> No, if reset was mandatory you'd use gpiod_get instead of
>>> gpiod_get_optional and fail iff that call fails, too.
>>
>> Ok, but the current code for "reset" is using devm_gpiod_get_optional()
>> so "reset" is clearly optional.
>> And that call will return NULL if "reset" is not present, even with
>> GPIOLIB=y
> 
> s/even//. And returning NULL is not an error. It means: There is no
> reset line specified in dt.

Indeed, and it is totally possible that the PHY ID is a faulty one, yet
the driver will bind anyway.
You argue about a case "reset is absolutely necessary for some devices",
but the code does not enforce that.

>>
>> I think somehow you try to make a difference on the reason why the
>> reset=INVALID (NULL or ERR), but IMHO there's none.
> 
> NULL is not invalid. With devm_gpiod_get_optional the driver asks: "Is
> there a reset gpio, and if yes, which one?". The reset line is optional,
> that means "some devices have one, some others don't.". It does NOT mean
> "You can ignore if there is a reset line or not."!

But the current code does ignore it :-)
Even for faulty devices.

Again, if you take a faulty PHY and a DT without the "reset" key, the
driver will bind.
Don't you think that goes against your hypothesis of "reset must be
present and available for some devices"?

By the way, is the "reset" key documented somewhere? How would the DT
look like?
Maybe in such documentation there's some explanation about the key being
compulsory or not.

> 
>> If somebody using AT8030 PHY (the one that requires the hack) either
>> does not configure a "reset" key on the DT, either does not enable
>> GPIOLIB, the result will be the same.
>> There is no warning in either case and it will run on a degraded mode.
> 
> My expectation is that if the dt includes a reset property, it should be
> used. So, there is a fine option for each situation:
> 
>  - device that doesn't need the hack => everything is fine;
>  - AT8030 with reset to a gpio => specify the reset in dt;
>  - AT8030 without a controllable reset line => don't specify a reset
>    property;

You do realise that in this last case the device will work in degraded
mode, right?

> 
> if you have a broken device with a reset line you can even drop the
> reset property from dt and not make use of the reset gpio if you think
> it's a good idea.
> 

So you are saying that "reset" is optional, even for faulty PHYs.

>>
>> Ok, then I did not understand what you meant with "the question is
>> obsolete because the device doesn't probe".
> 
>   if (GPIOLIB=y):
>     if (device has a reset property):
>       driver runs fine with being able to reset.
>     else:
>       if (device needs reset to function properly):
>         ideally fail to probe
>       else:
>         driver runs fine without reset and without the need
> 	  to bother the user about the absense of the gpio
>   else:
>     driver fails to bind

To me it should be:

   if (device needs reset):
      if (DT has reset):
         if (GPIOLIB=y):
            gpiod=callback, driver gets a reset line and can operate
properly (ie: perform the hack)
         else
            gpiod=-ENOSYS, driver does not get reset line: should it
fail to bind? should it continue?
      else
         gpiod=NULL, driver does not get a reset line: should it fail to
bind? should it continue?
   else
      gpiod is not even requested, driver does not need a reset line and
can operate properly.

> 
> In no branch of these cases there is a situation where you must issue a
> reset but cannot. 

Well, the thing is that what you described does not match the code,
AFAIK current code does:

   if (GPIOLIB=y):
      if (DT has reset):
         gpiod=callback
         bind driver
         if (faulty PHY):
            use gpiod
      else
         gpiod=NULL
         bind driver
         if (faulty PHY)
            cannot use gpiod, but continue anyway without warning
   else
      gpiod=-ENOSYS, fail to bind, regardless of PHY being faulty, and
regardless of "reset" presence (*)


(*) the way the code is written, "reset" presence is not checked
directly, but as a side-effect of GPIOLIB

>So there is no need to ask if you should issue a
> message each time this happens. That's what I meant with "the question
> is obsolete".
> 
>> Unless I'm missing something, the only way the driver won't bind
>> correctly with current code is if GPIOLIB=n
> 
> Right, because with GPIOLIB=n you cannot control a gpio line, but you
> don't know if you must.

Well right now nobody knows if the "reset" is actually a must, even for
faulty PHYs.
The driver binds happily without, even on faulty devices.

> 
> Consider you are an electrician (driver) and you should repair a power
> socket (control a device) but forgot your phase tester (GPIOLIB=n).
> So there is no way you can determine if it's save to modify the socket.
> You can choose the optimistic way and say: "If there is no voltage on the
> socket, I can repair it successfully" and just try it. Or you take the
> safe approach and say: "I don't know if it's safe to do, so I better
> don't.".

:-) thanks for the analogy.

> 
> The same applies to drivers: If it might be necessary to handle a gpio,
> but you cannot know if that's the case or not: Don't try it.
> 

But that's not how the code was written.
It is not mentioned anywhere that that was the initial intention either.

>>
>> To put an example: in our case we don't use the faulty PHY.
>> So, the DT does not has the "reset" key.
>> Why should then the PHY driver be dependent on GPIOLIB?
> 
> You want GPIOLIB to know that you don't need to handle the reset gpio.

But that's like saying "you want all possible drivers linked-in, if they
are not in DT they won't load/probe, so it's fine"

We don't want GPIOLIB because it is a bunch of useless code in our case.
We know in advance that the DT will not have the "reset" key.
We know in advance that the PHY is not faulty.
Why should we link-in a bunch of useless code then?

Alternatively, if GPIOLIB is mandatory due to the way drivers are
written or the way DT works, then why make it optional?

Best regards,

Sebastian

^ permalink raw reply

* Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB
From: Sebastian Frias @ 2016-03-22 14:34 UTC (permalink / raw)
  To: Daniel Mack
  Cc: Uwe Kleine-König, David S. Miller, netdev, lkml, mason,
	Florian Fainelli, Mans Rullgard, Fabio Estevam,
	Martin Blumenstingl, Linus Walleij
In-Reply-To: <56EC2525.8000706@laposte.net>

Hi Daniel,

Would you mind commenting on this thread?
Uwe and I are in a sort of deadlock because we each have a different
understanding of the intention of your commit 13a56b449325.

Basically the questions are:
- What is the intention of 13a56b449325?
- Did you mean for "reset" to be mandatory for faulty PHYs?
Mandatory meaning that you do not want the driver to work without.
- What do you think about the dependency on GPIOLIB?
You did not introduced such dependency with your change so it may point
to "reset" not being mandatory.

Best regards,

Sebastian

On 03/18/2016 04:56 PM, Sebastian Frias wrote:
> Hi Uwe, Daniel,
> 
> On 03/18/2016 01:54 PM, Uwe Kleine-König wrote:
>> [expand cc a bit more]
>>
>> Hello,
>>
>> On Wed, Mar 16, 2016 at 06:25:59PM +0100, Sebastian Frias wrote:
>>> Commit 687908c2b649 ("net: phy: at803x: simplify using
>>> devm_gpiod_get_optional and its 4th argument") introduced a dependency
>>> on GPIOLIB that was not there before.
>>>
>>> This commit removes such dependency by checking the return code and
>>> comparing it against ENOSYS which is returned when GPIOLIB is not
>>> selected.
>>>
>>> Fixes: 687908c2b649 ("net: phy: at803x: simplify using devm_gpiod_get_optional and its 4th argument")
>>>
>>> Signed-off-by: Sebastian Frias <sf84@laposte.net>
>>> ---
>>>  drivers/net/phy/at803x.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
>>> index 2174ec9..88b7ff3 100644
>>> --- a/drivers/net/phy/at803x.c
>>> +++ b/drivers/net/phy/at803x.c
>>> @@ -252,7 +252,9 @@ static int at803x_probe(struct phy_device *phydev)
>>>  		return -ENOMEM;
>>>
>>>  	gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>>> -	if (IS_ERR(gpiod_reset))
>>> +	if (PTR_ERR(gpiod_reset) == -ENOSYS)
>>> +		gpiod_reset = NULL;
>>> +	else if (IS_ERR(gpiod_reset))
>>
>> this isn't better either (IMHO it's worse, but maybe this is debatable
>> and also depends on your POV).
> 
> Well, from the code, the reset hack is only required when the PHY is
> ATH8030_PHY_ID, right?
> However, such change was introduced by Daniel Mack on commit 13a56b449325.
> Hopefully he can chime in and give his opinion on this.
> 
> Daniel, while on the subject, I have a question for you:
> 
> Change 2b8f2a28eac1 introduced "link_status_notify" callback which is
> called systematically on the PHY state machine.
> Any reason to make the call systematic as opposed to let say:
> 
> 	if (phydev->state != old_state) {
> 		if (phydev->drv->link_change_notify)
> 			phydev->drv->link_change_notify(phydev);
> 	}
> 
> (it does means that the callback would be called after the state machine
> processing though)
> 
>>
>> With 687908c2b649 I made kernels without GPIOLIB fail to bind this
>> device. I admit this is not maximally nice.
> 
> I see, that was not clear from the commit message, sorry.
> 
>>
>> Your change makes the driver bind in this case again and then the reset
>> gpio isn't handled at all which might result in a later and harder to
>> debug error.
>>
>> The better approach to fix your problem is: make the driver depend (or
>> force) on GPIOLIB. 
> 
> It was one of the solutions I had in mind, but:
> - since the dependency on GPIOLIB was not included on the patch
> - and given that the previous code had provision to work without GPIO
> I thought it was an overlook.
> 
>> Or alternatively, drop reset-handling from the
>> driver.
>>
>> From a driver perspecitive, it would be nice if devm_gpiod_get_optional
>> returned NULL iff the respective gpio isn't specified even with
>> GPIOLIB=n, but this isn't sensible either because it would result in
>> quite some gpiolib code to not being conditionally compiled on
>> CONFIG_GPIOLIB any more.
> 
> Let's say that was the case, what would the PHY code do?
> 
> I mean, it did not get a GPIO, whether it was because GPIOLIB=n or
> because there's no 'reset' GPIO attached
> Shall it fail? Shall it continue in a sort of degraded mode? Shall it warn?
> Because that's the real question here.
> 
> What would you think of making at803x_link_change_notify() print a
> message every time it should do a reset but does not has a way to do it?
> I can make such change to my patch if everybody agrees on it.
> By the way, in that case, can somebody suggest a way to print such warning?
> Would printk() be ok or should I use dev_dbg() ?
> 
> Best regards,
> 
> Sebastian
> 
>>
>> Best regards
>> Uwe
>>

^ permalink raw reply

* Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB
From: Sebastian Frias @ 2016-03-22 14:39 UTC (permalink / raw)
  To: Sergei Shtylyov, Uwe Kleine-König, David S. Miller, netdev
  Cc: lkml, mason
In-Reply-To: <56F05651.3030706@cogentembedded.com>

Hi Sergei,

On 03/21/2016 09:15 PM, Sergei Shtylyov wrote:
> 
>    Do you have the PHY that requires the GPIO reset workaround?

Unfortunately (or luckily :-) ) I don't have the faulty PHY, sorry.

Best regards,

Sebastian

^ permalink raw reply

* [iproute PATCH v2 5/8] man: tc-police.8: Emphasize on the two rate control mechanisms
From: Phil Sutter @ 2016-03-22 14:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jamal Hadi Salim, netdev
In-Reply-To: <1458658119-25881-1-git-send-email-phil@nwl.cc>

As Jamal pointed out, there are two different approaches to bandwidth
measurement. Try to make this clear by separating them in synopsis and
also documenting the way to fine-tune avrate.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 man/man8/tc-police.8 | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/man/man8/tc-police.8 b/man/man8/tc-police.8
index 2b1537ec52875..5c5a632335dc9 100644
--- a/man/man8/tc-police.8
+++ b/man/man8/tc-police.8
@@ -12,13 +12,21 @@ police - policing action
 .IR BYTES [\fB/ BYTES "] ] ["
 .BI peakrate " RATE"
 ] [
-.BI avrate " RATE"
-] [
 .BI overhead " BYTES"
 ] [
 .BI linklayer " TYPE"
 ] [
-.BI conform-exceed " EXCEEDACT\fR[\fB/\fIEXCEEDACT\fR]"
+.IR CONTROL " ]"
+
+.ti -8
+.BR tc " ... " filter " ... [ " estimator
+.IR "SAMPLE AVERAGE " ]
+.BR "action police avrate"
+.IR RATE " [ " CONTROL " ]"
+
+.ti -8
+.IR CONTROL " :="
+.BI conform-exceed " EXCEEDACT\fR[\fB/\fIEXCEEDACT"
 
 .ti -8
 .IR EXCEEDACT " := { "
@@ -27,7 +35,14 @@ police - policing action
 The
 .B police
 action allows to limit bandwidth of traffic matched by the filter it is
-attached to.
+attached to. Basically there are two different algorithms available to measure
+the packet rate: The first one uses an internal dual token bucket and is
+configured using the
+.BR rate ", " burst ", " mtu ", " peakrate ", " overhead " and " linklayer
+parameters. The second one uses an in-kernel sampling mechanism. It can be
+fine-tuned using the
+.B estimator
+filter parameter.
 .SH OPTIONS
 .TP
 .BI rate " RATE"
@@ -73,6 +88,12 @@ cell sizes, for
 .B ethernet
 no action is taken.
 .TP
+.BI estimator " SAMPLE AVERAGE"
+Fine-tune the in-kernel packet rate estimator.
+.IR SAMPLE " and " AVERAGE
+are time values and control the frequency in which samples are taken and over
+what timespan an average is built.
+.TP
 .BI conform-exceed " EXCEEDACT\fR[\fB/\fIEXCEEDACT\fR]"
 Define how to handle packets which exceed (and, if the second
 .I EXCEEDACT
-- 
2.7.2

^ permalink raw reply related

* [iproute PATCH v2 3/8] man: tc-csum.8: Add an example
From: Phil Sutter @ 2016-03-22 14:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jamal Hadi Salim, netdev
In-Reply-To: <1458658119-25881-1-git-send-email-phil@nwl.cc>

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 man/man8/tc-csum.8 | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/man/man8/tc-csum.8 b/man/man8/tc-csum.8
index 9d00aae346af0..3a64c82f09ba8 100644
--- a/man/man8/tc-csum.8
+++ b/man/man8/tc-csum.8
@@ -49,6 +49,21 @@ UDPLite header
 .TP
 .B SWEETS
 These are merely syntactic sugar and ignored internally.
+.SH EXAMPLES
+The following performs stateless NAT for incoming packets from 192.168.1.100 to
+new destination 18.52.86.120 (0x12345678 in hex). Assuming these are UDP
+packets, both IP and UDP checksums have to be recalculated:
+
+.RS
+.EX
+# tc qdisc add dev eth0 ingress handle ffff:
+# tc filter add eth0 prio 1 protocol ip parent ffff: \\
+	u32 match ip src 192.168.1.100/32 flowid :1 \\
+	action pedit munge ip dst set 0x12345678 pipe \\
+	csum ip and udp
+.EE
+.RE
+
 .SH SEE ALSO
 .BR tc (8),
 .BR tc-pedit (8)
-- 
2.7.2

^ permalink raw reply related

* [iproute PATCH v2 7/8] tc/m_vlan.c: mention CONTROL option in help text
From: Phil Sutter @ 2016-03-22 14:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jamal Hadi Salim, netdev
In-Reply-To: <1458658119-25881-1-git-send-email-phil@nwl.cc>

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tc/m_vlan.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tc/m_vlan.c b/tc/m_vlan.c
index 8d97963f351bc..3233d207375bf 100644
--- a/tc/m_vlan.c
+++ b/tc/m_vlan.c
@@ -22,9 +22,10 @@
 static void explain(void)
 {
 	fprintf(stderr, "Usage: vlan pop\n");
-	fprintf(stderr, "       vlan push [ protocol VLANPROTO ] id VLANID\n");
+	fprintf(stderr, "       vlan push [ protocol VLANPROTO ] id VLANID [CONTROL]\n");
 	fprintf(stderr, "       VLANPROTO is one of 802.1Q or 802.1AD\n");
 	fprintf(stderr, "            with default: 802.1Q\n");
+	fprintf(stderr, "       CONTROL := reclassify | pipe | drop | continue | pass\n");
 }
 
 static void usage(void)
-- 
2.7.2

^ permalink raw reply related

* [iproute PATCH v2 8/8] man: tc-vlan.8: Describe CONTROL option
From: Phil Sutter @ 2016-03-22 14:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jamal Hadi Salim, netdev
In-Reply-To: <1458658119-25881-1-git-send-email-phil@nwl.cc>

This should be made generic and part of a common tc-actions man page.
Though leave it here for now to not confuse readers of the example which
uses it.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 man/man8/tc-vlan.8 | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/man/man8/tc-vlan.8 b/man/man8/tc-vlan.8
index e650b72d3b395..4bfd72b129aaf 100644
--- a/man/man8/tc-vlan.8
+++ b/man/man8/tc-vlan.8
@@ -6,13 +6,17 @@ vlan - vlan manipulation module
 .in +8
 .ti -8
 .BR tc " ... " "action vlan" " { " pop " |"
-.IR PUSH " }"
+.IR PUSH " } [ " CONTROL " ]"
 
 .ti -8
 .IR PUSH " := "
 .BR push " [ " protocol
 .IR VLANPROTO " ]"
 .BI id " VLANID"
+
+.ti -8
+.IR CONTROL " := { "
+.BR reclassify " | " pipe " | " drop " | " continue " | " pass " }"
 .SH DESCRIPTION
 The
 .B vlan
@@ -50,5 +54,55 @@ for hexadecimal interpretation, etc.).
 .BI protocol " VLANPROTO"
 Choose the VLAN protocol to use. At the time of writing, the kernel accepts only
 .BR 802.1Q " or " 802.1ad .
+.TP
+.I CONTROL
+How to continue after executing this action.
+.RS
+.TP
+.B reclassify
+Restarts classification by jumping back to the first filter attached to this
+action's parent.
+.TP
+.B pipe
+Continue with the next action, this is the default.
+.TP
+.B drop
+Packet will be dropped without running further actions.
+.TP
+.B continue
+Continue classification with next filter in line.
+.TP
+.B pass
+Return to calling qdisc for packet processing. This ends the classification
+process.
+.RE
+.SH EXAMPLES
+The following example encapsulates incoming ICMP packets on eth0 from 10.0.0.2
+into VLAN ID 123:
+
+.RS
+.EX
+#tc qdisc add dev eth0 handle ffff: ingress
+#tc filter add dev eth0 parent ffff: pref 11 protocol ip \\
+	u32 match ip protocol 1 0xff flowid 1:1 \\
+	u32 match ip src 10.0.0.2 flowid 1:1 \\
+	action vlan push id 123
+.EE
+.RE
+
+Here is an example of the
+.B pop
+function: Incoming VLAN packets on eth0 are decapsulated and the classification
+process then restarted for the plain packet:
+
+.RS
+.EX
+#tc qdisc add dev eth0 handle ffff: ingress
+#tc filter add dev $ETH parent ffff: pref 1 protocol 802.1Q \\
+	u32 match u32 0 0 flowid 1:1 \\
+	action vlan pop reclassify
+.EE
+.RE
+
 .SH SEE ALSO
 .BR tc (8)
-- 
2.7.2

^ permalink raw reply related

* [iproute PATCH v2 2/8] tc: connmark, pedit: Rename BRANCH to CONTROL
From: Phil Sutter @ 2016-03-22 14:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jamal Hadi Salim, netdev
In-Reply-To: <1458658119-25881-1-git-send-email-phil@nwl.cc>

As Jamal suggested, BRANCH is the wrong name, as these keywords go
beyond simple branch control - e.g. loops are possible, too. Therefore
rename the non-terminal to CONTROL instead which should be more
appropriate.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 man/man8/tc-connmark.8 | 6 +++---
 man/man8/tc-pedit.8    | 6 +++---
 tc/m_connmark.c        | 4 ++--
 tc/m_pedit.c           | 4 ++--
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/man/man8/tc-connmark.8 b/man/man8/tc-connmark.8
index bb4cf7543dfdb..44f29f508d673 100644
--- a/man/man8/tc-connmark.8
+++ b/man/man8/tc-connmark.8
@@ -6,12 +6,12 @@ connmark - netfilter connmark retriever action
 .in +8
 .ti -8
 .BR tc " ... " "action connmark " [ " zone"
-.IR u16_zone_index " ] [ " BRANCH " ] ["
+.IR u16_zone_index " ] [ " CONTROL " ] ["
 .BI index " u32_index "
 ]
 
 .ti -8
-.IR BRANCH " := { " reclassify " | " pipe " | " drop " | " continue " | " ok " }"
+.IR CONTROL " := { " reclassify " | " pipe " | " drop " | " continue " | " ok " }"
 .SH DESCRIPTION
 The connmark action is used to restore the connection's mark value into the
 packet's fwmark.
@@ -22,7 +22,7 @@ Specify the conntrack zone when doing conntrack lookups for packets.
 .I u16_zone_index
 is a 16bit unsigned decimal value.
 .TP
-.I BRANCH
+.I CONTROL
 How to continue after executing this action.
 .RS
 .TP
diff --git a/man/man8/tc-pedit.8 b/man/man8/tc-pedit.8
index c30927ec50954..c34520c046a6c 100644
--- a/man/man8/tc-pedit.8
+++ b/man/man8/tc-pedit.8
@@ -6,7 +6,7 @@ pedit - generic packet editor action
 .in +8
 .ti -8
 .BR tc " ... " "action pedit munge " {
-.IR RAW_OP " | " LAYERED_OP " } [ " BRANCH " ]"
+.IR RAW_OP " | " LAYERED_OP " } [ " CONTROL " ]"
 
 .ti -8
 .IR RAW_OP " := "
@@ -45,7 +45,7 @@ pedit - generic packet editor action
 .IR RVAL " ]"
 
 .ti -8
-.IR BRANCH " := {"
+.IR CONTROL " := {"
 .BR reclassify " | " pipe " | " drop " | " shot " | " continue " | " pass " }"
 .SH DESCRIPTION
 The
@@ -165,7 +165,7 @@ This optional extra part of
 .I CMD_SPEC
 allows to exclude bits from being changed.
 .TP
-.I BRANCH
+.I CONTROL
 The following keywords allow to control how the tree of qdisc, classes,
 filters and actions is further traversed after this action.
 .RS
diff --git a/tc/m_connmark.c b/tc/m_connmark.c
index 2414f321c1f8f..b1c7d3af54cc4 100644
--- a/tc/m_connmark.c
+++ b/tc/m_connmark.c
@@ -27,10 +27,10 @@
 static void
 explain(void)
 {
-	fprintf(stderr, "Usage: ... connmark [zone ZONE] [BRANCH] [index <INDEX>]\n");
+	fprintf(stderr, "Usage: ... connmark [zone ZONE] [CONTROL] [index <INDEX>]\n");
 	fprintf(stderr, "where :\n"
 		"\tZONE is the conntrack zone\n"
-		"\tBRANCH := reclassify|pipe|drop|continue|ok\n");
+		"\tCONTROL := reclassify|pipe|drop|continue|ok\n");
 }
 
 static void
diff --git a/tc/m_pedit.c b/tc/m_pedit.c
index ca78a83dd9d9d..9fe1a7ae3b90c 100644
--- a/tc/m_pedit.c
+++ b/tc/m_pedit.c
@@ -35,7 +35,7 @@ static int pedit_debug;
 static void
 explain(void)
 {
-	fprintf(stderr, "Usage: ... pedit munge <MUNGE> [<BRANCH>]\n");
+	fprintf(stderr, "Usage: ... pedit munge <MUNGE> [CONTROL]\n");
 	fprintf(stderr,
 		"Where: MUNGE := <RAW>|<LAYERED>\n"
 		"\t<RAW>:= <OFFSETC>[ATC]<CMD>\n \t\tOFFSETC:= offset <offval> <u8|u16|u32>\n "
@@ -43,7 +43,7 @@ explain(void)
 		"\t\tNOTE: maskval is a 32 bit hex number\n \t\tNOTE: shiftval is a is a shift value\n "
 		"\t\tCMD:= clear | invert | set <setval>| retain\n \t<LAYERED>:= ip <ipdata> | ip6 <ip6data> \n "
 		" \t\t| udp <udpdata> | tcp <tcpdata> | icmp <icmpdata>\n"
-		"\t<BRANCH>:= reclassify | pipe | drop | continue | pass\n"
+		"\tCONTROL:= reclassify | pipe | drop | continue | pass\n"
 		"For Example usage look at the examples directory\n");
 
 }
-- 
2.7.2

^ permalink raw reply related

* [iproute PATCH v2 4/8] man: tc-mirred.8: Reword man page a bit, add generic mirror example
From: Phil Sutter @ 2016-03-22 14:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jamal Hadi Salim, netdev
In-Reply-To: <1458658119-25881-1-git-send-email-phil@nwl.cc>

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 man/man8/tc-mirred.8 | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/man/man8/tc-mirred.8 b/man/man8/tc-mirred.8
index 52d98bc416563..bba96e0e5d8c0 100644
--- a/man/man8/tc-mirred.8
+++ b/man/man8/tc-mirred.8
@@ -21,11 +21,9 @@ mirred - mirror/redirect action
 .SH DESCRIPTION
 The
 .B mirred
-action allows to redirect or mirror packets to another network interface on the
-same system. It is typically used in combination with the
-.B ifb
-pseudo device to create a shrared instance where QoS happens, but serves well
-for debugging or monitoring purposes, too.
+action allows packet mirroring (copying) or redirecting (stealing) the packet it
+receives. Mirroring is what is sometimes referred to as Switch Port Analyzer
+(SPAN) and is commonly used to analyze and/or debug flows.
 .SH OPTIONS
 .TP
 .B ingress
@@ -67,9 +65,23 @@ debugging purposes:
 .EE
 .RE
 
-Use an
+Mirror all incoming ICMP packets on eth0 to a dummy interface for examination
+with e.g. tcpdump:
+
+.RS
+.EX
+# ip link add dummy0 type dummy
+# ip link set dummy0 up
+# tc qdisc add dev eth0 handle ffff: ingress
+# tc filter add dev eth0 parent ffff: protocol ip \\
+	u32 match ip protocol 1 0xff \\
+	action mirred egress mirror dev dummy0
+.EE
+.RE
+
+Using an
 .B ifb
-interface to send ingress traffic on eth0 through an instance of
+interface, it is possible to send ingress traffic through an instance of
 .BR sfq :
 
 .RS
-- 
2.7.2

^ permalink raw reply related

* [iproute PATCH v2 1/8] doc/tc-filters.tex: Drop overly subjective paragraphs
From: Phil Sutter @ 2016-03-22 14:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jamal Hadi Salim, netdev, Alexei Starovoitov
In-Reply-To: <1458658119-25881-1-git-send-email-phil@nwl.cc>

Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 doc/tc-filters.tex | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/doc/tc-filters.tex b/doc/tc-filters.tex
index 59127d6672ed7..54cc0c9920ce2 100644
--- a/doc/tc-filters.tex
+++ b/doc/tc-filters.tex
@@ -18,10 +18,6 @@
 \date{January 2016}
 \maketitle
 
-TC, the Traffic Control utility, has been there for a very long time - forever
-in my humble perception. It is still (and has ever been if I'm not mistaken) the
-only tool to configure QoS in Linux.
-
 Standard practice when transmitting packets over a medium which may block (due
 to congestion, e.g.) is to use a queue which temporarily holds these packets. In
 Linux, this queueing approach is where QoS happens: A Queueing Discipline
@@ -496,21 +492,10 @@ kernel itself doesn't.
 
 \section*{Conclusion}
 
-My personal impression is that although the \cmd{tc} utility is an absolute
-necessity for anyone aiming at doing QoS in Linux professionally, there are way
-too many loose ends and trip wires present in it's environment. Contributing to
-this is the fact, that much of the non-essential functionality is redundantly
-available in netfilter. Another problem which adds weight to the first one is a
-general lack of documentation. Of course, there are many HOWTOs and guides in
-the internet, but since it's often not clear how up to date these are, I prefer
-the usual resources such as man or info pages. Surely nothing one couldn't fix
-in hindsight, but quality certainly suffers if the original author of the code
-does not or can not contribute to that.
-
-All that being said, once the steep learning curve has been mastered, the
-conglomerate of (classful) qdiscs, filters and actions provides a highly
-sophisticated and flexible infrastructure to perform QoS, which plays nicely
-along with routing and firewalling setups.
+Once the steep learning curve has been mastered, the conglomerate of (classful)
+qdiscs, filters and actions provides a highly sophisticated and flexible
+infrastructure to perform QoS, which plays nicely along with routing and
+firewalling setups.
 
 
 \section*{Further Reading}
-- 
2.7.2

^ permalink raw reply related

* [iproute PATCH v2 0/8] Follow-up to my action man pages series
From: Phil Sutter @ 2016-03-22 14:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jamal Hadi Salim, netdev

The following patch series aims at addressing feedback provided by Jamal
and Alexei. Thanks a lot for your input!

Changes since v1:
- Rebased onto current master.
- Dropped some leftover TODO note from tc-skbedit.8

Phil Sutter (8):
  doc/tc-filters.tex: Drop overly subjective paragraphs
  tc: connmark, pedit: Rename BRANCH to CONTROL
  man: tc-csum.8: Add an example
  man: tc-mirred.8: Reword man page a bit, add generic mirror example
  man: tc-police.8: Emphasize on the two rate control mechanisms
  man: tc-skbedit.8: Elaborate a bit on TX queues
  tc/m_vlan.c: mention CONTROL option in help text
  man: tc-vlan.8: Describe CONTROL option

 doc/tc-filters.tex     | 23 ++++-----------------
 man/man8/tc-connmark.8 |  6 +++---
 man/man8/tc-csum.8     | 15 ++++++++++++++
 man/man8/tc-mirred.8   | 26 ++++++++++++++++-------
 man/man8/tc-pedit.8    |  6 +++---
 man/man8/tc-police.8   | 29 ++++++++++++++++++++++----
 man/man8/tc-skbedit.8  | 12 +++++++++++
 man/man8/tc-vlan.8     | 56 +++++++++++++++++++++++++++++++++++++++++++++++++-
 tc/m_connmark.c        |  4 ++--
 tc/m_pedit.c           |  4 ++--
 tc/m_vlan.c            |  3 ++-
 11 files changed, 142 insertions(+), 42 deletions(-)

-- 
2.7.2

^ permalink raw reply

* [iproute PATCH v2 6/8] man: tc-skbedit.8: Elaborate a bit on TX queues
From: Phil Sutter @ 2016-03-22 14:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jamal Hadi Salim, netdev
In-Reply-To: <1458658119-25881-1-git-send-email-phil@nwl.cc>

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 man/man8/tc-skbedit.8 | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/man/man8/tc-skbedit.8 b/man/man8/tc-skbedit.8
index b585a4d4253ba..e6902960eee27 100644
--- a/man/man8/tc-skbedit.8
+++ b/man/man8/tc-skbedit.8
@@ -17,6 +17,18 @@ The
 action allows to change a packet's associated meta data. It complements the
 .B pedit
 action, which in turn allows to change parts of the packet data itself.
+
+The most unique feature of
+.B skbedit
+is it's ability to decide over which queue of an interface with multiple
+transmit queues the packet is to be sent out. The number of available transmit
+queues is reflected by sysfs entries within
+.I /sys/class/net/<interface>/queues
+with name
+.I tx-N
+(where
+.I N
+is the actual queue number).
 .SH OPTIONS
 .TP
 .BI queue_mapping " QUEUE_MAPPING"
-- 
2.7.2

^ permalink raw reply related

* Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB
From: Sebastian Frias @ 2016-03-22 14:53 UTC (permalink / raw)
  To: Uwe Kleine-König, Sergei Shtylyov
  Cc: David S. Miller, netdev, lkml, mason, Daniel Mack
In-Reply-To: <20160321204111.GB6191@pengutronix.de>

Hi,

On 03/21/2016 09:41 PM, Uwe Kleine-König wrote:
>>    My patch basically gets rid of all this code. The thing that worries me
>> is that the driver assumes that the reset singal is active low, despite what
>> the GPIO specifier in the device tree has for the GPIO polarity. In fact, it
>> will only work correctly if the specified has GPIO_ACTIVE_HIGH -- which is
>> wrong because the reset signal is active low!
> 
> Note that gpio descriptors handle the polarity just fine (i.e. the pin
> is set to 0 after doing gpiod_set_value(1) if the gpio is active low).
> 

Isn't that source of bugs?
What about using some #define (or probably better, an enum)?, something
like:

    gpiod_set_value(gpiod, GPIO_SET_VALUE_ACTIVE)
    gpiod_set_value(gpiod, GPIO_SET_VALUE_INACTIVE)
    gpiod_set_value(gpiod, GPIO_SET_VALUE_TRISTATE)

then, somebody reading the code would have to stop and think what do
these mean.
IIUC, currently the "0" or "1" can easily be confused with the actual
logical value of the GPIO.

gpiod_set_value() could also return an int with the actual value it
applied to the GPIO.
For example: if gpiod is active low, gpiod_set_value(gpiod,
GPIO_SET_VALUE_ACTIVE) would return 0;
Conversely, if gpiod is active high, gpiod_set_value(gpiod,
GPIO_SET_VALUE_ACTIVE) would return 1;

Best regards,

Sebastian

> But having said that, the driver gets it wrong.
> 
> The right sequence to reset a device using a gpio is:
> 
> 	gpiod_set_value(priv->gpiod_reset, 1);
> 	msleep(some_time);
> 	gpiod_set_value(priv->gpiod_reset, 0);
> 
> and if the gpio is active low, this should be specified in the device
> tree. This was done wrong in 13a56b449325 (net: phy: at803x: Add support
> for hardware reset).
> 
> Best regards
> Uwe
> 

^ permalink raw reply

* net/sctp: stack-out-of-bounds in sctp_getsockopt
From: Baozeng Ding @ 2016-03-22 15:08 UTC (permalink / raw)
  To: vyasevich, nhorman, davem; +Cc: linux-sctp, netdev, linux-kernel

Hi all,

The following program triggers an out-of-bounds bug in
sctp_getsockopt. The kernel version is 4.5 (on Mar 16
commit 09fd671ccb2475436bd5f597f751ca4a7d177aea). 

==================================================================
BUG: KASAN: stack-out-of-bounds in string+0x1ef/0x200 at addr
ffff88003ae679e0
Read of size 1 by task syz-executor/19753
page:ffffea0000eb99c0 count:0 mapcount:0 mapping:          (null)
index:0x0
flags: 0x1fffc0000000000()
page dumped because: kasan: bad access detected
CPU: 3 PID: 19753 Comm: syz-executor Not tainted 4.5.0+ #8
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
 0000000000000003 ffff88003ae67578 ffffffff82945051 ffff88003ae67608
 ffff88003ae679e0 0000000000000096 dffffc0000000000 ffff88003ae675f8
 ffffffff81709f88 000000000000030d 0000000000000000 0000000000000286
Call Trace:
 [<     inline     >] __dump_stack lib/dump_stack.c:15
 [<ffffffff82945051>] dump_stack+0xb3/0x112 lib/dump_stack.c:51
 [<     inline     >] print_address_description mm/kasan/report.c:150
 [<ffffffff81709f88>] kasan_report_error+0x4f8/0x530 mm/kasan/report.c:236
 [<ffffffff8140785b>] ? __lock_acquire+0x15fb/0x5dd0 kernel/locking/lockdep.c:3226
 [<     inline     >] kasan_report mm/kasan/report.c:259
 [<ffffffff81709ffe>] __asan_report_load1_noabort+0x3e/0x40 mm/kasan/report.c:277
 [<ffffffff8296613f>] ? string+0x1ef/0x200 lib/vsprintf.c:591
 [<ffffffff8296613f>] string+0x1ef/0x200 lib/vsprintf.c:591
 [<ffffffff8296f103>] vsnprintf+0xb83/0x1900 lib/vsprintf.c:2049
 [<ffffffff8296e580>] ? pointer+0xab0/0xab0 lib/vsprintf.c:1584
 [<ffffffff813456f2>] __request_module+0x132/0x6b0 kernel/kmod.c:146
 [<ffffffff814056b0>] ? mark_held_locks+0xd0/0x130 kernel/locking/lockdep.c:2552
 [<ffffffff813455c0>] ? call_usermodehelper_setup+0x2b0/0x2b0 kernel/kmod.c:530
 [<ffffffff85da47b0>] ? mutex_lock_interruptible_nested+0x980/0x980
 [<ffffffff8168fed4>] ? __might_fault+0xe4/0x1d0 mm/memory.c:3833
 [<ffffffff8538f74c>] find_inlist_lock.constprop.17+0x10c/0x210 net/bridge/netfilter/ebtables.c:347
 [<     inline     >] find_table_lock net/bridge/netfilter/ebtables.c:356
 [<ffffffff853904ab>] do_ebt_get_ctl+0x13b/0x540 net/bridge/netfilter/ebtables.c:1524
 [<ffffffff85390370>] ? copy_everything_to_user+0x600/0x600 net/bridge/netfilter/ebtables.c:1455
 [<     inline     >] ? __mutex_unlock_common_slowpath kernel/locking/mutex.c:751
 [<ffffffff85da6799>] ? __mutex_unlock_slowpath+0x239/0x3f0 kernel/locking/mutex.c:762
 [<ffffffff85da6959>] ? mutex_unlock+0x9/0x10 kernel/locking/mutex.c:437
 [<ffffffff84dea126>] ? nf_sockopt_find+0x1a6/0x220 net/netfilter/nf_sockopt.c:87
 [<     inline     >] nf_sockopt net/netfilter/nf_sockopt.c:103
 [<ffffffff84dea20d>] nf_getsockopt+0x6d/0xc0 net/netfilter/nf_sockopt.c:121
 [<ffffffff84fadf05>] ip_getsockopt+0x135/0x190 net/ipv4/ip_sockglue.c:1523
 [<ffffffff84faddd0>] ? do_ip_getsockopt+0x1520/0x1520 net/ipv4/ip_sockglue.c:1353
 [<     inline     >] ? wake_up_process kernel/sched/core.c:2024
 [<ffffffff8138bcc2>] ? wake_up_q+0x82/0xe0 kernel/sched/core.c:416
 [<     inline     >] ? atomic_dec_and_test /arch/x86/include/asm/atomic.h:117
 [<     inline     >] ? mmdrop include/linux/sched.h:2611
 [<ffffffff814a3310>] ? drop_futex_key_refs.isra.13+0x70/0xe0 kernel/futex.c:444
 [<ffffffff8583a4dd>] sctp_getsockopt+0x18d/0x3f40 net/sctp/socket.c:5964
 [<ffffffff8140785b>] ? __lock_acquire+0x15fb/0x5dd0 kernel/locking/lockdep.c:3226
 [<ffffffff8583a350>] ? sctp_do_peeloff+0x2b0/0x2b0 net/sctp/socket.c:4434
 [<ffffffff81406260>] ? debug_check_no_locks_freed+0x290/0x290 kernel/locking/lockdep.c:4104
 [<     inline     >] ? rcu_read_unlock include/linux/rcupdate.h:922
 [<ffffffff817b398c>] ? __fget+0x20c/0x3b0 fs/file.c:712
 [<     inline     >] ? rcu_lock_release include/linux/rcupdate.h:491
 [<     inline     >] ? rcu_read_unlock include/linux/rcupdate.h:926
 [<ffffffff817b39b5>] ? __fget+0x235/0x3b0 fs/file.c:712
 [<ffffffff817b37c7>] ? __fget+0x47/0x3b0 fs/file.c:696
 [<ffffffff817b3c11>] ? __fget_light+0xa1/0x1f0 fs/file.c:759
 [<ffffffff84c3a695>] sock_common_getsockopt+0x95/0xd0 net/core/sock.c:2579
 [<     inline     >] SYSC_getsockopt net/socket.c:1783
 [<ffffffff84c37e12>] SyS_getsockopt+0x142/0x230 net/socket.c:1765
 [<ffffffff84c37cd0>] ? SyS_setsockopt+0x240/0x240 net/socket.c:1752
 [<ffffffff85dab922>] ? entry_SYSCALL_64_fastpath+0x5/0xc1 arch/x86/entry/entry_64.S:191
 [<ffffffff81003017>] ? trace_hardirqs_on_thunk+0x17/0x19 arch/x86/entry/thunk_64.S:39
 [<ffffffff85dab940>] entry_SYSCALL_64_fastpath+0x23/0xc1 arch/x86/entry/entry_64.S:207
Memory state around the buggy address:
 ffff88003ae67880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 ffff88003ae67900: 00 f1 f1 f1 f1 04 f4 f4 f4 f2 f2 f2 f2 00 00 00
>ffff88003ae67980: 00 00 00 00 00 00 00 00 00 00 00 00 f4 f3 f3 f3
                                                       ^
 ffff88003ae67a00: f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 ffff88003ae67a80: f1 f1 f1 f1 04 f4 f4 f4 f3 f3 f3 f3 00 00 00 00
==================================================================

#include <unistd.h>
#include <sys/syscall.h>
#include <netinet/in.h>
#include <string.h>
#include <stdint.h>
#include <sys/mman.h>
#include <sys/socket.h>

int main()
{
        int sock = 0;
        int sock_dup = 0;
        mmap((void *)0x20000000ul, 0x5000ul, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0);
        sock = socket(AF_INET, SOCK_STREAM, IPPROTO_SCTP);
        sock_dup = dup(sock);
        memcpy((void*)0x20000bf3,"\xac\x71\x93\x68\x02\xb3\xd1\x86\x52\xf1\xf0\x18\x09\x56\xc6\x98\x6f\x8e\x74\xb7\x17\xd4\x3a\x64\x51\x68\x13\x2d\x25\xba\x6d\x3f\x74\x68\x84\x89\x04\xd1\xa6\xe2\x7d\xaf\xfa\xd9\xce\x52\xbe\x6f\xb6\xe3\xff\x92\x35\xa1\x88\x4a\x68\x27\xaa\x25\xf8\xc1\xd5\x3b\xe5\x69\x11\x4f\x75\x4c\xe9\xff\x8b\x86\x53\x20\xb7\x10\xa2\x62\xcc\xc3\x06\x85\xde\x3e\x1c\x5a\x62\x3a\x2d\x0d\x0b\x0c\xb2\xac\x75\x42\x4d\x82\x3f\x7b\xf7\x28\xea\x2d\xff\x42\xa8\xdf\xb3\x49\x1a\xfd\xae\x2c\xd4\x35\x8e\x96\xb3\xe1\x0a\x92\x56\xb7\xde\xe8\x9e\xc3\x9e\x88\x79\xc4\x71\x46\x27\xf4\x9e\x85\xf4\x8f\x1f\x9a\xe5\x7e\x02\x09\x34\x80\x1e\x87\xa8\x9a\xce\xac\xfb\x43\x07\xdf\x15\xe8\x71\x9a\xa3\x80\x18\x1b\x15\xbd\x57\xb6\xc1\x73\x6e\xb1\x28\x3a\x01\xd5\x8e\x15\x85\xbd\x52\xdf\xfa\x64\xaa\x13\x0e\x
 2f\x64\x05\x11\xce\x79\x8b\xa8\x02\x29\x7f\x72\x0f\x37\x89\xb4\x54\x0b\x09\x02\x75\xc2\x8e\xd7\xcd\x7e\xfb\x4f\x72\xf1\x47\xea\xa2\x2a\xc3\xc4\xe9\x70\xfe\xa5\x80\x88\x21\x33\xcf\x13\x66\x98\x23\x10\x5c\xa4\xbd\xee\xc0\xb4\xdd\xfb\xff\xf2\x38\xab\xca\x36\x62\x35\x84\xe4\x73\x5c\xc7\x3e\x72\x2e\x17\x43\x6f\x85\x45\x4f\x82\x62\x0d\x77\xae\xcb\xe1\x8f\xe8\xf0\x84\x3e\x62\x8b\x70\x2b\x55\xb5\xa7\x13\xcf\xa1\x78\x77\x82\xe2\xb7\x1c\x65\x7f\xb5\x79\x73\x01\x07\xd1\x9f\x45\x6a\xbb\x3d\xbf\xc8\x71\x5b\x9f\x30\xc7\xb9\xb8\x53\x9f\xe1\xba\xb6\x78\x9e\x05\x75\xa3\x55\xb1\x26\x96\xa9\xb2\x82\xce\x81\x5c\x8a\x18\xb3\x4b\x0c\x18\x8c\xf2\x7c\x09\xde\xcb\xcf\x78\x22\x58\xf6\x15\xf6\xf7\x48\xda\x08\x75\xd4\xc1\x20\xc3\x18\x2e\x89\xe8\x5b\x48\xd9\xbc\x1f\xbb\xed\x31\xaf\x12\x4d\xcd\x46\x60\xa0\xef\x0e\x2e
 \x21\x1d\x2b\x68\x75\xb9\x42\x5e\xd7\xae\x35\x46\xe9\x06\x63\x1d\x3c\xd6\x9c\x14\x3b\x09\x29\x49\x70\xb9\xe1\xe0\x09\x45\x41\x62\x0c\xff\x5a\x77\xbe\x31\xa6\x03\x94\x92\xde\x41\x99\xfa\x68\x99\x74\xbb\x0a\x3d\xac\x9c\x7e\x00\x6b\xcd\xc1\x83\xa7\xc5\x63\xdd\x10\xea\x59\x27\xdc\x02\x98\xd6\x43\x20\x24\x4e\xc0\xdc\xa2\x98\xdf\x3e\xaf\x61\x35\xa0\x95\x3f\x9a\xaa\x7d\xe9\xe9\x0d\xe5\x97\x66\x1a\x9f\xbf\x56\xc8\x37\x84\x18\x2b\xd2\xcd\xd6\xb3\x19\xd8\x4a\x30\x6e\xcb\x99\x1c\xe9\x0f\xdb\xca\x30\xe1\xe2\x90\xba\xb9\x61\x00\xbf\xeb\xad\x6a\xc8\x52\xea\x1a\x92\x05\x0c\x3b\x78\x82\x01\xac\xfd\x88\x6c\xca\xe2\xfb\xe7\x0f\xcc\x75\x9c\x98\x12\x26\xcf\xa6\x80\x02\x35\xdf\x6e\xe1\x11\x1d\xa7\x30\x17\x38\x41\xd9\x81\x55\x1a\x1e\xd1\xfe\x60\xbf\xef\x09\x25\xc0\xdb\x9f\xc4\xc6\x54\x1a\x85\x36\x85\x05\xb3\x
 9f\x2c\xc5\xcd\x12\x51\xef\xbe\x10\x79\xbf\x11\x00\x47\x0d\x9c\x14\x43\x1a\x46\xea\xd1\x34\x2e\x10\x6b\xa4\x3c\x25\x21\xe3\xb9\x15\x78\x6c\x40\x87\x90\xf7\x93\x5a\x66\x5f\x0a\x76\xff\xc2\xe2\x14\x35\x88\x47\xa1\x33\x5b\x8f\x3d\xc5\x89\xb7\xf9\x8a\x40\xf0\x1e\xc9\x30\xcd\xd8\x96\x41\x78\x58\x97\x49\xc8\x50\x61\x36\x8f\x7e\x44\x41\xc0\x84\xbb\x35\xf0\x63\xa9\xc2\x2a\xbd\xcc\x4b\xab\x8b\x16\x33\xc0\x66\xbf\x47\x62\x9b\xc4\x47\x2d\x68\x83\xca\xe3\x52\x79\xd7\xe0\x61\x80\x15\xf1\x90\x83\xa2\xbb\x4c\xe5\x8b\x50\xc8\x1b\x68\x7b\xee\x57\xdc\x54\xfa\x90\xf1\xf5\xec\x7d\x93\xe0\x80\x74\x06\xbe\xac\xc8\x85\x4d\xe8\xbf\xd3\xdd\x34\x55\xc4\xbf\x2f\x24\x19\xad\x86\x1e\x69\x2b\x6c\x3f\x00\xe8\x4b\xbb\x99\xcf\x17\x99\x00\x9d\x6c\x70\x57\xcc\x35\xee\x07\x87\x25\x8c\x0c\x8b\x9b\x38\x15\xcc\x05\x6f\xf8\x16
 \x78\x0b\x41\xfa\x23\x96\xc0\x79\xf8\xb7\xf0\x2b\x60\x7e\x98\xe3\x7b\xab\x80\x1f\x0d\xbf\xf6\x7e\x37\x06\xf1\x11\x42\x38\x2a\x70\xdf\xa4\xca\xf5\xf3\xf4\x7d\xca\x10\x0c\xd5\xe2\x90\xa0\x15\xde\xc2\x61\xa2\x88\xea\x32\x37\x97\x83\xd0\x4c\xad\xe2\xae\x9b\x53\xa2\xc2\x54\x0c\xbd\xe1\x50\x3b\x15\xd4\xb1\xa9\x41\x6e\x18\x2e\x30\x3f\x91\x03\x81\x86\x8c\x5c\x1f\x76\x51\x92\xf5\xb5\xb2\xc3\x16\x01\xef\xe3\x9e\xb1\x92\x0e\x0e\xcb\x20\x7f\x10\x29\x08\x6e\x15\x3d\x1e\x7c\x70\xf5\xb5\x3c\x56\x15\x3c\x59\xe6\xe7\x9e\x16\xcd\xfc\x8e\xfa\x12\x99\xbb\x07\xaa\xd7\x1c\xd0\xae\x93\x4c\xba\x16\x5d\x0c\xed\x1d\x02\x87\xcd\x38\x31\xc6\x10\x42\xe1\x46\x4e\xa3\xae\xb6\xda\xb6\xb0\x49\x55\x89\x57\xe6\xac\xe3\xbf\xb5\x5c\x59\x93\x0d\x21\x35\xdd\x57\x8c\x04\x15\x91\x05\x69\x4a\xdb\x5e\xcb\x4d\xa3\x5d\xa8\x7e\x95\x
 9e\x9d\x95\x61\xc9\x1c\xdd\x66\x0a\x76\x18\xbb\x59\x6a\xa5\xc0\xf2\xb8\x2f\xa9\x4c\xa8\xb3\x2b\xa3\x8a\xbf\x5c\xe8\x18\x3d\x7f\x0e\x2f\xe9\x06\xf9\xb6\xcc\x60\xcc\x38\x6c\x9a\x78\xa7\x7c\x61",
1037);
        getsockopt(sock_dup, IPPROTO_IP, 0x81,  (void *)0x20000bf3ul,
(socklen_t *)0x20003000ul);        
        return 0;
}

Best Regards,

Baozeng Ding

^ permalink raw reply

* Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
From: Alan Stern @ 2016-03-22 15:13 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: David S. Miller, Geert Uytterhoeven,
	Microchip Linux Driver Support, Woojung Huh, Rafael J. Wysocki,
	Guenter Roeck, linux-kernel, linux-pm, linux-usb, netdev
In-Reply-To: <1458656903.1990.12.camel@suse.com>

On Tue, 22 Mar 2016, Oliver Neukum wrote:

> On Tue, 2016-03-22 at 10:21 -0400, Alan Stern wrote:
> > I don't see any point in resuming the device just in order to collect 
> > operating statistics.  If it was already suspended then it wasn't 
> > operating, so there will be no statistics to collect.
> 
> Indeed. In that case the point is moot. But it is correct to ask
> the core whether the device is autosuspended at that point rather
> than keep a private flag if you can.

That's why we have pm_runtime_status_suspended().

> All that is relevant only if the upper layers ask for information
> that the driver cannot provide without resuming the device.
> Those are fundamentally different issues.

Of course.

Alan Stern


^ permalink raw reply

* Re: net/sctp: stack-out-of-bounds in sctp_getsockopt
From: Eric Dumazet @ 2016-03-22 15:21 UTC (permalink / raw)
  To: Baozeng Ding; +Cc: vyasevich, nhorman, davem, linux-sctp, netdev, linux-kernel
In-Reply-To: <56f16034.890c620a.e04dc.ffffc770@mx.google.com>

On Tue, 2016-03-22 at 23:08 +0800, Baozeng Ding wrote:
> Hi all,
> 
> The following program triggers an out-of-bounds bug in
> sctp_getsockopt. The kernel version is 4.5 (on Mar 16
> commit 09fd671ccb2475436bd5f597f751ca4a7d177aea). 
> 
> ==================================================================
> BUG: KASAN: stack-out-of-bounds in string+0x1ef/0x200 at addr
> ffff88003ae679e0
> Read of size 1 by task syz-executor/19753
> page:ffffea0000eb99c0 count:0 mapcount:0 mapping:          (null)
> index:0x0
> flags: 0x1fffc0000000000()
> page dumped because: kasan: bad access detected
> CPU: 3 PID: 19753 Comm: syz-executor Not tainted 4.5.0+ #8
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
>  0000000000000003 ffff88003ae67578 ffffffff82945051 ffff88003ae67608
>  ffff88003ae679e0 0000000000000096 dffffc0000000000 ffff88003ae675f8
>  ffffffff81709f88 000000000000030d 0000000000000000 0000000000000286
> Call Trace:
>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>  [<ffffffff82945051>] dump_stack+0xb3/0x112 lib/dump_stack.c:51
>  [<     inline     >] print_address_description mm/kasan/report.c:150
>  [<ffffffff81709f88>] kasan_report_error+0x4f8/0x530 mm/kasan/report.c:236
>  [<ffffffff8140785b>] ? __lock_acquire+0x15fb/0x5dd0 kernel/locking/lockdep.c:3226
>  [<     inline     >] kasan_report mm/kasan/report.c:259
>  [<ffffffff81709ffe>] __asan_report_load1_noabort+0x3e/0x40 mm/kasan/report.c:277
>  [<ffffffff8296613f>] ? string+0x1ef/0x200 lib/vsprintf.c:591
>  [<ffffffff8296613f>] string+0x1ef/0x200 lib/vsprintf.c:591
>  [<ffffffff8296f103>] vsnprintf+0xb83/0x1900 lib/vsprintf.c:2049
>  [<ffffffff8296e580>] ? pointer+0xab0/0xab0 lib/vsprintf.c:1584
>  [<ffffffff813456f2>] __request_module+0x132/0x6b0 kernel/kmod.c:146
>  [<ffffffff814056b0>] ? mark_held_locks+0xd0/0x130 kernel/locking/lockdep.c:2552
>  [<ffffffff813455c0>] ? call_usermodehelper_setup+0x2b0/0x2b0 kernel/kmod.c:530
>  [<ffffffff85da47b0>] ? mutex_lock_interruptible_nested+0x980/0x980
>  [<ffffffff8168fed4>] ? __might_fault+0xe4/0x1d0 mm/memory.c:3833
>  [<ffffffff8538f74c>] find_inlist_lock.constprop.17+0x10c/0x210 net/bridge/netfilter/ebtables.c:347
>  [<     inline     >] find_table_lock net/bridge/netfilter/ebtables.c:356
>  [<ffffffff853904ab>] do_ebt_get_ctl+0x13b/0x540 net/bridge/netfilter/ebtables.c:1524
>  [<ffffffff85390370>] ? copy_everything_to_user+0x600/0x600 net/bridge/netfilter/ebtables.c:1455
>  [<     inline     >] ? __mutex_unlock_common_slowpath kernel/locking/mutex.c:751
>  [<ffffffff85da6799>] ? __mutex_unlock_slowpath+0x239/0x3f0 kernel/locking/mutex.c:762
>  [<ffffffff85da6959>] ? mutex_unlock+0x9/0x10 kernel/locking/mutex.c:437
>  [<ffffffff84dea126>] ? nf_sockopt_find+0x1a6/0x220 net/netfilter/nf_sockopt.c:87
>  [<     inline     >] nf_sockopt net/netfilter/nf_sockopt.c:103
>  [<ffffffff84dea20d>] nf_getsockopt+0x6d/0xc0 net/netfilter/nf_sockopt.c:121
>  [<ffffffff84fadf05>] ip_getsockopt+0x135/0x190 net/ipv4/ip_sockglue.c:1523
>  [<ffffffff84faddd0>] ? do_ip_getsockopt+0x1520/0x1520 net/ipv4/ip_sockglue.c:1353
>  [<     inline     >] ? wake_up_process kernel/sched/core.c:2024
>  [<ffffffff8138bcc2>] ? wake_up_q+0x82/0xe0 kernel/sched/core.c:416
>  [<     inline     >] ? atomic_dec_and_test /arch/x86/include/asm/atomic.h:117
>  [<     inline     >] ? mmdrop include/linux/sched.h:2611
>  [<ffffffff814a3310>] ? drop_futex_key_refs.isra.13+0x70/0xe0 kernel/futex.c:444
>  [<ffffffff8583a4dd>] sctp_getsockopt+0x18d/0x3f40 net/sctp/socket.c:5964
>  [<ffffffff8140785b>] ? __lock_acquire+0x15fb/0x5dd0 kernel/locking/lockdep.c:3226
>  [<ffffffff8583a350>] ? sctp_do_peeloff+0x2b0/0x2b0 net/sctp/socket.c:4434
>  [<ffffffff81406260>] ? debug_check_no_locks_freed+0x290/0x290 kernel/locking/lockdep.c:4104
>  [<     inline     >] ? rcu_read_unlock include/linux/rcupdate.h:922
>  [<ffffffff817b398c>] ? __fget+0x20c/0x3b0 fs/file.c:712
>  [<     inline     >] ? rcu_lock_release include/linux/rcupdate.h:491
>  [<     inline     >] ? rcu_read_unlock include/linux/rcupdate.h:926
>  [<ffffffff817b39b5>] ? __fget+0x235/0x3b0 fs/file.c:712
>  [<ffffffff817b37c7>] ? __fget+0x47/0x3b0 fs/file.c:696
>  [<ffffffff817b3c11>] ? __fget_light+0xa1/0x1f0 fs/file.c:759
>  [<ffffffff84c3a695>] sock_common_getsockopt+0x95/0xd0 net/core/sock.c:2579
>  [<     inline     >] SYSC_getsockopt net/socket.c:1783
>  [<ffffffff84c37e12>] SyS_getsockopt+0x142/0x230 net/socket.c:1765
>  [<ffffffff84c37cd0>] ? SyS_setsockopt+0x240/0x240 net/socket.c:1752
>  [<ffffffff85dab922>] ? entry_SYSCALL_64_fastpath+0x5/0xc1 arch/x86/entry/entry_64.S:191
>  [<ffffffff81003017>] ? trace_hardirqs_on_thunk+0x17/0x19 arch/x86/entry/thunk_64.S:39
>  [<ffffffff85dab940>] entry_SYSCALL_64_fastpath+0x23/0xc1 arch/x86/entry/entry_64.S:207
> Memory state around the buggy address:
>  ffff88003ae67880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  ffff88003ae67900: 00 f1 f1 f1 f1 04 f4 f4 f4 f2 f2 f2 f2 00 00 00
> >ffff88003ae67980: 00 00 00 00 00 00 00 00 00 00 00 00 f4 f3 f3 f3
>                                                        ^
>  ffff88003ae67a00: f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  ffff88003ae67a80: f1 f1 f1 f1 04 f4 f4 f4 f3 f3 f3 f3 00 00 00 00
> ==================================================================
> 
> #include <unistd.h>
> #include <sys/syscall.h>
> #include <netinet/in.h>
> #include <string.h>
> #include <stdint.h>
> #include <sys/mman.h>
> #include <sys/socket.h>
> 
> int main()
> {
>         int sock = 0;
>         int sock_dup = 0;
>         mmap((void *)0x20000000ul, 0x5000ul, PROT_READ|PROT_WRITE,
> MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0);
>         sock = socket(AF_INET, SOCK_STREAM, IPPROTO_SCTP);
>         sock_dup = dup(sock);
>         memcpy((void*)0x20000bf3,"\xac\x71\x93\x68\x02\xb3\xd1\x86\x52\xf1\xf0\x18\x09\x56\xc6\x98\x6f\x8e\x74\xb7\x17\xd4\x3a\x64\x51\x68\x13\x2d\x25\xba\x6d\x3f\x74\x68\x84\x89\x04\xd1\xa6\xe2\x7d\xaf\xfa\xd9\xce\x52\xbe\x6f\xb6\xe3\xff\x92\x35\xa1\x88\x4a\x68\x27\xaa\x25\xf8\xc1\xd5\x3b\xe5\x69\x11\x4f\x75\x4c\xe9\xff\x8b\x86\x53\x20\xb7\x10\xa2\x62\xcc\xc3\x06\x85\xde\x3e\x1c\x5a\x62\x3a\x2d\x0d\x0b\x0c\xb2\xac\x75\x42\x4d\x82\x3f\x7b\xf7\x28\xea\x2d\xff\x42\xa8\xdf\xb3\x49\x1a\xfd\xae\x2c\xd4\x35\x8e\x96\xb3\xe1\x0a\x92\x56\xb7\xde\xe8\x9e\xc3\x9e\x88\x79\xc4\x71\x46\x27\xf4\x9e\x85\xf4\x8f\x1f\x9a\xe5\x7e\x02\x09\x34\x80\x1e\x87\xa8\x9a\xce\xac\xfb\x43\x07\xdf\x15\xe8\x71\x9a\xa3\x80\x18\x1b\x15\xbd\x57\xb6\xc1\x73\x6e\xb1\x28\x3a\x01\xd5\x8e\x15\x85\xbd\x52\xdf\xfa\x64\xaa\x13\x0e
 \x2f\x64\x05\x11\xce\x79\x8b\xa8\x02\x29\x7f\x72\x0f\x37\x89\xb4\x54\x0b\x09\x02\x75\xc2\x8e\xd7\xcd\x7e\xfb\x4f\x72\xf1\x47\xea\xa2\x2a\xc3\xc4\xe9\x70\xfe\xa5\x80\x88\x21\x33\xcf\x13\x66\x98\x23\x10\x5c\xa4\xbd\xee\xc0\xb4\xdd\xfb\xff\xf2\x38\xab\xca\x36\x62\x35\x84\xe4\x73\x5c\xc7\x3e\x72\x2e\x17\x43\x6f\x85\x45\x4f\x82\x62\x0d\x77\xae\xcb\xe1\x8f\xe8\xf0\x84\x3e\x62\x8b\x70\x2b\x55\xb5\xa7\x13\xcf\xa1\x78\x77\x82\xe2\xb7\x1c\x65\x7f\xb5\x79\x73\x01\x07\xd1\x9f\x45\x6a\xbb\x3d\xbf\xc8\x71\x5b\x9f\x30\xc7\xb9\xb8\x53\x9f\xe1\xba\xb6\x78\x9e\x05\x75\xa3\x55\xb1\x26\x96\xa9\xb2\x82\xce\x81\x5c\x8a\x18\xb3\x4b\x0c\x18\x8c\xf2\x7c\x09\xde\xcb\xcf\x78\x22\x58\xf6\x15\xf6\xf7\x48\xda\x08\x75\xd4\xc1\x20\xc3\x18\x2e\x89\xe8\x5b\x48\xd9\xbc\x1f\xbb\xed\x31\xaf\x12\x4d\xcd\x46\x60\xa0\xef\x0e\x
 2e\x21\x1d\x2b\x68\x75\xb9\x42\x5e\xd7\xae\x35\x46\xe9\x06\x63\x1d\x3c\xd6\x9c\x14\x3b\x09\x29\x49\x70\xb9\xe1\xe0\x09\x45\x41\x62\x0c\xff\x5a\x77\xbe\x31\xa6\x03\x94\x92\xde\x41\x99\xfa\x68\x99\x74\xbb\x0a\x3d\xac\x9c\x7e\x00\x6b\xcd\xc1\x83\xa7\xc5\x63\xdd\x10\xea\x59\x27\xdc\x02\x98\xd6\x43\x20\x24\x4e\xc0\xdc\xa2\x98\xdf\x3e\xaf\x61\x35\xa0\x95\x3f\x9a\xaa\x7d\xe9\xe9\x0d\xe5\x97\x66\x1a\x9f\xbf\x56\xc8\x37\x84\x18\x2b\xd2\xcd\xd6\xb3\x19\xd8\x4a\x30\x6e\xcb\x99\x1c\xe9\x0f\xdb\xca\x30\xe1\xe2\x90\xba\xb9\x61\x00\xbf\xeb\xad\x6a\xc8\x52\xea\x1a\x92\x05\x0c\x3b\x78\x82\x01\xac\xfd\x88\x6c\xca\xe2\xfb\xe7\x0f\xcc\x75\x9c\x98\x12\x26\xcf\xa6\x80\x02\x35\xdf\x6e\xe1\x11\x1d\xa7\x30\x17\x38\x41\xd9\x81\x55\x1a\x1e\xd1\xfe\x60\xbf\xef\x09\x25\xc0\xdb\x9f\xc4\xc6\x54\x1a\x85\x36\x85\x05\xb3
 \x9f\x2c\xc5\xcd\x12\x51\xef\xbe\x10\x79\xbf\x11\x00\x47\x0d\x9c\x14\x43\x1a\x46\xea\xd1\x34\x2e\x10\x6b\xa4\x3c\x25\x21\xe3\xb9\x15\x78\x6c\x40\x87\x90\xf7\x93\x5a\x66\x5f\x0a\x76\xff\xc2\xe2\x14\x35\x88\x47\xa1\x33\x5b\x8f\x3d\xc5\x89\xb7\xf9\x8a\x40\xf0\x1e\xc9\x30\xcd\xd8\x96\x41\x78\x58\x97\x49\xc8\x50\x61\x36\x8f\x7e\x44\x41\xc0\x84\xbb\x35\xf0\x63\xa9\xc2\x2a\xbd\xcc\x4b\xab\x8b\x16\x33\xc0\x66\xbf\x47\x62\x9b\xc4\x47\x2d\x68\x83\xca\xe3\x52\x79\xd7\xe0\x61\x80\x15\xf1\x90\x83\xa2\xbb\x4c\xe5\x8b\x50\xc8\x1b\x68\x7b\xee\x57\xdc\x54\xfa\x90\xf1\xf5\xec\x7d\x93\xe0\x80\x74\x06\xbe\xac\xc8\x85\x4d\xe8\xbf\xd3\xdd\x34\x55\xc4\xbf\x2f\x24\x19\xad\x86\x1e\x69\x2b\x6c\x3f\x00\xe8\x4b\xbb\x99\xcf\x17\x99\x00\x9d\x6c\x70\x57\xcc\x35\xee\x07\x87\x25\x8c\x0c\x8b\x9b\x38\x15\xcc\x05\x6f\xf8\x
 16\x78\x0b\x41\xfa\x23\x96\xc0\x79\xf8\xb7\xf0\x2b\x60\x7e\x98\xe3\x7b\xab\x80\x1f\x0d\xbf\xf6\x7e\x37\x06\xf1\x11\x42\x38\x2a\x70\xdf\xa4\xca\xf5\xf3\xf4\x7d\xca\x10\x0c\xd5\xe2\x90\xa0\x15\xde\xc2\x61\xa2\x88\xea\x32\x37\x97\x83\xd0\x4c\xad\xe2\xae\x9b\x53\xa2\xc2\x54\x0c\xbd\xe1\x50\x3b\x15\xd4\xb1\xa9\x41\x6e\x18\x2e\x30\x3f\x91\x03\x81\x86\x8c\x5c\x1f\x76\x51\x92\xf5\xb5\xb2\xc3\x16\x01\xef\xe3\x9e\xb1\x92\x0e\x0e\xcb\x20\x7f\x10\x29\x08\x6e\x15\x3d\x1e\x7c\x70\xf5\xb5\x3c\x56\x15\x3c\x59\xe6\xe7\x9e\x16\xcd\xfc\x8e\xfa\x12\x99\xbb\x07\xaa\xd7\x1c\xd0\xae\x93\x4c\xba\x16\x5d\x0c\xed\x1d\x02\x87\xcd\x38\x31\xc6\x10\x42\xe1\x46\x4e\xa3\xae\xb6\xda\xb6\xb0\x49\x55\x89\x57\xe6\xac\xe3\xbf\xb5\x5c\x59\x93\x0d\x21\x35\xdd\x57\x8c\x04\x15\x91\x05\x69\x4a\xdb\x5e\xcb\x4d\xa3\x5d\xa8\x7e\x95
 \x9e\x9d\x95\x61\xc9\x1c\xdd\x66\x0a\x76\x18\xbb\x59\x6a\xa5\xc0\xf2\xb8\x2f\xa9\x4c\xa8\xb3\x2b\xa3\x8a\xbf\x5c\xe8\x18\x3d\x7f\x0e\x2f\xe9\x06\xf9\xb6\xcc\x60\xcc\x38\x6c\x9a\x78\xa7\x7c\x61",
> 1037);
>         getsockopt(sock_dup, IPPROTO_IP, 0x81,  (void *)0x20000bf3ul,
> (socklen_t *)0x20003000ul);        
>         return 0;
> }
> 
> Best Regards,
> 
> Baozeng Ding

More likely a netfilter bug in net/bridge/netfilter/ebtables.c

^ permalink raw reply

* [PATCH v8 net-next] ravb: Add dma queue interrupt support
From: Yoshihiro Kaneko @ 2016-03-22 15:22 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Sergei Shtylyov, Simon Horman, Magnus Damm,
	linux-renesas-soc

From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>

This patch supports the following interrupts.

- One interrupt for multiple (timestamp, error, gPTP)
- One interrupt for emac
- Four interrupts for dma queue (best effort rx/tx, network control rx/tx)

This patch improve efficiency of the interrupt handler by adding the
interrupt handler corresponding to each interrupt source described
above. Additionally, it reduces the number of times of the access to
EthernetAVB IF.
Also this patch prevent this driver depends on the whim of a boot loader.

[ykaneko0929@gmail.com: define bit names of registers]
[ykaneko0929@gmail.com: add comment for gen3 only registers]
[ykaneko0929@gmail.com: fix coding style]
[ykaneko0929@gmail.com: update changelog]
[ykaneko0929@gmail.com: gen3: fix initialization of interrupts]
[ykaneko0929@gmail.com: gen3: fix clearing interrupts]
[ykaneko0929@gmail.com: gen3: add helper function for request_irq()]
[ykaneko0929@gmail.com: gen3: remove IRQF_SHARED flag for request_irq()]
[ykaneko0929@gmail.com: revert ravb_close() and ravb_ptp_stop()]
[ykaneko0929@gmail.com: avoid calling free_irq() to non-hooked interrupts]
[ykaneko0929@gmail.com: make NC/BE interrupt handler a function]
[ykaneko0929@gmail.com: make timestamp interrupt handler a function]
[ykaneko0929@gmail.com: timestamp interrupt is handled in multiple
 interrupt handler instead of dma queue interrupt handler]
Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
---

This patch is based on the master branch of David Miller's next networking
tree.

v8 [Yoshihiro Kaneko]
  - rebased

v7 [Yoshihiro Kaneko]
  drivers/net/ethernet/renesas/ravb_main.c:
  - read TIS in ravb_timestamp_interrupt()
  - avoid overwriting the 'result' by a return value of ravb_ptp_interrupt()
* As suggested by Sergei Shtylyov
  drivers/net/ethernet/renesas/ravb_main.c:
  - read RIS, RIC, TIS and TIC in ravb_queue_interrupt()
  - rename ravb_rx_tx_interrupt to ravb_dma_interrupt
  - shorten argument ravb_queue of ravb_dma_interrupt()

v6 [Yoshihiro Kaneko]
* As suggested by Sergei Shtylyov
  drivers/net/ethernet/renesas/ravb_main.c:
    - rename ravb_nc_be_interrupt to ravb_queue_interrupt, change the type
       of return value to 'bool', rename ravb_queue to 'q'
    - stop use of 'for' loop for queue interrupt in ravb_interrupt()
    - fix comment for ravb_multi_interrupt()
    - rename ravb_dmaq_interrupt to ravb_rx_tx_interrupt
    - move timestamp interrupt handler into ravb_multi_interrupt()
    - make timestamp interrupt handler a funtion
    - rename out_free_irq2 label to out_free_irq_nc_tx
    - remove IRQF_SHARED flag for request_irq()
  drivers/net/ethernet/renesas/ravb_ptp.c:
    - fix coding style

v5 [Yoshihiro Kaneko]
* As suggested by Sergei Shtylyov
  drivers/net/ethernet/renesas/ravb_main.c:
    - stop copying ravb_queue parameter in ravb_dmaq_interrupt()
    - clear TFUF instead of disabling
    - factored out NC/BE interrupt handler
    - rename hook_irq() in ravb_hook_irq()
    - add calling free_irq() for the EMAC IRQ
    - stop using a loop for free_irq() to avoid calling free_irq() for
      non-hooked interrupt handlers
    - add test for failure of devm_kasprintf in ravb_hook_irq()
    - update changelog

v4 [Yoshihiro Kaneko]
* compile tested only
* As suggested by Sergei Shtylyov
  drivers/net/ethernet/renesas/ravb.h:
    - make two lines of comment into one line.
    - remove unused definition of xxx_ALL.
  drivers/net/ethernet/renesas/ravb_main.c:
    - remove unrelated change (fix indentation).
    - output warning messages when napi_schedule_prep() fails in ravb_dmaq_
      interrupt() like ravb_interrupt().
    - change the function name from req_irq to hook_irq.
    - fix programming error in hook_irq().
    - do free_irq() for rx_irqs[] and tx_irqs[] for only gen3 in out_free_
      irq label in ravb_open().

v3 [Yoshihiro Kaneko]
* compile tested only
* As suggested by Sergei Shtylyov
  - update changelog
  drivers/net/ethernet/renesas/ravb.h:
    - add comments to the additional registers like CIE
  drivers/net/ethernet/renesas/ravb_main.c:
    - fix the initialization of the interrupt in ravb_dmac_init()
    - revert ravb_error_interrupt() because gen3 code is wrong
    - change the comment "Management" in ravb_multi_interrupt()
    - add a helper function for request_irq() in ravb_open()
    - revert ravb_close() because atomicity is not necessary here
  drivers/net/ethernet/renesas/ravb_ptp.c:
    - revert ravb_ptp_stop() because atomicity is not necessary here

v2 [Yoshihiro Kaneko]
* compile tested only
* As suggested by Sergei Shtylyov
  - add comment to CIE
  - remove comments from CIE bits
  - fix value of TIx_ALL
  - define each bits for CIE, GIE, GID, RIE0, RID0, RIE2, RID2, TIE, TID
  - reversed Christmas tree declaration ordered
  - rename _ravb_emac_interrupt() to ravb_emac_interrupt_unlocked()
  - remove unnecessary clearing of CIE
  - use a bit name corresponding to the target register, RIE0, RIE2, TIE,
    TID, RID2, GID, GIE

 drivers/net/ethernet/renesas/ravb.h      | 204 ++++++++++++++++++++++++
 drivers/net/ethernet/renesas/ravb_main.c | 266 +++++++++++++++++++++++++------
 drivers/net/ethernet/renesas/ravb_ptp.c  |  17 +-
 3 files changed, 439 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index b2160d1..5c16241 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -157,6 +157,7 @@ enum ravb_reg {
 	TIC	= 0x0378,
 	TIS	= 0x037C,
 	ISS	= 0x0380,
+	CIE	= 0x0384,	/* R-Car Gen3 only */
 	GCCR	= 0x0390,
 	GMTT	= 0x0394,
 	GPTC	= 0x0398,
@@ -170,6 +171,15 @@ enum ravb_reg {
 	GCT0	= 0x03B8,
 	GCT1	= 0x03BC,
 	GCT2	= 0x03C0,
+	GIE	= 0x03CC,	/* R-Car Gen3 only */
+	GID	= 0x03D0,	/* R-Car Gen3 only */
+	DIL	= 0x0440,	/* R-Car Gen3 only */
+	RIE0	= 0x0460,	/* R-Car Gen3 only */
+	RID0	= 0x0464,	/* R-Car Gen3 only */
+	RIE2	= 0x0470,	/* R-Car Gen3 only */
+	RID2	= 0x0474,	/* R-Car Gen3 only */
+	TIE	= 0x0478,	/* R-Car Gen3 only */
+	TID	= 0x047c,	/* R-Car Gen3 only */
 
 	/* E-MAC registers */
 	ECMR	= 0x0500,
@@ -556,6 +566,16 @@ enum ISS_BIT {
 	ISS_DPS15	= 0x80000000,
 };
 
+/* CIE (R-Car Gen3 only) */
+enum CIE_BIT {
+	CIE_CRIE	= 0x00000001,
+	CIE_CTIE	= 0x00000100,
+	CIE_RQFM	= 0x00010000,
+	CIE_CL0M	= 0x00020000,
+	CIE_RFWL	= 0x00040000,
+	CIE_RFFL	= 0x00080000,
+};
+
 /* GCCR */
 enum GCCR_BIT {
 	GCCR_TCR	= 0x00000003,
@@ -592,6 +612,188 @@ enum GIS_BIT {
 	GIS_PTMF	= 0x00000004,
 };
 
+/* GIE (R-Car Gen3 only) */
+enum GIE_BIT {
+	GIE_PTCS	= 0x00000001,
+	GIE_PTOS	= 0x00000002,
+	GIE_PTMS0	= 0x00000004,
+	GIE_PTMS1	= 0x00000008,
+	GIE_PTMS2	= 0x00000010,
+	GIE_PTMS3	= 0x00000020,
+	GIE_PTMS4	= 0x00000040,
+	GIE_PTMS5	= 0x00000080,
+	GIE_PTMS6	= 0x00000100,
+	GIE_PTMS7	= 0x00000200,
+	GIE_ATCS0	= 0x00010000,
+	GIE_ATCS1	= 0x00020000,
+	GIE_ATCS2	= 0x00040000,
+	GIE_ATCS3	= 0x00080000,
+	GIE_ATCS4	= 0x00100000,
+	GIE_ATCS5	= 0x00200000,
+	GIE_ATCS6	= 0x00400000,
+	GIE_ATCS7	= 0x00800000,
+	GIE_ATCS8	= 0x01000000,
+	GIE_ATCS9	= 0x02000000,
+	GIE_ATCS10	= 0x04000000,
+	GIE_ATCS11	= 0x08000000,
+	GIE_ATCS12	= 0x10000000,
+	GIE_ATCS13	= 0x20000000,
+	GIE_ATCS14	= 0x40000000,
+	GIE_ATCS15	= 0x80000000,
+};
+
+/* GID (R-Car Gen3 only) */
+enum GID_BIT {
+	GID_PTCD	= 0x00000001,
+	GID_PTOD	= 0x00000002,
+	GID_PTMD0	= 0x00000004,
+	GID_PTMD1	= 0x00000008,
+	GID_PTMD2	= 0x00000010,
+	GID_PTMD3	= 0x00000020,
+	GID_PTMD4	= 0x00000040,
+	GID_PTMD5	= 0x00000080,
+	GID_PTMD6	= 0x00000100,
+	GID_PTMD7	= 0x00000200,
+	GID_ATCD0	= 0x00010000,
+	GID_ATCD1	= 0x00020000,
+	GID_ATCD2	= 0x00040000,
+	GID_ATCD3	= 0x00080000,
+	GID_ATCD4	= 0x00100000,
+	GID_ATCD5	= 0x00200000,
+	GID_ATCD6	= 0x00400000,
+	GID_ATCD7	= 0x00800000,
+	GID_ATCD8	= 0x01000000,
+	GID_ATCD9	= 0x02000000,
+	GID_ATCD10	= 0x04000000,
+	GID_ATCD11	= 0x08000000,
+	GID_ATCD12	= 0x10000000,
+	GID_ATCD13	= 0x20000000,
+	GID_ATCD14	= 0x40000000,
+	GID_ATCD15	= 0x80000000,
+};
+
+/* RIE0 (R-Car Gen3 only) */
+enum RIE0_BIT {
+	RIE0_FRS0	= 0x00000001,
+	RIE0_FRS1	= 0x00000002,
+	RIE0_FRS2	= 0x00000004,
+	RIE0_FRS3	= 0x00000008,
+	RIE0_FRS4	= 0x00000010,
+	RIE0_FRS5	= 0x00000020,
+	RIE0_FRS6	= 0x00000040,
+	RIE0_FRS7	= 0x00000080,
+	RIE0_FRS8	= 0x00000100,
+	RIE0_FRS9	= 0x00000200,
+	RIE0_FRS10	= 0x00000400,
+	RIE0_FRS11	= 0x00000800,
+	RIE0_FRS12	= 0x00001000,
+	RIE0_FRS13	= 0x00002000,
+	RIE0_FRS14	= 0x00004000,
+	RIE0_FRS15	= 0x00008000,
+	RIE0_FRS16	= 0x00010000,
+	RIE0_FRS17	= 0x00020000,
+};
+
+/* RID0 (R-Car Gen3 only) */
+enum RID0_BIT {
+	RID0_FRD0	= 0x00000001,
+	RID0_FRD1	= 0x00000002,
+	RID0_FRD2	= 0x00000004,
+	RID0_FRD3	= 0x00000008,
+	RID0_FRD4	= 0x00000010,
+	RID0_FRD5	= 0x00000020,
+	RID0_FRD6	= 0x00000040,
+	RID0_FRD7	= 0x00000080,
+	RID0_FRD8	= 0x00000100,
+	RID0_FRD9	= 0x00000200,
+	RID0_FRD10	= 0x00000400,
+	RID0_FRD11	= 0x00000800,
+	RID0_FRD12	= 0x00001000,
+	RID0_FRD13	= 0x00002000,
+	RID0_FRD14	= 0x00004000,
+	RID0_FRD15	= 0x00008000,
+	RID0_FRD16	= 0x00010000,
+	RID0_FRD17	= 0x00020000,
+};
+
+/* RIE2 (R-Car Gen3 only) */
+enum RIE2_BIT {
+	RIE2_QFS0	= 0x00000001,
+	RIE2_QFS1	= 0x00000002,
+	RIE2_QFS2	= 0x00000004,
+	RIE2_QFS3	= 0x00000008,
+	RIE2_QFS4	= 0x00000010,
+	RIE2_QFS5	= 0x00000020,
+	RIE2_QFS6	= 0x00000040,
+	RIE2_QFS7	= 0x00000080,
+	RIE2_QFS8	= 0x00000100,
+	RIE2_QFS9	= 0x00000200,
+	RIE2_QFS10	= 0x00000400,
+	RIE2_QFS11	= 0x00000800,
+	RIE2_QFS12	= 0x00001000,
+	RIE2_QFS13	= 0x00002000,
+	RIE2_QFS14	= 0x00004000,
+	RIE2_QFS15	= 0x00008000,
+	RIE2_QFS16	= 0x00010000,
+	RIE2_QFS17	= 0x00020000,
+	RIE2_RFFS	= 0x80000000,
+};
+
+/* RID2 (R-Car Gen3 only) */
+enum RID2_BIT {
+	RID2_QFD0	= 0x00000001,
+	RID2_QFD1	= 0x00000002,
+	RID2_QFD2	= 0x00000004,
+	RID2_QFD3	= 0x00000008,
+	RID2_QFD4	= 0x00000010,
+	RID2_QFD5	= 0x00000020,
+	RID2_QFD6	= 0x00000040,
+	RID2_QFD7	= 0x00000080,
+	RID2_QFD8	= 0x00000100,
+	RID2_QFD9	= 0x00000200,
+	RID2_QFD10	= 0x00000400,
+	RID2_QFD11	= 0x00000800,
+	RID2_QFD12	= 0x00001000,
+	RID2_QFD13	= 0x00002000,
+	RID2_QFD14	= 0x00004000,
+	RID2_QFD15	= 0x00008000,
+	RID2_QFD16	= 0x00010000,
+	RID2_QFD17	= 0x00020000,
+	RID2_RFFD	= 0x80000000,
+};
+
+/* TIE (R-Car Gen3 only) */
+enum TIE_BIT {
+	TIE_FTS0	= 0x00000001,
+	TIE_FTS1	= 0x00000002,
+	TIE_FTS2	= 0x00000004,
+	TIE_FTS3	= 0x00000008,
+	TIE_TFUS	= 0x00000100,
+	TIE_TFWS	= 0x00000200,
+	TIE_MFUS	= 0x00000400,
+	TIE_MFWS	= 0x00000800,
+	TIE_TDPS0	= 0x00010000,
+	TIE_TDPS1	= 0x00020000,
+	TIE_TDPS2	= 0x00040000,
+	TIE_TDPS3	= 0x00080000,
+};
+
+/* TID (R-Car Gen3 only) */
+enum TID_BIT {
+	TID_FTD0	= 0x00000001,
+	TID_FTD1	= 0x00000002,
+	TID_FTD2	= 0x00000004,
+	TID_FTD3	= 0x00000008,
+	TID_TFUD	= 0x00000100,
+	TID_TFWD	= 0x00000200,
+	TID_MFUD	= 0x00000400,
+	TID_MFWD	= 0x00000800,
+	TID_TDPD0	= 0x00010000,
+	TID_TDPD1	= 0x00020000,
+	TID_TDPD2	= 0x00040000,
+	TID_TDPD3	= 0x00080000,
+};
+
 /* ECMR */
 enum ECMR_BIT {
 	ECMR_PRM	= 0x00000001,
@@ -817,6 +1019,8 @@ struct ravb_private {
 	int duplex;
 	int emac_irq;
 	enum ravb_chip_id chip_id;
+	int rx_irqs[NUM_RX_QUEUE];
+	int tx_irqs[NUM_TX_QUEUE];
 
 	unsigned no_avb_link:1;
 	unsigned avb_link_active_low:1;
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 4e1a7db..8fa8ffe 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -42,6 +42,16 @@
 		 NETIF_MSG_RX_ERR | \
 		 NETIF_MSG_TX_ERR)
 
+static const char *ravb_rx_irqs[NUM_RX_QUEUE] = {
+	"ch0", /* RAVB_BE */
+	"ch1", /* RAVB_NC */
+};
+
+static const char *ravb_tx_irqs[NUM_TX_QUEUE] = {
+	"ch18", /* RAVB_BE */
+	"ch19", /* RAVB_NC */
+};
+
 void ravb_modify(struct net_device *ndev, enum ravb_reg reg, u32 clear,
 		 u32 set)
 {
@@ -365,6 +375,7 @@ static void ravb_emac_init(struct net_device *ndev)
 /* Device init function for Ethernet AVB */
 static int ravb_dmac_init(struct net_device *ndev)
 {
+	struct ravb_private *priv = netdev_priv(ndev);
 	int error;
 
 	/* Set CONFIG mode */
@@ -401,6 +412,12 @@ static int ravb_dmac_init(struct net_device *ndev)
 	ravb_write(ndev, TCCR_TFEN, TCCR);
 
 	/* Interrupt init: */
+	if (priv->chip_id == RCAR_GEN3) {
+		/* Clear DIL.DPLx */
+		ravb_write(ndev, 0, DIL);
+		/* Set queue specific interrupt */
+		ravb_write(ndev, CIE_CRIE | CIE_CTIE | CIE_CL0M, CIE);
+	}
 	/* Frame receive */
 	ravb_write(ndev, RIC0_FRE0 | RIC0_FRE1, RIC0);
 	/* Disable FIFO full warning */
@@ -643,7 +660,7 @@ static int ravb_stop_dma(struct net_device *ndev)
 }
 
 /* E-MAC interrupt handler */
-static void ravb_emac_interrupt(struct net_device *ndev)
+static void ravb_emac_interrupt_unlocked(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
 	u32 ecsr, psr;
@@ -669,6 +686,18 @@ static void ravb_emac_interrupt(struct net_device *ndev)
 	}
 }
 
+static irqreturn_t ravb_emac_interrupt(int irq, void *dev_id)
+{
+	struct net_device *ndev = dev_id;
+	struct ravb_private *priv = netdev_priv(ndev);
+
+	spin_lock(&priv->lock);
+	ravb_emac_interrupt_unlocked(ndev);
+	mmiowb();
+	spin_unlock(&priv->lock);
+	return IRQ_HANDLED;
+}
+
 /* Error interrupt handler */
 static void ravb_error_interrupt(struct net_device *ndev)
 {
@@ -695,6 +724,50 @@ static void ravb_error_interrupt(struct net_device *ndev)
 	}
 }
 
+static bool ravb_queue_interrupt(struct net_device *ndev, int q)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	u32 ris0 = ravb_read(ndev, RIS0);
+	u32 ric0 = ravb_read(ndev, RIC0);
+	u32 tis  = ravb_read(ndev, TIS);
+	u32 tic  = ravb_read(ndev, TIC);
+
+	if (((ris0 & ric0) & BIT(q)) || ((tis  & tic)  & BIT(q))) {
+		if (napi_schedule_prep(&priv->napi[q])) {
+			/* Mask RX and TX interrupts */
+			if (priv->chip_id == RCAR_GEN2) {
+				ravb_write(ndev, ric0 & ~BIT(q), RIC0);
+				ravb_write(ndev, tic & ~BIT(q), TIC);
+			} else {
+				ravb_write(ndev, BIT(q), RID0);
+				ravb_write(ndev, BIT(q), TID);
+			}
+			__napi_schedule(&priv->napi[q]);
+		} else {
+			netdev_warn(ndev,
+				    "ignoring interrupt, rx status 0x%08x, rx mask 0x%08x,\n",
+				    ris0, ric0);
+			netdev_warn(ndev,
+				    "                    tx status 0x%08x, tx mask 0x%08x.\n",
+				    tis, tic);
+		}
+		return true;
+	}
+	return false;
+}
+
+static bool ravb_timestamp_interrupt(struct net_device *ndev)
+{
+	u32 tis = ravb_read(ndev, TIS);
+
+	if (tis & TIS_TFUF) {
+		ravb_write(ndev, ~TIS_TFUF, TIS);
+		ravb_get_tx_tstamp(ndev);
+		return true;
+	}
+	return false;
+}
+
 static irqreturn_t ravb_interrupt(int irq, void *dev_id)
 {
 	struct net_device *ndev = dev_id;
@@ -708,46 +781,22 @@ static irqreturn_t ravb_interrupt(int irq, void *dev_id)
 
 	/* Received and transmitted interrupts */
 	if (iss & (ISS_FRS | ISS_FTS | ISS_TFUS)) {
-		u32 ris0 = ravb_read(ndev, RIS0);
-		u32 ric0 = ravb_read(ndev, RIC0);
-		u32 tis  = ravb_read(ndev, TIS);
-		u32 tic  = ravb_read(ndev, TIC);
 		int q;
 
 		/* Timestamp updated */
-		if (tis & TIS_TFUF) {
-			ravb_write(ndev, ~TIS_TFUF, TIS);
-			ravb_get_tx_tstamp(ndev);
+		if (ravb_timestamp_interrupt(ndev))
 			result = IRQ_HANDLED;
-		}
 
 		/* Network control and best effort queue RX/TX */
 		for (q = RAVB_NC; q >= RAVB_BE; q--) {
-			if (((ris0 & ric0) & BIT(q)) ||
-			    ((tis  & tic)  & BIT(q))) {
-				if (napi_schedule_prep(&priv->napi[q])) {
-					/* Mask RX and TX interrupts */
-					ric0 &= ~BIT(q);
-					tic &= ~BIT(q);
-					ravb_write(ndev, ric0, RIC0);
-					ravb_write(ndev, tic, TIC);
-					__napi_schedule(&priv->napi[q]);
-				} else {
-					netdev_warn(ndev,
-						    "ignoring interrupt, rx status 0x%08x, rx mask 0x%08x,\n",
-						    ris0, ric0);
-					netdev_warn(ndev,
-						    "                    tx status 0x%08x, tx mask 0x%08x.\n",
-						    tis, tic);
-				}
+			if (ravb_queue_interrupt(ndev, q))
 				result = IRQ_HANDLED;
-			}
 		}
 	}
 
 	/* E-MAC status summary */
 	if (iss & ISS_MS) {
-		ravb_emac_interrupt(ndev);
+		ravb_emac_interrupt_unlocked(ndev);
 		result = IRQ_HANDLED;
 	}
 
@@ -757,6 +806,7 @@ static irqreturn_t ravb_interrupt(int irq, void *dev_id)
 		result = IRQ_HANDLED;
 	}
 
+	/* gPTP interrupt status summary */
 	if ((iss & ISS_CGIS) && ravb_ptp_interrupt(ndev) == IRQ_HANDLED)
 		result = IRQ_HANDLED;
 
@@ -765,6 +815,64 @@ static irqreturn_t ravb_interrupt(int irq, void *dev_id)
 	return result;
 }
 
+/* Timestamp/Error/gPTP interrupt handler */
+static irqreturn_t ravb_multi_interrupt(int irq, void *dev_id)
+{
+	struct net_device *ndev = dev_id;
+	struct ravb_private *priv = netdev_priv(ndev);
+	irqreturn_t result = IRQ_NONE;
+	u32 iss;
+
+	spin_lock(&priv->lock);
+	/* Get interrupt status */
+	iss = ravb_read(ndev, ISS);
+
+	/* Timestamp updated */
+	if ((iss & ISS_TFUS) && ravb_timestamp_interrupt(ndev))
+		result = IRQ_HANDLED;
+
+	/* Error status summary */
+	if (iss & ISS_ES) {
+		ravb_error_interrupt(ndev);
+		result = IRQ_HANDLED;
+	}
+
+	/* gPTP interrupt status summary */
+	if ((iss & ISS_CGIS) && ravb_ptp_interrupt(ndev) == IRQ_HANDLED)
+		result = IRQ_HANDLED;
+
+	mmiowb();
+	spin_unlock(&priv->lock);
+	return result;
+}
+
+static irqreturn_t ravb_dma_interrupt(int irq, void *dev_id, int q)
+{
+	struct net_device *ndev = dev_id;
+	struct ravb_private *priv = netdev_priv(ndev);
+	irqreturn_t result = IRQ_NONE;
+
+	spin_lock(&priv->lock);
+
+	/* Network control/Best effort queue RX/TX */
+	if (ravb_queue_interrupt(ndev, q))
+		result = IRQ_HANDLED;
+
+	mmiowb();
+	spin_unlock(&priv->lock);
+	return result;
+}
+
+static irqreturn_t ravb_be_interrupt(int irq, void *dev_id)
+{
+	return ravb_dma_interrupt(irq, dev_id, RAVB_BE);
+}
+
+static irqreturn_t ravb_nc_interrupt(int irq, void *dev_id)
+{
+	return ravb_dma_interrupt(irq, dev_id, RAVB_NC);
+}
+
 static int ravb_poll(struct napi_struct *napi, int budget)
 {
 	struct net_device *ndev = napi->dev;
@@ -804,8 +912,13 @@ static int ravb_poll(struct napi_struct *napi, int budget)
 
 	/* Re-enable RX/TX interrupts */
 	spin_lock_irqsave(&priv->lock, flags);
-	ravb_modify(ndev, RIC0, mask, mask);
-	ravb_modify(ndev, TIC,  mask, mask);
+	if (priv->chip_id == RCAR_GEN2) {
+		ravb_modify(ndev, RIC0, mask, mask);
+		ravb_modify(ndev, TIC,  mask, mask);
+	} else {
+		ravb_write(ndev, mask, RIE0);
+		ravb_write(ndev, mask, TIE);
+	}
 	mmiowb();
 	spin_unlock_irqrestore(&priv->lock, flags);
 
@@ -1208,35 +1321,72 @@ static const struct ethtool_ops ravb_ethtool_ops = {
 	.get_ts_info		= ravb_get_ts_info,
 };
 
+static inline int ravb_hook_irq(unsigned int irq, irq_handler_t handler,
+				struct net_device *ndev, struct device *dev,
+				const char *ch)
+{
+	char *name;
+	int error;
+
+	name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s", ndev->name, ch);
+	if (!name)
+		return -ENOMEM;
+	error = request_irq(irq, handler, 0, name, ndev);
+	if (error)
+		netdev_err(ndev, "cannot request IRQ %s\n", name);
+
+	return error;
+}
+
 /* Network device open function for Ethernet AVB */
 static int ravb_open(struct net_device *ndev)
 {
 	struct ravb_private *priv = netdev_priv(ndev);
+	struct platform_device *pdev = priv->pdev;
+	struct device *dev = &pdev->dev;
 	int error;
 
 	napi_enable(&priv->napi[RAVB_BE]);
 	napi_enable(&priv->napi[RAVB_NC]);
 
-	error = request_irq(ndev->irq, ravb_interrupt, IRQF_SHARED, ndev->name,
-			    ndev);
-	if (error) {
-		netdev_err(ndev, "cannot request IRQ\n");
-		goto out_napi_off;
-	}
-
-	if (priv->chip_id == RCAR_GEN3) {
-		error = request_irq(priv->emac_irq, ravb_interrupt,
-				    IRQF_SHARED, ndev->name, ndev);
+	if (priv->chip_id == RCAR_GEN2) {
+		error = request_irq(ndev->irq, ravb_interrupt, IRQF_SHARED,
+				    ndev->name, ndev);
 		if (error) {
 			netdev_err(ndev, "cannot request IRQ\n");
-			goto out_free_irq;
+			goto out_napi_off;
 		}
+	} else {
+		error = ravb_hook_irq(ndev->irq, ravb_multi_interrupt, ndev,
+				      dev, "ch22:multi");
+		if (error)
+			goto out_napi_off;
+		error = ravb_hook_irq(priv->emac_irq, ravb_emac_interrupt, ndev,
+				      dev, "ch24:emac");
+		if (error)
+			goto out_free_irq;
+		error = ravb_hook_irq(priv->rx_irqs[RAVB_BE], ravb_be_interrupt,
+				      ndev, dev, "ch0:rx_be");
+		if (error)
+			goto out_free_irq_emac;
+		error = ravb_hook_irq(priv->tx_irqs[RAVB_BE], ravb_be_interrupt,
+				      ndev, dev, "ch18:tx_be");
+		if (error)
+			goto out_free_irq_be_rx;
+		error = ravb_hook_irq(priv->rx_irqs[RAVB_NC], ravb_nc_interrupt,
+				      ndev, dev, "ch1:rx_nc");
+		if (error)
+			goto out_free_irq_be_tx;
+		error = ravb_hook_irq(priv->tx_irqs[RAVB_NC], ravb_nc_interrupt,
+				      ndev, dev, "ch19:tx_nc");
+		if (error)
+			goto out_free_irq_nc_rx;
 	}
 
 	/* Device init */
 	error = ravb_dmac_init(ndev);
 	if (error)
-		goto out_free_irq2;
+		goto out_free_irq_nc_tx;
 	ravb_emac_init(ndev);
 
 	/* Initialise PTP Clock driver */
@@ -1256,9 +1406,18 @@ out_ptp_stop:
 	/* Stop PTP Clock driver */
 	if (priv->chip_id == RCAR_GEN2)
 		ravb_ptp_stop(ndev);
-out_free_irq2:
-	if (priv->chip_id == RCAR_GEN3)
-		free_irq(priv->emac_irq, ndev);
+out_free_irq_nc_tx:
+	if (priv->chip_id == RCAR_GEN2)
+		goto out_free_irq;
+	free_irq(priv->tx_irqs[RAVB_NC], ndev);
+out_free_irq_nc_rx:
+	free_irq(priv->rx_irqs[RAVB_NC], ndev);
+out_free_irq_be_tx:
+	free_irq(priv->tx_irqs[RAVB_BE], ndev);
+out_free_irq_be_rx:
+	free_irq(priv->rx_irqs[RAVB_BE], ndev);
+out_free_irq_emac:
+	free_irq(priv->emac_irq, ndev);
 out_free_irq:
 	free_irq(ndev->irq, ndev);
 out_napi_off:
@@ -1713,6 +1872,7 @@ static int ravb_probe(struct platform_device *pdev)
 	struct net_device *ndev;
 	int error, irq, q;
 	struct resource *res;
+	int i;
 
 	if (!np) {
 		dev_err(&pdev->dev,
@@ -1782,6 +1942,22 @@ static int ravb_probe(struct platform_device *pdev)
 			goto out_release;
 		}
 		priv->emac_irq = irq;
+		for (i = 0; i < NUM_RX_QUEUE; i++) {
+			irq = platform_get_irq_byname(pdev, ravb_rx_irqs[i]);
+			if (irq < 0) {
+				error = irq;
+				goto out_release;
+			}
+			priv->rx_irqs[i] = irq;
+		}
+		for (i = 0; i < NUM_TX_QUEUE; i++) {
+			irq = platform_get_irq_byname(pdev, ravb_tx_irqs[i]);
+			if (irq < 0) {
+				error = irq;
+				goto out_release;
+			}
+			priv->tx_irqs[i] = irq;
+		}
 	}
 
 	priv->chip_id = chip_id;
diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c b/drivers/net/ethernet/renesas/ravb_ptp.c
index 57992cc..f1b2cbb 100644
--- a/drivers/net/ethernet/renesas/ravb_ptp.c
+++ b/drivers/net/ethernet/renesas/ravb_ptp.c
@@ -194,7 +194,12 @@ static int ravb_ptp_extts(struct ptp_clock_info *ptp,
 	priv->ptp.extts[req->index] = on;
 
 	spin_lock_irqsave(&priv->lock, flags);
-	ravb_modify(ndev, GIC, GIC_PTCE, on ? GIC_PTCE : 0);
+	if (priv->chip_id == RCAR_GEN2)
+		ravb_modify(ndev, GIC, GIC_PTCE, on ? GIC_PTCE : 0);
+	else if (on)
+		ravb_write(ndev, GIE_PTCS, GIE);
+	else
+		ravb_write(ndev, GID_PTCD, GID);
 	mmiowb();
 	spin_unlock_irqrestore(&priv->lock, flags);
 
@@ -241,7 +246,10 @@ static int ravb_ptp_perout(struct ptp_clock_info *ptp,
 		error = ravb_ptp_update_compare(priv, (u32)start_ns);
 		if (!error) {
 			/* Unmask interrupt */
-			ravb_modify(ndev, GIC, GIC_PTME, GIC_PTME);
+			if (priv->chip_id == RCAR_GEN2)
+				ravb_modify(ndev, GIC, GIC_PTME, GIC_PTME);
+			else
+				ravb_write(ndev, GIE_PTMS0, GIE);
 		}
 	} else	{
 		spin_lock_irqsave(&priv->lock, flags);
@@ -250,7 +258,10 @@ static int ravb_ptp_perout(struct ptp_clock_info *ptp,
 		perout->period = 0;
 
 		/* Mask interrupt */
-		ravb_modify(ndev, GIC, GIC_PTME, 0);
+		if (priv->chip_id == RCAR_GEN2)
+			ravb_modify(ndev, GIC, GIC_PTME, 0);
+		else
+			ravb_write(ndev, GID_PTMD0, GID);
 	}
 	mmiowb();
 	spin_unlock_irqrestore(&priv->lock, flags);
-- 
1.9.1

^ permalink raw reply related

* Re: net/sctp: stack-out-of-bounds in sctp_getsockopt
From: Eric Dumazet @ 2016-03-22 15:27 UTC (permalink / raw)
  To: Baozeng Ding, netfilter-devel
  Cc: vyasevich, nhorman, davem, linux-sctp, netdev, linux-kernel
In-Reply-To: <1458660088.10868.19.camel@edumazet-glaptop3.roam.corp.google.com>

On Tue, 2016-03-22 at 08:21 -0700, Eric Dumazet wrote:
> On Tue, 2016-03-22 at 23:08 +0800, Baozeng Ding wrote:
> > Hi all,
> > 
> > The following program triggers an out-of-bounds bug in
> > sctp_getsockopt. The kernel version is 4.5 (on Mar 16
> > commit 09fd671ccb2475436bd5f597f751ca4a7d177aea). 
> > 
> > ==================================================================
> > BUG: KASAN: stack-out-of-bounds in string+0x1ef/0x200 at addr
> > ffff88003ae679e0
> > Read of size 1 by task syz-executor/19753
> > page:ffffea0000eb99c0 count:0 mapcount:0 mapping:          (null)
> > index:0x0
> > flags: 0x1fffc0000000000()
> > page dumped because: kasan: bad access detected
> > CPU: 3 PID: 19753 Comm: syz-executor Not tainted 4.5.0+ #8
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
> >  0000000000000003 ffff88003ae67578 ffffffff82945051 ffff88003ae67608
> >  ffff88003ae679e0 0000000000000096 dffffc0000000000 ffff88003ae675f8
> >  ffffffff81709f88 000000000000030d 0000000000000000 0000000000000286
> > Call Trace:
> >  [<     inline     >] __dump_stack lib/dump_stack.c:15
> >  [<ffffffff82945051>] dump_stack+0xb3/0x112 lib/dump_stack.c:51
> >  [<     inline     >] print_address_description mm/kasan/report.c:150
> >  [<ffffffff81709f88>] kasan_report_error+0x4f8/0x530 mm/kasan/report.c:236
> >  [<ffffffff8140785b>] ? __lock_acquire+0x15fb/0x5dd0 kernel/locking/lockdep.c:3226
> >  [<     inline     >] kasan_report mm/kasan/report.c:259
> >  [<ffffffff81709ffe>] __asan_report_load1_noabort+0x3e/0x40 mm/kasan/report.c:277
> >  [<ffffffff8296613f>] ? string+0x1ef/0x200 lib/vsprintf.c:591
> >  [<ffffffff8296613f>] string+0x1ef/0x200 lib/vsprintf.c:591
> >  [<ffffffff8296f103>] vsnprintf+0xb83/0x1900 lib/vsprintf.c:2049
> >  [<ffffffff8296e580>] ? pointer+0xab0/0xab0 lib/vsprintf.c:1584
> >  [<ffffffff813456f2>] __request_module+0x132/0x6b0 kernel/kmod.c:146
> >  [<ffffffff814056b0>] ? mark_held_locks+0xd0/0x130 kernel/locking/lockdep.c:2552
> >  [<ffffffff813455c0>] ? call_usermodehelper_setup+0x2b0/0x2b0 kernel/kmod.c:530
> >  [<ffffffff85da47b0>] ? mutex_lock_interruptible_nested+0x980/0x980
> >  [<ffffffff8168fed4>] ? __might_fault+0xe4/0x1d0 mm/memory.c:3833
> >  [<ffffffff8538f74c>] find_inlist_lock.constprop.17+0x10c/0x210 net/bridge/netfilter/ebtables.c:347
> >  [<     inline     >] find_table_lock net/bridge/netfilter/ebtables.c:356
> >  [<ffffffff853904ab>] do_ebt_get_ctl+0x13b/0x540 net/bridge/netfilter/ebtables.c:1524
> >  [<ffffffff85390370>] ? copy_everything_to_user+0x600/0x600 net/bridge/netfilter/ebtables.c:1455
> >  [<     inline     >] ? __mutex_unlock_common_slowpath kernel/locking/mutex.c:751
> >  [<ffffffff85da6799>] ? __mutex_unlock_slowpath+0x239/0x3f0 kernel/locking/mutex.c:762
> >  [<ffffffff85da6959>] ? mutex_unlock+0x9/0x10 kernel/locking/mutex.c:437
> >  [<ffffffff84dea126>] ? nf_sockopt_find+0x1a6/0x220 net/netfilter/nf_sockopt.c:87
> >  [<     inline     >] nf_sockopt net/netfilter/nf_sockopt.c:103
> >  [<ffffffff84dea20d>] nf_getsockopt+0x6d/0xc0 net/netfilter/nf_sockopt.c:121
> >  [<ffffffff84fadf05>] ip_getsockopt+0x135/0x190 net/ipv4/ip_sockglue.c:1523
> >  [<ffffffff84faddd0>] ? do_ip_getsockopt+0x1520/0x1520 net/ipv4/ip_sockglue.c:1353
> >  [<     inline     >] ? wake_up_process kernel/sched/core.c:2024
> >  [<ffffffff8138bcc2>] ? wake_up_q+0x82/0xe0 kernel/sched/core.c:416
> >  [<     inline     >] ? atomic_dec_and_test /arch/x86/include/asm/atomic.h:117
> >  [<     inline     >] ? mmdrop include/linux/sched.h:2611
> >  [<ffffffff814a3310>] ? drop_futex_key_refs.isra.13+0x70/0xe0 kernel/futex.c:444
> >  [<ffffffff8583a4dd>] sctp_getsockopt+0x18d/0x3f40 net/sctp/socket.c:5964
> >  [<ffffffff8140785b>] ? __lock_acquire+0x15fb/0x5dd0 kernel/locking/lockdep.c:3226
> >  [<ffffffff8583a350>] ? sctp_do_peeloff+0x2b0/0x2b0 net/sctp/socket.c:4434
> >  [<ffffffff81406260>] ? debug_check_no_locks_freed+0x290/0x290 kernel/locking/lockdep.c:4104
> >  [<     inline     >] ? rcu_read_unlock include/linux/rcupdate.h:922
> >  [<ffffffff817b398c>] ? __fget+0x20c/0x3b0 fs/file.c:712
> >  [<     inline     >] ? rcu_lock_release include/linux/rcupdate.h:491
> >  [<     inline     >] ? rcu_read_unlock include/linux/rcupdate.h:926
> >  [<ffffffff817b39b5>] ? __fget+0x235/0x3b0 fs/file.c:712
> >  [<ffffffff817b37c7>] ? __fget+0x47/0x3b0 fs/file.c:696
> >  [<ffffffff817b3c11>] ? __fget_light+0xa1/0x1f0 fs/file.c:759
> >  [<ffffffff84c3a695>] sock_common_getsockopt+0x95/0xd0 net/core/sock.c:2579
> >  [<     inline     >] SYSC_getsockopt net/socket.c:1783
> >  [<ffffffff84c37e12>] SyS_getsockopt+0x142/0x230 net/socket.c:1765
> >  [<ffffffff84c37cd0>] ? SyS_setsockopt+0x240/0x240 net/socket.c:1752
> >  [<ffffffff85dab922>] ? entry_SYSCALL_64_fastpath+0x5/0xc1 arch/x86/entry/entry_64.S:191
> >  [<ffffffff81003017>] ? trace_hardirqs_on_thunk+0x17/0x19 arch/x86/entry/thunk_64.S:39
> >  [<ffffffff85dab940>] entry_SYSCALL_64_fastpath+0x23/0xc1 arch/x86/entry/entry_64.S:207
> > Memory state around the buggy address:
> >  ffff88003ae67880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >  ffff88003ae67900: 00 f1 f1 f1 f1 04 f4 f4 f4 f2 f2 f2 f2 00 00 00
> > >ffff88003ae67980: 00 00 00 00 00 00 00 00 00 00 00 00 f4 f3 f3 f3
> >                                                        ^
> >  ffff88003ae67a00: f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >  ffff88003ae67a80: f1 f1 f1 f1 04 f4 f4 f4 f3 f3 f3 f3 00 00 00 00
> > ==================================================================
> > 
> > #include <unistd.h>
> > #include <sys/syscall.h>
> > #include <netinet/in.h>
> > #include <string.h>
> > #include <stdint.h>
> > #include <sys/mman.h>
> > #include <sys/socket.h>
> > 
> > int main()
> > {
> >         int sock = 0;
> >         int sock_dup = 0;
> >         mmap((void *)0x20000000ul, 0x5000ul, PROT_READ|PROT_WRITE,
> > MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0);
> >         sock = socket(AF_INET, SOCK_STREAM, IPPROTO_SCTP);
> >         sock_dup = dup(sock);
> >         memcpy((void*)0x20000bf3,"\xac\x71\x93\x68\x02\xb3\xd1\x86\x52\xf1\xf0\x18\x09\x56\xc6\x98\x6f\x8e\x74\xb7\x17\xd4\x3a\x64\x51\x68\x13\x2d\x25\xba\x6d\x3f\x74\x68\x84\x89\x04\xd1\xa6\xe2\x7d\xaf\xfa\xd9\xce\x52\xbe\x6f\xb6\xe3\xff\x92\x35\xa1\x88\x4a\x68\x27\xaa\x25\xf8\xc1\xd5\x3b\xe5\x69\x11\x4f\x75\x4c\xe9\xff\x8b\x86\x53\x20\xb7\x10\xa2\x62\xcc\xc3\x06\x85\xde\x3e\x1c\x5a\x62\x3a\x2d\x0d\x0b\x0c\xb2\xac\x75\x42\x4d\x82\x3f\x7b\xf7\x28\xea\x2d\xff\x42\xa8\xdf\xb3\x49\x1a\xfd\xae\x2c\xd4\x35\x8e\x96\xb3\xe1\x0a\x92\x56\xb7\xde\xe8\x9e\xc3\x9e\x88\x79\xc4\x71\x46\x27\xf4\x9e\x85\xf4\x8f\x1f\x9a\xe5\x7e\x02\x09\x34\x80\x1e\x87\xa8\x9a\xce\xac\xfb\x43\x07\xdf\x15\xe8\x71\x9a\xa3\x80\x18\x1b\x15\xbd\x57\xb6\xc1\x73\x6e\xb1\x28\x3a\x01\xd5\x8e\x15\x85\xbd\x52\xdf\xfa\x64\xaa\x13\x
 0e\x2f\x64\x05\x11\xce\x79\x8b\xa8\x02\x29\x7f\x72\x0f\x37\x89\xb4\x54\x0b\x09\x02\x75\xc2\x8e\xd7\xcd\x7e\xfb\x4f\x72\xf1\x47\xea\xa2\x2a\xc3\xc4\xe9\x70\xfe\xa5\x80\x88\x21\x33\xcf\x13\x66\x98\x23\x10\x5c\xa4\xbd\xee\xc0\xb4\xdd\xfb\xff\xf2\x38\xab\xca\x36\x62\x35\x84\xe4\x73\x5c\xc7\x3e\x72\x2e\x17\x43\x6f\x85\x45\x4f\x82\x62\x0d\x77\xae\xcb\xe1\x8f\xe8\xf0\x84\x3e\x62\x8b\x70\x2b\x55\xb5\xa7\x13\xcf\xa1\x78\x77\x82\xe2\xb7\x1c\x65\x7f\xb5\x79\x73\x01\x07\xd1\x9f\x45\x6a\xbb\x3d\xbf\xc8\x71\x5b\x9f\x30\xc7\xb9\xb8\x53\x9f\xe1\xba\xb6\x78\x9e\x05\x75\xa3\x55\xb1\x26\x96\xa9\xb2\x82\xce\x81\x5c\x8a\x18\xb3\x4b\x0c\x18\x8c\xf2\x7c\x09\xde\xcb\xcf\x78\x22\x58\xf6\x15\xf6\xf7\x48\xda\x08\x75\xd4\xc1\x20\xc3\x18\x2e\x89\xe8\x5b\x48\xd9\xbc\x1f\xbb\xed\x31\xaf\x12\x4d\xcd\x46\x60\xa0\xef\x0e
 \x2e\x21\x1d\x2b\x68\x75\xb9\x42\x5e\xd7\xae\x35\x46\xe9\x06\x63\x1d\x3c\xd6\x9c\x14\x3b\x09\x29\x49\x70\xb9\xe1\xe0\x09\x45\x41\x62\x0c\xff\x5a\x77\xbe\x31\xa6\x03\x94\x92\xde\x41\x99\xfa\x68\x99\x74\xbb\x0a\x3d\xac\x9c\x7e\x00\x6b\xcd\xc1\x83\xa7\xc5\x63\xdd\x10\xea\x59\x27\xdc\x02\x98\xd6\x43\x20\x24\x4e\xc0\xdc\xa2\x98\xdf\x3e\xaf\x61\x35\xa0\x95\x3f\x9a\xaa\x7d\xe9\xe9\x0d\xe5\x97\x66\x1a\x9f\xbf\x56\xc8\x37\x84\x18\x2b\xd2\xcd\xd6\xb3\x19\xd8\x4a\x30\x6e\xcb\x99\x1c\xe9\x0f\xdb\xca\x30\xe1\xe2\x90\xba\xb9\x61\x00\xbf\xeb\xad\x6a\xc8\x52\xea\x1a\x92\x05\x0c\x3b\x78\x82\x01\xac\xfd\x88\x6c\xca\xe2\xfb\xe7\x0f\xcc\x75\x9c\x98\x12\x26\xcf\xa6\x80\x02\x35\xdf\x6e\xe1\x11\x1d\xa7\x30\x17\x38\x41\xd9\x81\x55\x1a\x1e\xd1\xfe\x60\xbf\xef\x09\x25\xc0\xdb\x9f\xc4\xc6\x54\x1a\x85\x36\x85\x05\x
 b3\x9f\x2c\xc5\xcd\x12\x51\xef\xbe\x10\x79\xbf\x11\x00\x47\x0d\x9c\x14\x43\x1a\x46\xea\xd1\x34\x2e\x10\x6b\xa4\x3c\x25\x21\xe3\xb9\x15\x78\x6c\x40\x87\x90\xf7\x93\x5a\x66\x5f\x0a\x76\xff\xc2\xe2\x14\x35\x88\x47\xa1\x33\x5b\x8f\x3d\xc5\x89\xb7\xf9\x8a\x40\xf0\x1e\xc9\x30\xcd\xd8\x96\x41\x78\x58\x97\x49\xc8\x50\x61\x36\x8f\x7e\x44\x41\xc0\x84\xbb\x35\xf0\x63\xa9\xc2\x2a\xbd\xcc\x4b\xab\x8b\x16\x33\xc0\x66\xbf\x47\x62\x9b\xc4\x47\x2d\x68\x83\xca\xe3\x52\x79\xd7\xe0\x61\x80\x15\xf1\x90\x83\xa2\xbb\x4c\xe5\x8b\x50\xc8\x1b\x68\x7b\xee\x57\xdc\x54\xfa\x90\xf1\xf5\xec\x7d\x93\xe0\x80\x74\x06\xbe\xac\xc8\x85\x4d\xe8\xbf\xd3\xdd\x34\x55\xc4\xbf\x2f\x24\x19\xad\x86\x1e\x69\x2b\x6c\x3f\x00\xe8\x4b\xbb\x99\xcf\x17\x99\x00\x9d\x6c\x70\x57\xcc\x35\xee\x07\x87\x25\x8c\x0c\x8b\x9b\x38\x15\xcc\x05\x6f\xf8
 \x16\x78\x0b\x41\xfa\x23\x96\xc0\x79\xf8\xb7\xf0\x2b\x60\x7e\x98\xe3\x7b\xab\x80\x1f\x0d\xbf\xf6\x7e\x37\x06\xf1\x11\x42\x38\x2a\x70\xdf\xa4\xca\xf5\xf3\xf4\x7d\xca\x10\x0c\xd5\xe2\x90\xa0\x15\xde\xc2\x61\xa2\x88\xea\x32\x37\x97\x83\xd0\x4c\xad\xe2\xae\x9b\x53\xa2\xc2\x54\x0c\xbd\xe1\x50\x3b\x15\xd4\xb1\xa9\x41\x6e\x18\x2e\x30\x3f\x91\x03\x81\x86\x8c\x5c\x1f\x76\x51\x92\xf5\xb5\xb2\xc3\x16\x01\xef\xe3\x9e\xb1\x92\x0e\x0e\xcb\x20\x7f\x10\x29\x08\x6e\x15\x3d\x1e\x7c\x70\xf5\xb5\x3c\x56\x15\x3c\x59\xe6\xe7\x9e\x16\xcd\xfc\x8e\xfa\x12\x99\xbb\x07\xaa\xd7\x1c\xd0\xae\x93\x4c\xba\x16\x5d\x0c\xed\x1d\x02\x87\xcd\x38\x31\xc6\x10\x42\xe1\x46\x4e\xa3\xae\xb6\xda\xb6\xb0\x49\x55\x89\x57\xe6\xac\xe3\xbf\xb5\x5c\x59\x93\x0d\x21\x35\xdd\x57\x8c\x04\x15\x91\x05\x69\x4a\xdb\x5e\xcb\x4d\xa3\x5d\xa8\x7e\x
 95\x9e\x9d\x95\x61\xc9\x1c\xdd\x66\x0a\x76\x18\xbb\x59\x6a\xa5\xc0\xf2\xb8\x2f\xa9\x4c\xa8\xb3\x2b\xa3\x8a\xbf\x5c\xe8\x18\x3d\x7f\x0e\x2f\xe9\x06\xf9\xb6\xcc\x60\xcc\x38\x6c\x9a\x78\xa7\x7c\x61",
> > 1037);
> >         getsockopt(sock_dup, IPPROTO_IP, 0x81,  (void *)0x20000bf3ul,
> > (socklen_t *)0x20003000ul);        
> >         return 0;
> > }
> > 
> > Best Regards,
> > 
> > Baozeng Ding
> 
> More likely a netfilter bug in net/bridge/netfilter/ebtables.c
> 

Untested patch would be :

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 67b2e27999aa..fceb7354d169 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -346,7 +346,7 @@ find_inlist_lock(struct list_head *head, const char *name, const char *prefix,
 {
 	return try_then_request_module(
 			find_inlist_lock_noload(head, name, error, mutex),
-			"%s%s", prefix, name);
+			"%.*s%s", EBT_TABLE_MAXNAMELEN, prefix, name);
 }
 
 static inline struct ebt_table *




^ permalink raw reply related

* Re: net/sctp: stack-out-of-bounds in sctp_getsockopt
From: Marcelo Ricardo Leitner @ 2016-03-22 15:28 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Baozeng Ding, vyasevich, nhorman, davem, linux-sctp, netdev,
	linux-kernel
In-Reply-To: <1458660088.10868.19.camel@edumazet-glaptop3.roam.corp.google.com>

On Tue, Mar 22, 2016 at 08:21:28AM -0700, Eric Dumazet wrote:
> On Tue, 2016-03-22 at 23:08 +0800, Baozeng Ding wrote:
> > Hi all,
> > 
> > The following program triggers an out-of-bounds bug in
> > sctp_getsockopt. The kernel version is 4.5 (on Mar 16
> > commit 09fd671ccb2475436bd5f597f751ca4a7d177aea). 
> > 
> > ==================================================================
> > BUG: KASAN: stack-out-of-bounds in string+0x1ef/0x200 at addr
> > ffff88003ae679e0
> > Read of size 1 by task syz-executor/19753
> > page:ffffea0000eb99c0 count:0 mapcount:0 mapping:          (null)
> > index:0x0
> > flags: 0x1fffc0000000000()
> > page dumped because: kasan: bad access detected
> > CPU: 3 PID: 19753 Comm: syz-executor Not tainted 4.5.0+ #8
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
> >  0000000000000003 ffff88003ae67578 ffffffff82945051 ffff88003ae67608
> >  ffff88003ae679e0 0000000000000096 dffffc0000000000 ffff88003ae675f8
> >  ffffffff81709f88 000000000000030d 0000000000000000 0000000000000286
> > Call Trace:
> >  [<     inline     >] __dump_stack lib/dump_stack.c:15
> >  [<ffffffff82945051>] dump_stack+0xb3/0x112 lib/dump_stack.c:51
> >  [<     inline     >] print_address_description mm/kasan/report.c:150
> >  [<ffffffff81709f88>] kasan_report_error+0x4f8/0x530 mm/kasan/report.c:236
> >  [<ffffffff8140785b>] ? __lock_acquire+0x15fb/0x5dd0 kernel/locking/lockdep.c:3226
> >  [<     inline     >] kasan_report mm/kasan/report.c:259
> >  [<ffffffff81709ffe>] __asan_report_load1_noabort+0x3e/0x40 mm/kasan/report.c:277
> >  [<ffffffff8296613f>] ? string+0x1ef/0x200 lib/vsprintf.c:591
> >  [<ffffffff8296613f>] string+0x1ef/0x200 lib/vsprintf.c:591
> >  [<ffffffff8296f103>] vsnprintf+0xb83/0x1900 lib/vsprintf.c:2049
> >  [<ffffffff8296e580>] ? pointer+0xab0/0xab0 lib/vsprintf.c:1584
> >  [<ffffffff813456f2>] __request_module+0x132/0x6b0 kernel/kmod.c:146
> >  [<ffffffff814056b0>] ? mark_held_locks+0xd0/0x130 kernel/locking/lockdep.c:2552
> >  [<ffffffff813455c0>] ? call_usermodehelper_setup+0x2b0/0x2b0 kernel/kmod.c:530
> >  [<ffffffff85da47b0>] ? mutex_lock_interruptible_nested+0x980/0x980
> >  [<ffffffff8168fed4>] ? __might_fault+0xe4/0x1d0 mm/memory.c:3833
> >  [<ffffffff8538f74c>] find_inlist_lock.constprop.17+0x10c/0x210 net/bridge/netfilter/ebtables.c:347
> >  [<     inline     >] find_table_lock net/bridge/netfilter/ebtables.c:356
> >  [<ffffffff853904ab>] do_ebt_get_ctl+0x13b/0x540 net/bridge/netfilter/ebtables.c:1524
> >  [<ffffffff85390370>] ? copy_everything_to_user+0x600/0x600 net/bridge/netfilter/ebtables.c:1455
> >  [<     inline     >] ? __mutex_unlock_common_slowpath kernel/locking/mutex.c:751
> >  [<ffffffff85da6799>] ? __mutex_unlock_slowpath+0x239/0x3f0 kernel/locking/mutex.c:762
> >  [<ffffffff85da6959>] ? mutex_unlock+0x9/0x10 kernel/locking/mutex.c:437
> >  [<ffffffff84dea126>] ? nf_sockopt_find+0x1a6/0x220 net/netfilter/nf_sockopt.c:87
> >  [<     inline     >] nf_sockopt net/netfilter/nf_sockopt.c:103
> >  [<ffffffff84dea20d>] nf_getsockopt+0x6d/0xc0 net/netfilter/nf_sockopt.c:121
> >  [<ffffffff84fadf05>] ip_getsockopt+0x135/0x190 net/ipv4/ip_sockglue.c:1523
> >  [<ffffffff84faddd0>] ? do_ip_getsockopt+0x1520/0x1520 net/ipv4/ip_sockglue.c:1353
> >  [<     inline     >] ? wake_up_process kernel/sched/core.c:2024
> >  [<ffffffff8138bcc2>] ? wake_up_q+0x82/0xe0 kernel/sched/core.c:416
> >  [<     inline     >] ? atomic_dec_and_test /arch/x86/include/asm/atomic.h:117
> >  [<     inline     >] ? mmdrop include/linux/sched.h:2611
> >  [<ffffffff814a3310>] ? drop_futex_key_refs.isra.13+0x70/0xe0 kernel/futex.c:444
> >  [<ffffffff8583a4dd>] sctp_getsockopt+0x18d/0x3f40 net/sctp/socket.c:5964
> >  [<ffffffff8140785b>] ? __lock_acquire+0x15fb/0x5dd0 kernel/locking/lockdep.c:3226
> >  [<ffffffff8583a350>] ? sctp_do_peeloff+0x2b0/0x2b0 net/sctp/socket.c:4434
> >  [<ffffffff81406260>] ? debug_check_no_locks_freed+0x290/0x290 kernel/locking/lockdep.c:4104
> >  [<     inline     >] ? rcu_read_unlock include/linux/rcupdate.h:922
> >  [<ffffffff817b398c>] ? __fget+0x20c/0x3b0 fs/file.c:712
> >  [<     inline     >] ? rcu_lock_release include/linux/rcupdate.h:491
> >  [<     inline     >] ? rcu_read_unlock include/linux/rcupdate.h:926
> >  [<ffffffff817b39b5>] ? __fget+0x235/0x3b0 fs/file.c:712
> >  [<ffffffff817b37c7>] ? __fget+0x47/0x3b0 fs/file.c:696
> >  [<ffffffff817b3c11>] ? __fget_light+0xa1/0x1f0 fs/file.c:759
> >  [<ffffffff84c3a695>] sock_common_getsockopt+0x95/0xd0 net/core/sock.c:2579
> >  [<     inline     >] SYSC_getsockopt net/socket.c:1783
> >  [<ffffffff84c37e12>] SyS_getsockopt+0x142/0x230 net/socket.c:1765
> >  [<ffffffff84c37cd0>] ? SyS_setsockopt+0x240/0x240 net/socket.c:1752
> >  [<ffffffff85dab922>] ? entry_SYSCALL_64_fastpath+0x5/0xc1 arch/x86/entry/entry_64.S:191
> >  [<ffffffff81003017>] ? trace_hardirqs_on_thunk+0x17/0x19 arch/x86/entry/thunk_64.S:39
> >  [<ffffffff85dab940>] entry_SYSCALL_64_fastpath+0x23/0xc1 arch/x86/entry/entry_64.S:207
> > Memory state around the buggy address:
> >  ffff88003ae67880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >  ffff88003ae67900: 00 f1 f1 f1 f1 04 f4 f4 f4 f2 f2 f2 f2 00 00 00
> > >ffff88003ae67980: 00 00 00 00 00 00 00 00 00 00 00 00 f4 f3 f3 f3
> >                                                        ^
> >  ffff88003ae67a00: f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >  ffff88003ae67a80: f1 f1 f1 f1 04 f4 f4 f4 f3 f3 f3 f3 00 00 00 00
> > ==================================================================
> > 
> > #include <unistd.h>
> > #include <sys/syscall.h>
> > #include <netinet/in.h>
> > #include <string.h>
> > #include <stdint.h>
> > #include <sys/mman.h>
> > #include <sys/socket.h>
> > 
> > int main()
> > {
> >         int sock = 0;
> >         int sock_dup = 0;
> >         mmap((void *)0x20000000ul, 0x5000ul, PROT_READ|PROT_WRITE,
> > MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0);
> >         sock = socket(AF_INET, SOCK_STREAM, IPPROTO_SCTP);
> >         sock_dup = dup(sock);
> >         memcpy((void*)0x20000bf3,"\xac\x71\x93\x68\x02\xb3\xd1\x86\x52\xf1\xf0\x18\x09\x56\xc6\x98\x6f\x8e\x74\xb7\x17\xd4\x3a\x64\x51\x68\x13\x2d\x25\xba\x6d\x3f\x74\x68\x84\x89\x04\xd1\xa6\xe2\x7d\xaf\xfa\xd9\xce\x52\xbe\x6f\xb6\xe3\xff\x92\x35\xa1\x88\x4a\x68\x27\xaa\x25\xf8\xc1\xd5\x3b\xe5\x69\x11\x4f\x75\x4c\xe9\xff\x8b\x86\x53\x20\xb7\x10\xa2\x62\xcc\xc3\x06\x85\xde\x3e\x1c\x5a\x62\x3a\x2d\x0d\x0b\x0c\xb2\xac\x75\x42\x4d\x82\x3f\x7b\xf7\x28\xea\x2d\xff\x42\xa8\xdf\xb3\x49\x1a\xfd\xae\x2c\xd4\x35\x8e\x96\xb3\xe1\x0a\x92\x56\xb7\xde\xe8\x9e\xc3\x9e\x88\x79\xc4\x71\x46\x27\xf4\x9e\x85\xf4\x8f\x1f\x9a\xe5\x7e\x02\x09\x34\x80\x1e\x87\xa8\x9a\xce\xac\xfb\x43\x07\xdf\x15\xe8\x71\x9a\xa3\x80\x18\x1b\x15\xbd\x57\xb6\xc1\x73\x6e\xb1\x28\x3a\x01\xd5\x8e\x15\x85\xbd\x52\xdf\xfa\x64\xaa\x13\x
 0e\x2f\x64\x05\x11\xce\x79\x8b\xa8\x02\x29\x7f\x72\x0f\x37\x89\xb4\x54\x0b\x09\x02\x75\xc2\x8e\xd7\xcd\x7e\xfb\x4f\x72\xf1\x47\xea\xa2\x2a\xc3\xc4\xe9\x70\xfe\xa5\x80\x88\x21\x33\xcf\x13\x66\!
>  x98\x23\x
>  10\x5c\xa4\xbd\xee\xc0\xb4\xdd\xfb\xff\xf2\x38\xab\xca\x36\x62\x35\x84\xe4\x73\x5c\xc7\x3e\x72\x2e\x17\x43\x6f\x85\x45\x4f\x82\x62\x0d\x77\xae\xcb\xe1\x8f\xe8\xf0\x84\x3e\x62\x8b\x70\x2b\x55\xb5\xa7\x13\xcf\xa1\x78\x77\x82\xe2\xb7\x1c\x65\x7f\xb5\x79\x73\x01\x07\xd1\x9f\x45\x6a\xbb\x3d\xbf\xc8\x71\x5b\x9f\x30\xc7\xb9\xb8\x53\x9f\xe1\xba\xb6\x78\x9e\x05\x75\xa3\x55\xb1\x26\x96\xa9\xb2\x82\xce\x81\x5c\x8a\x18\xb3\x4b\x0c\x18\x8c\xf2\x7c\x09\xde\xcb\xcf\x78\x22\x58\xf6\x15\xf6\xf7\x48\xda\x08\x75\xd4\xc1\x20\xc3\x18\x2e\x89\xe8\x5b\x48\xd9\xbc\x1f\xbb\xed\x31\xaf\x12\x4d\xcd\x46\x60\xa0\xef\x0e\x2e\x21\x1d\x2b\x68\x75\xb9\x42\x5e\xd7\xae\x35\x46\xe9\x06\x63\x1d\x3c\xd6\x9c\x14\x3b\x09\x29\x49\x70\xb9\xe1\xe0\x09\x45\x41\x62\x0c\xff\x5a\x77\xbe\x31\xa6\x03\x94\x92\xde\x41\x99\xfa\x68\x99\x7
 4\xbb\x0a\x3d\xac\x9c\x7e\x00\x6b\xcd\xc1\x83\xa7\xc5\x63\xdd\x10\xea\x59\x27\xdc\x02\x98\xd6\x43\x20\x24\x4e\xc0\xdc\xa2\x98\xdf\x3e\xaf\x61\x35\xa0\x95\x3f\x9a\xaa\x7d\xe9\xe9\x0d\xe5\x97\x!
>  66\x1a\x9
>  f\xbf\x56\xc8\x37\x84\x18\x2b\xd2\xcd\xd6\xb3\x19\xd8\x4a\x30\x6e\xcb\x99\x1c\xe9\x0f\xdb\xca\x30\xe1\xe2\x90\xba\xb9\x61\x00\xbf\xeb\xad\x6a\xc8\x52\xea\x1a\x92\x05\x0c\x3b\x78\x82\x01\xac\xfd\x88\x6c\xca\xe2\xfb\xe7\x0f\xcc\x75\x9c\x98\x12\x26\xcf\xa6\x80\x02\x35\xdf\x6e\xe1\x11\x1d\xa7\x30\x17\x38\x41\xd9\x81\x55\x1a\x1e\xd1\xfe\x60\xbf\xef\x09\x25\xc0\xdb\x9f\xc4\xc6\x54\x1a\x85\x36\x85\x05\xb3\x9f\x2c\xc5\xcd\x12\x51\xef\xbe\x10\x79\xbf\x11\x00\x47\x0d\x9c\x14\x43\x1a\x46\xea\xd1\x34\x2e\x10\x6b\xa4\x3c\x25\x21\xe3\xb9\x15\x78\x6c\x40\x87\x90\xf7\x93\x5a\x66\x5f\x0a\x76\xff\xc2\xe2\x14\x35\x88\x47\xa1\x33\x5b\x8f\x3d\xc5\x89\xb7\xf9\x8a\x40\xf0\x1e\xc9\x30\xcd\xd8\x96\x41\x78\x58\x97\x49\xc8\x50\x61\x36\x8f\x7e\x44\x41\xc0\x84\xbb\x35\xf0\x63\xa9\xc2\x2a\xbd\xcc\x4b\xab\x8b\x16\x33
 \xc0\x66\xbf\x47\x62\x9b\xc4\x47\x2d\x68\x83\xca\xe3\x52\x79\xd7\xe0\x61\x80\x15\xf1\x90\x83\xa2\xbb\x4c\xe5\x8b\x50\xc8\x1b\x68\x7b\xee\x57\xdc\x54\xfa\x90\xf1\xf5\xec\x7d\x93\xe0\x80\x74\x0!
>  6\xbe\xac
>  \xc8\x85\x4d\xe8\xbf\xd3\xdd\x34\x55\xc4\xbf\x2f\x24\x19\xad\x86\x1e\x69\x2b\x6c\x3f\x00\xe8\x4b\xbb\x99\xcf\x17\x99\x00\x9d\x6c\x70\x57\xcc\x35\xee\x07\x87\x25\x8c\x0c\x8b\x9b\x38\x15\xcc\x05\x6f\xf8\x16\x78\x0b\x41\xfa\x23\x96\xc0\x79\xf8\xb7\xf0\x2b\x60\x7e\x98\xe3\x7b\xab\x80\x1f\x0d\xbf\xf6\x7e\x37\x06\xf1\x11\x42\x38\x2a\x70\xdf\xa4\xca\xf5\xf3\xf4\x7d\xca\x10\x0c\xd5\xe2\x90\xa0\x15\xde\xc2\x61\xa2\x88\xea\x32\x37\x97\x83\xd0\x4c\xad\xe2\xae\x9b\x53\xa2\xc2\x54\x0c\xbd\xe1\x50\x3b\x15\xd4\xb1\xa9\x41\x6e\x18\x2e\x30\x3f\x91\x03\x81\x86\x8c\x5c\x1f\x76\x51\x92\xf5\xb5\xb2\xc3\x16\x01\xef\xe3\x9e\xb1\x92\x0e\x0e\xcb\x20\x7f\x10\x29\x08\x6e\x15\x3d\x1e\x7c\x70\xf5\xb5\x3c\x56\x15\x3c\x59\xe6\xe7\x9e\x16\xcd\xfc\x8e\xfa\x12\x99\xbb\x07\xaa\xd7\x1c\xd0\xae\x93\x4c\xba\x16\x5d\x0c\xed\
 x1d\x02\x87\xcd\x38\x31\xc6\x10\x42\xe1\x46\x4e\xa3\xae\xb6\xda\xb6\xb0\x49\x55\x89\x57\xe6\xac\xe3\xbf\xb5\x5c\x59\x93\x0d\x21\x35\xdd\x57\x8c\x04\x15\x91\x05\x69\x4a\xdb\x5e\xcb\x4d\xa3\x5d!
>  \xa8\x7e\
>  x95\x9e\x9d\x95\x61\xc9\x1c\xdd\x66\x0a\x76\x18\xbb\x59\x6a\xa5\xc0\xf2\xb8\x2f\xa9\x4c\xa8\xb3\x2b\xa3\x8a\xbf\x5c\xe8\x18\x3d\x7f\x0e\x2f\xe9\x06\xf9\xb6\xcc\x60\xcc\x38\x6c\x9a\x78\xa7\x7c\x61",
> > 1037);
> >         getsockopt(sock_dup, IPPROTO_IP, 0x81,  (void *)0x20000bf3ul,
> > (socklen_t *)0x20003000ul);        
> >         return 0;
> > }
> > 
> > Best Regards,
> > 
> > Baozeng Ding
> 
> More likely a netfilter bug in net/bridge/netfilter/ebtables.c

+1. sctp diverted it as the option level is IPPROTO_IP and not
SOL_SCTP.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).