* Re: [PATCH net-next 1/1] tc-testing: initial version of tunnel_key unit tests
From: Lucas Bates @ 2018-06-27 18:50 UTC (permalink / raw)
To: Davide Caratti
Cc: Keara Leibovitz, David Miller, Linux Kernel Network Developers,
Jamal Hadi Salim, Cong Wang, Jiri Pirko
In-Reply-To: <b5a9955f3f64ad5c017e0a995ae2d1580f08092e.camel@redhat.com>
On Tue, Jun 26, 2018 at 10:51 AM, Davide Caratti <dcaratti@redhat.com> wrote:
> On Tue, 2018-06-26 at 09:17 -0400, Keara Leibovitz wrote:
>> Create unittests for the tc tunnel_key action.
>>
>>
>> Signed-off-by: Keara Leibovitz <kleib@mojatatu.com>
>> ---
>> .../tc-testing/tc-tests/actions/tunnel_key.json | 676 +++++++++++++++++++++
>> 1 file changed, 676 insertions(+)
>> create mode 100644 tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
>>
>> diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json b/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
>> new file mode 100644
>> index 000000000000..bfe522ac8177
>
> hello Keara!
>
> I think the 'teardown' stage in some of these tests should be reviewed.
> Those that are meant to test invalid configurations (like dc6b) should
> allow non-zero exit codes in the teardown stage, if the wrong
> configuration is catched by the userspace TC tool, before talking to the
> kernel.
>
> Otherwise, those tests will fail when they are invoked one by one with the
> act_tunnel_key module unloaded.
>
Hi Davide, I thought I'd weigh in here.
In the short term, I think this is reasonable, but it's not a feasible
long-term solution. Here's why:
Allowing non-zero exit codes on setup and teardown was a precaution
that needed to be implemented as flushing actions in a freshly-booted
kernel returned errors - certain actions would only allow you to flush
after that action had been added.
But, doing this on so many test cases means that we can lose control
of the test environment, especially since a lot of commands get copied
between test cases. One test's command under test becomes the next
test case's setup command, etc. This can cause false results and
potentially waste a lot of time for someone trying to track down a
bug... Or cause bugs to be missed.
So, how to fix: we've had some discussions about it already. Jiri had
requested the addition of a config file (like the one at
tools/testing/selftests/net/forwarding/config, and maybe an addition
to the README for tdc for explanation. People would then possibly be
restricted to running one test case file at a time based on what
options they had loaded... This is still not ideal.
I think the best possible fix is to add a new plugin for tdc to
exclude tests based on the kernel config. This would require the
addition of a new optional field to the test case format, where any
and all included modules required for the test to work would be
listed. The plugin would look at this information, do its best to
determine if the currently running kernel supports it, and allows the
test to run or be skipped as a result.
Let me show an example of the new field:
>> --- /dev/null
>> +++ b/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
>> @@ -0,0 +1,676 @@
>>
> ...
>
>> + {
>> + "id": "dc6b",
>> + "name": "Add tunnel_key set action with missing mandatory src_ip parameter",
>> + "category": [
>> + "actions",
>> + "tunnel_key"
>> + ],
"reqModules": [
"CONFIG_NET_ACT_TUNNEL_KEY"
],
>> + "setup": [
>> + [
>> + "$TC actions flush action tunnel_key",
>> + 0,
>> + 1,
>> + 255
>> + ]
>> + ],
>> + "cmdUnderTest": "$TC actions add action tunnel_key set dst_ip 20.20.20.2 id 100",
>> + "expExitCode": "255",
>> + "verifyCmd": "$TC actions list action tunnel_key",
>> + "matchPattern": "action order [0-9]+: tunnel_key set.*dst_ip 20.20.20.2.*key_id 100",
>> + "matchCount": "0",
>> + "teardown": [
>> + "$TC actions flush action tunnel_key"
>> + ]
>> + },
As we venture into more and more complicated tests, where different
modules would start getting mixed together, this might be the most
effective route.
This plugin will require some changes I've made to our local version
of tdc that I've been testing out - they change the way tdc handles
its test results, and also give it the ability to skip tests without
affecting the rest of the test run.
Until I'm able to submit everything, I'd be OK with having Keara add
the non-zero exit codes to the teardown on her tests. In the meantime
we'll get the README updated and config file added as well.
How does this sound?
- Lucas
^ permalink raw reply
* Re: [PATCH rdma-next 08/12] overflow.h: Add arithmetic shift helper
From: Kees Cook @ 2018-06-27 18:44 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Rasmus Villemoes, Leon Romanovsky, Doug Ledford, Leon Romanovsky,
RDMA mailing list, Hadar Hen Zion, Matan Barak, Michael J Ruhl,
Noa Osherovich, Raed Salem, Yishai Hadas, Saeed Mahameed,
linux-netdev, LKML
In-Reply-To: <20180627181012.GM20754@mellanox.com>
On Wed, Jun 27, 2018 at 11:10 AM, Jason Gunthorpe <jgg@mellanox.com> wrote:
> Leon? Seems like agreement, Can you work with this version?
>
> #include <stdint.h>
> #include <stdbool.h>
> #include <assert.h>
>
> #define u64 uint64_t
>
> /*
> * Compute *d = (a << s)
> *
> * Returns true if '*d' cannot hold the result or 'a << s' doesn't make sense.
> * - 'a << s' causes bits to be lost when stored in d
> * - 's' is garbage (eg negative) or so large that a << s is guarenteed to be 0
> * - 'a' is negative
> * - 'a << s' sets the sign bit, if any, in '*d'
> * *d is not defined if false is returned.
> */
> #define check_shift_overflow(a, s, d) \
> ({ \
> typeof(a) _a = a; \
> typeof(s) _s = s; \
> typeof(d) _d = d; \
> u64 _a_full = _a; \
> unsigned int _to_shift = \
> _s >= 0 && _s < 8 * sizeof(*d) ? _s : 0; \
> \
> *_d = (_a_full << _to_shift); \
> \
> (_to_shift != _s || *_d < 0 || _a < 0 || \
> (*_d >> _to_shift) != a); \
> })
>
> int main(int argc, const char *argv[])
> {
> int32_t s32;
> uint32_t u32;
>
> assert(check_shift_overflow(1, 0, &s32) == false && s32 == (1 << 0));
> assert(check_shift_overflow(1, 1, &s32) == false && s32 == (1 << 1));
> assert(check_shift_overflow(1, 30, &s32) == false && s32 == (1 << 30));
> assert(check_shift_overflow(1, 31, &s32) == true);
> assert(check_shift_overflow(1, 32, &s32) == true);
> assert(check_shift_overflow(-1, 1, &s32) == true);
> assert(check_shift_overflow(-1, 0, &s32) == true);
>
> assert(check_shift_overflow(1, 0, &u32) == false && u32 == (1 << 0));
> assert(check_shift_overflow(1, 1, &u32) == false && u32 == (1 << 1));
> assert(check_shift_overflow(1, 30, &u32) == false && u32 == (1 << 30));
> assert(check_shift_overflow(1, 31, &u32) == false && u32 == (1UL << 31));
> assert(check_shift_overflow(1, 32, &u32) == true);
> assert(check_shift_overflow(-1, 1, &u32) == true);
> assert(check_shift_overflow(-1, 0, &u32) == true);
>
> assert(check_shift_overflow(0xFFFFFFFF, 0, &u32) == false && u32 == (0xFFFFFFFFUL << 0));
> assert(check_shift_overflow(0xFFFFFFFF, 1, &u32) == true);
> assert(check_shift_overflow(0xFFFFFFFF, 0, &s32) == true);
> assert(check_shift_overflow(0xFFFFFFFF, 1, &s32) == true);
> }
Oh yes, please include these tests in lib/test_overflow.c too! Nice. :)
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [patch net-next 0/9] net: sched: introduce chain templates support with offloading to mlxsw
From: Jakub Kicinski @ 2018-06-27 18:36 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linux Netdev List, David Miller, Jamal Hadi Salim, Cong Wang,
Simon Horman, John Hurley, David Ahern, mlxsw
In-Reply-To: <20180627075017.GA2007@nanopsycho>
On Wed, 27 Jun 2018 09:50:17 +0200, Jiri Pirko wrote:
> Tue, Jun 26, 2018 at 11:18:58PM CEST, jakub.kicinski@netronome.com wrote:
> >On Tue, 26 Jun 2018 09:12:17 +0200, Jiri Pirko wrote:
> >> Tue, Jun 26, 2018 at 09:00:45AM CEST, jakub.kicinski@netronome.com wrote:
> >> >On Mon, Jun 25, 2018 at 11:43 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> >> >> Tue, Jun 26, 2018 at 06:58:50AM CEST, jakub.kicinski@netronome.com wrote:
> >> >>>On Mon, 25 Jun 2018 23:01:39 +0200, Jiri Pirko wrote:
> >> >>>> From: Jiri Pirko <jiri@mellanox.com>
> >> >>>>
> >> >>>> For the TC clsact offload these days, some of HW drivers need
> >> >>>> to hold a magic ball. The reason is, with the first inserted rule inside
> >> >>>> HW they need to guess what fields will be used for the matching. If
> >> >>>> later on this guess proves to be wrong and user adds a filter with a
> >> >>>> different field to match, there's a problem. Mlxsw resolves it now with
> >> >>>> couple of patterns. Those try to cover as many match fields as possible.
> >> >>>> This aproach is far from optimal, both performance-wise and scale-wise.
> >> >>>> Also, there is a combination of filters that in certain order won't
> >> >>>> succeed.
> >> >>>>
> >> >>>> Most of the time, when user inserts filters in chain, he knows right away
> >> >>>> how the filters are going to look like - what type and option will they
> >> >>>> have. For example, he knows that he will only insert filters of type
> >> >>>> flower matching destination IP address. He can specify a template that
> >> >>>> would cover all the filters in the chain.
> >> >>>
> >> >>>Perhaps it's lack of sleep, but this paragraph threw me a little off
> >> >>>the track. IIUC the goal of this set is to provide a way to inform the
> >> >>>HW about expected matches before any rule is programmed into the HW.
> >> >>>Not before any rule is added to a particular chain. One can just use
> >> >>>the first rule in the chain to make a guess about the chain, but thanks
> >> >>>to this set user can configure *all* chains before any rules are added.
> >> >>
> >> >> The template is per-chain. User can use template for chain x and
> >> >> not-use it for chain y. Up to him.
> >> >
> >> >Makes sense.
> >> >
> >> >I can't help but wonder if it'd be better to associate the
> >> >constraints/rules with chains instead of creating a new "template"
> >> >object. It seems more natural to create a chain with specific
> >> >constraints in place than add and delete template of which there can
> >> >be at most one to a chain... Perhaps that's more about the user space
> >> >tc command line. Anyway, not a strong objection, just a thought.
> >>
> >> Hmm. I don't think it is good idea. User should see the template in a
> >> "show" command per chain. We would have to have 2 show commands, one to
> >> list the template objects and one to list templates per chains. It makes
> >> things more complicated for no good reason. I think that this simple
> >> chain-lock is easier and serves the purpose.
> >
> >Hm, I think the dump is fine, what I was thinking about was:
> >
> ># tc chain add dev dummy0 ingress chain_index 22 \
> > ^^^^^
> > template proto ip \
> > ^^^^^^^^
> > flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>
> Okay, I got it. I see 2 issues.
> 1) user might expect to add a chain without the template. But that does
> not make sense really. Chains are created/deleted implicitly
> according to refcount.
> 2) there is not chain object like this available to user. Adding it just
> for template looks odd. Also, the "filter" and "template" are very
> much alike. They both are added to a chain, they both implicitly
> create chain if it does not exist, etc.
Yeah, that part makes is tricky :/
> if you don't like "tc filter template add dev dummy0 ingress", how
> about:
> "tc template add dev dummy0 ingress ..."
> "tc template add dev dummy0 ingress chain 22 ..."
> that makes more sense I think.
Mmm.. how about:
tc chaintemplate add dev dummy0 ingress...
or
tc restrictedchain add dev dummy0 ingress chain_index XX template ...
^ permalink raw reply
* [PATCH 1/2] net: phy: DP83TC811: Add INT_STAT3
From: Dan Murphy @ 2018-06-27 18:16 UTC (permalink / raw)
To: andrew, f.fainelli; +Cc: netdev, Dan Murphy
Add INT_STAT3 interrupt setting and clearing.
Also fixed writing to INT_STAT2 when disabling
the interrupts as there was a double write to
INT_STAT1.
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
drivers/net/phy/dp83tc811.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/dp83tc811.c b/drivers/net/phy/dp83tc811.c
index 081d99aa3985..f8653f5d8789 100644
--- a/drivers/net/phy/dp83tc811.c
+++ b/drivers/net/phy/dp83tc811.c
@@ -21,6 +21,7 @@
#define MII_DP83811_SGMII_CTRL 0x09
#define MII_DP83811_INT_STAT1 0x12
#define MII_DP83811_INT_STAT2 0x13
+#define MII_DP83811_INT_STAT3 0x18
#define MII_DP83811_RESET_CTRL 0x1f
#define DP83811_HW_RESET BIT(15)
@@ -44,6 +45,11 @@
#define DP83811_OVERVOLTAGE_INT_EN BIT(6)
#define DP83811_UNDERVOLTAGE_INT_EN BIT(7)
+/* INT_STAT3 bits */
+#define DP83811_LPS_INT_EN BIT(0)
+#define DP83811_NO_FRAME_INT_EN BIT(3)
+#define DP83811_POR_DONE_INT_EN BIT(4)
+
#define MII_DP83811_RXSOP1 0x04a5
#define MII_DP83811_RXSOP2 0x04a6
#define MII_DP83811_RXSOP3 0x04a7
@@ -81,6 +87,10 @@ static int dp83811_ack_interrupt(struct phy_device *phydev)
if (err < 0)
return err;
+ err = phy_read(phydev, MII_DP83811_INT_STAT3);
+ if (err < 0)
+ return err;
+
return 0;
}
@@ -216,13 +226,29 @@ static int dp83811_config_intr(struct phy_device *phydev)
DP83811_UNDERVOLTAGE_INT_EN);
err = phy_write(phydev, MII_DP83811_INT_STAT2, misr_status);
+ if (err < 0)
+ return err;
+
+ misr_status = phy_read(phydev, MII_DP83811_INT_STAT3);
+ if (misr_status < 0)
+ return misr_status;
+
+ misr_status |= (DP83811_LPS_INT_EN |
+ DP83811_NO_FRAME_INT_EN |
+ DP83811_POR_DONE_INT_EN);
+
+ err = phy_write(phydev, MII_DP83811_INT_STAT3, misr_status);
} else {
err = phy_write(phydev, MII_DP83811_INT_STAT1, 0);
if (err < 0)
return err;
- err = phy_write(phydev, MII_DP83811_INT_STAT1, 0);
+ err = phy_write(phydev, MII_DP83811_INT_STAT2, 0);
+ if (err < 0)
+ return err;
+
+ err = phy_write(phydev, MII_DP83811_INT_STAT3, 0);
}
return err;
--
2.17.0.582.gccdcbd54c
^ permalink raw reply related
* [PATCH 2/2] net: phy: DP83TC811: Fix SGMII enable/disable
From: Dan Murphy @ 2018-06-27 18:16 UTC (permalink / raw)
To: andrew, f.fainelli; +Cc: netdev, Dan Murphy
In-Reply-To: <20180627181618.23463-1-dmurphy@ti.com>
If SGMII was selected in the DT then the device should
write the SGMII enable bit.
If SGMII is not selected in the DT then the SGMII bit
should be disabled.
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
arch/arm/configs/omap2plus_defconfig | 1 +
drivers/net/phy/dp83tc811.c | 20 +++++++++-----------
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
index 06fb948ecfb3..30857d5b7a6c 100644
--- a/arch/arm/configs/omap2plus_defconfig
+++ b/arch/arm/configs/omap2plus_defconfig
@@ -182,6 +182,7 @@ CONFIG_TI_CPTS=y
CONFIG_AT803X_PHY=y
CONFIG_DP83848_PHY=y
CONFIG_DP83867_PHY=y
+CONFIG_DP83TC811_PHY=y
CONFIG_MICREL_PHY=y
CONFIG_SMSC_PHY=y
CONFIG_PPP=m
diff --git a/drivers/net/phy/dp83tc811.c b/drivers/net/phy/dp83tc811.c
index f8653f5d8789..78cad134a79e 100644
--- a/drivers/net/phy/dp83tc811.c
+++ b/drivers/net/phy/dp83tc811.c
@@ -284,21 +284,19 @@ static int dp83811_config_init(struct phy_device *phydev)
if (err < 0)
return err;
+ value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
- value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
- if (!(value & DP83811_SGMII_EN)) {
- err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
+ err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
(DP83811_SGMII_EN | value));
- if (err < 0)
- return err;
- } else {
- err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
- (~DP83811_SGMII_EN & value));
- if (err < 0)
- return err;
- }
+ } else {
+ err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
+ (~DP83811_SGMII_EN & value));
}
+ if (err < 0)
+
+ return err;
+
value = DP83811_WOL_MAGIC_EN | DP83811_WOL_SECURE_ON | DP83811_WOL_EN;
return phy_write_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_CFG,
--
2.17.0.582.gccdcbd54c
^ permalink raw reply related
* Re: [PATCH net-next] liquidio: fix kernel panic when NIC firmware is older than 1.7.2
From: Felix Manlunas @ 2018-06-28 5:18 UTC (permalink / raw)
To: Shannon Nelson
Cc: davem, netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
ricardo.farrington
In-Reply-To: <5670609b-5056-305d-ee0f-8fed471d8381@oracle.com>
On Tue, Jun 26, 2018 at 09:03:25AM -0700, Shannon Nelson wrote:
> On 6/26/2018 4:58 AM, Felix Manlunas wrote:
> > From: Rick Farrington <ricardo.farrington@cavium.com>
> >
> > Pre-1.7.2 NIC firmware does not support (and does not respond to) the "get
> > speed" command which is sent by the 1.7.2 driver during modprobe. Due to a
> > bug in older firmware (with respect to unknown commands), this unsupported
> > command causes a cascade of errors that ends in a kernel panic.
> >
> > Fix it by making the sending of the "get speed" command conditional on the
> > firmware version.
> >
> > Signed-off-by: Rick Farrington <ricardo.farrington@cavium.com>
> > Acked-by: Derek Chickles <derek.chickles@cavium.com>
> > Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
> > ---
> > Note: To avoid checkpatch.pl "WARNING: line over 80 characters", the comma
> > that separates the arguments in the call to strcmp() was placed one
> > line below the usual spot.
> >
> > drivers/net/ethernet/cavium/liquidio/lio_main.c | 11 ++++++++++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
> > index 7cb4e75..f83f884 100644
> > --- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
> > +++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
> > @@ -3671,7 +3671,16 @@ static int setup_nic_devices(struct octeon_device *octeon_dev)
> > OCTEON_CN2350_25GB_SUBSYS_ID ||
> > octeon_dev->subsystem_id ==
> > OCTEON_CN2360_25GB_SUBSYS_ID) {
> > - liquidio_get_speed(lio);
> > + /* speed control unsupported in f/w older than 1.7.2 */
> > + if (strcmp(octeon_dev->fw_info.liquidio_firmware_version
> > + , "1.7.2") < 0) {
>
> Will the liquidio_firmware_version ever end up something like 1.7.10?
> If so, this strcmp() may not do what you want.
>
> sln
Yes, it's possible that the liquidio_firmware_version will reach 1.7.10.
We agree that using strcmp() will not give the correct result for that case,
so please disregard this patch.
We will submit a V2 patch.
Felix
^ permalink raw reply
* Re: [PATCH rdma-next 08/12] overflow.h: Add arithmetic shift helper
From: Leon Romanovsky @ 2018-06-27 18:22 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Rasmus Villemoes, Doug Ledford, Kees Cook, RDMA mailing list,
Hadar Hen Zion, Matan Barak, Michael J Ruhl, Noa Osherovich,
Raed Salem, Yishai Hadas, Saeed Mahameed, linux-netdev,
linux-kernel
In-Reply-To: <20180627181012.GM20754@mellanox.com>
On Wed, Jun 27, 2018 at 12:10:12PM -0600, Jason Gunthorpe wrote:
> On Wed, Jun 27, 2018 at 11:36:03AM +0200, Rasmus Villemoes wrote:
> > OK. The requirement of everything having the same type for the
> > check_*_overflow when gccs builtins are not available was mostly a
> > consequence of my inability to implement completely type-generic
> > versions (but also to enforce some sanity, so people don't do
> > check_add_overflow( s8, size_t, int*)). There's no gcc builtin for
> > shift, but if it's relatively simple to one allowing a and *d to have
> > different types, then why not. It's of course particularly convenient
> > to allow a bare "1" (i.e. int) as a while having *d have some random
> > type.
>
> Yes
>
> > Wouldn't check_shift_overflow(-1, 4, &someint) just put -16 in someint
> > and report no overflow? That's what I'd expect, if negative values are
> > to be supported at all.
>
> I would say that is not a desired outcome, bitshift is defined on
> bits, if the caller wanted something defined as signed multiply they
> should use multiply.
>
> IMHO, nobody writes 'a << b' expecting sign preservation..
>
> > Well, the types you can check at compile-time, the values not, so you
> > still have to define the result, i.e. contents of *d, for negative
> > values (even if we decide that "overflow" should always be signalled in
> > that case).
>
> Why do a need to define a 'result' beyond whatever the not-undefined
> behavior shift expression produces?
>
> > What about more like this?
> > check_shift_overflow(a, s, d) ({
> > // Shift is always performed on the machine's largest
> > unsigned
> > u64 _a = a;
> > typeof(s) _s = s;
> > typeof(d) _d = d;
> > // Make s safe against UB
> > unsigned int _to_shift = _s >= 0 && _s < 8*sizeof(*d) : _s ? 0;
> > *_d = (_a << _to_shift);
> > // s is malformed
> > (_to_shift != _s ||
> > // d is a signed type and became negative
> > *_d < 0 ||
> > // a is a signed type and was negative
> > _a < 0 ||
> > // Not invertable means a was truncated during
> > shifting
> > (*_d >> _to_shift) != a))
> > })
> > I'm not seeing a UB with this?
> >
> > Something like that might work, but you're not there yet. In
> > particular, your test for whether a is negative is thwarted by using
> > u64 for _a and testing _a < 0...
>
> Oops, yes that was intended to be 'a', and of course we need to
> capture it..
>
> Leon? Seems like agreement, Can you work with this version?
Yes, sure, I waited for an agreement.
>
> #include <stdint.h>
> #include <stdbool.h>
> #include <assert.h>
>
> #define u64 uint64_t
>
> /*
> * Compute *d = (a << s)
> *
> * Returns true if '*d' cannot hold the result or 'a << s' doesn't make sense.
> * - 'a << s' causes bits to be lost when stored in d
> * - 's' is garbage (eg negative) or so large that a << s is guarenteed to be 0
> * - 'a' is negative
> * - 'a << s' sets the sign bit, if any, in '*d'
> * *d is not defined if false is returned.
> */
> #define check_shift_overflow(a, s, d) \
> ({ \
> typeof(a) _a = a; \
> typeof(s) _s = s; \
> typeof(d) _d = d; \
> u64 _a_full = _a; \
> unsigned int _to_shift = \
> _s >= 0 && _s < 8 * sizeof(*d) ? _s : 0; \
> \
> *_d = (_a_full << _to_shift); \
> \
> (_to_shift != _s || *_d < 0 || _a < 0 || \
> (*_d >> _to_shift) != a); \
> })
>
> int main(int argc, const char *argv[])
> {
> int32_t s32;
> uint32_t u32;
>
> assert(check_shift_overflow(1, 0, &s32) == false && s32 == (1 << 0));
> assert(check_shift_overflow(1, 1, &s32) == false && s32 == (1 << 1));
> assert(check_shift_overflow(1, 30, &s32) == false && s32 == (1 << 30));
> assert(check_shift_overflow(1, 31, &s32) == true);
> assert(check_shift_overflow(1, 32, &s32) == true);
> assert(check_shift_overflow(-1, 1, &s32) == true);
> assert(check_shift_overflow(-1, 0, &s32) == true);
>
> assert(check_shift_overflow(1, 0, &u32) == false && u32 == (1 << 0));
> assert(check_shift_overflow(1, 1, &u32) == false && u32 == (1 << 1));
> assert(check_shift_overflow(1, 30, &u32) == false && u32 == (1 << 30));
> assert(check_shift_overflow(1, 31, &u32) == false && u32 == (1UL << 31));
> assert(check_shift_overflow(1, 32, &u32) == true);
> assert(check_shift_overflow(-1, 1, &u32) == true);
> assert(check_shift_overflow(-1, 0, &u32) == true);
>
> assert(check_shift_overflow(0xFFFFFFFF, 0, &u32) == false && u32 == (0xFFFFFFFFUL << 0));
> assert(check_shift_overflow(0xFFFFFFFF, 1, &u32) == true);
> assert(check_shift_overflow(0xFFFFFFFF, 0, &s32) == true);
> assert(check_shift_overflow(0xFFFFFFFF, 1, &s32) == true);
> }
>
> Thanks,
> Jason
^ permalink raw reply
* Re: [PATCH bpf 3/4] samples/bpf: deal with EBUSY return code from sendmsg in xdpsock sample
From: Song Liu @ 2018-06-27 18:21 UTC (permalink / raw)
To: Magnus Karlsson
Cc: bjorn.topel, ast, Daniel Borkmann, Networking, qi.z.zhang, pavel
In-Reply-To: <1530108136-4984-4-git-send-email-magnus.karlsson@intel.com>
On Wed, Jun 27, 2018 at 7:02 AM, Magnus Karlsson
<magnus.karlsson@intel.com> wrote:
> Sendmsg in the SKB path of AF_XDP can now return EBUSY when a packet
> was discarded and completed by the driver. Just ignore this message
> in the sample application.
>
> Fixes: b4b8faa1ded7 ("samples/bpf: sample application and documentation for AF_XDP sockets")
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> Reported-by: Pavel Odintsov <pavel@fastnetmon.com>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> samples/bpf/xdpsock_user.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
> index d69c8d78d3fd..aec3a61fac44 100644
> --- a/samples/bpf/xdpsock_user.c
> +++ b/samples/bpf/xdpsock_user.c
> @@ -729,7 +729,7 @@ static void kick_tx(int fd)
> int ret;
>
> ret = sendto(fd, NULL, 0, MSG_DONTWAIT, NULL, 0);
> - if (ret >= 0 || errno == ENOBUFS || errno == EAGAIN)
> + if (ret >= 0 || errno == EAGAIN || errno == EBUSY)
> return;
> lassert(0);
> }
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH bpf 2/4] xsk: frame could be completed more than once in SKB path
From: Song Liu @ 2018-06-27 18:19 UTC (permalink / raw)
To: Magnus Karlsson
Cc: bjorn.topel, ast, Daniel Borkmann, Networking, qi.z.zhang, pavel
In-Reply-To: <1530108136-4984-3-git-send-email-magnus.karlsson@intel.com>
On Wed, Jun 27, 2018 at 7:02 AM, Magnus Karlsson
<magnus.karlsson@intel.com> wrote:
> Fixed a bug in which a frame could be completed more than once
> when an error was returned from dev_direct_xmit(). The code
> erroneously retried sending the message leading to multiple
> calls to the SKB destructor and therefore multiple completions
> of the same buffer to user space.
>
> The error code in this case has been changed from EAGAIN to EBUSY
> in order to tell user space that the sending of the packet failed
> and the buffer has been return to user space through the completion
> queue.
>
> Fixes: 35fcde7f8deb ("xsk: support for Tx")
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> Reported-by: Pavel Odintsov <pavel@fastnetmon.com>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> net/xdp/xsk.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 3b3410ada097..d482f727f4c2 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -268,15 +268,15 @@ static int xsk_generic_xmit(struct sock *sk, struct msghdr *m,
> skb->destructor = xsk_destruct_skb;
>
> err = dev_direct_xmit(skb, xs->queue_id);
> + xskq_discard_desc(xs->tx);
> /* Ignore NET_XMIT_CN as packet might have been sent */
> if (err == NET_XMIT_DROP || err == NETDEV_TX_BUSY) {
> - err = -EAGAIN;
> - /* SKB consumed by dev_direct_xmit() */
> + /* SKB completed but not sent */
> + err = -EBUSY;
> goto out;
> }
>
> sent_frame = true;
> - xskq_discard_desc(xs->tx);
> }
>
> out:
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v7 06/11] net: pch_gbe: Only enable MAC when PHY link is active
From: Paul Burton @ 2018-06-27 18:15 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, David S . Miller, Andrew Lunn
In-Reply-To: <34bf7133-5f4d-ef61-2448-e1fcc1e3036a@gmail.com>
Hi Florian,
On Wed, Jun 27, 2018 at 10:54:24AM -0700, Florian Fainelli wrote:
> On 06/26/2018 05:06 PM, Paul Burton wrote:
> > When using a PHY connected via RGMII, as the pch_gbe driver presumes is
> > the case, the RX clock is provided by the PHY to the MAC. Various PHYs,
> > including both the AR8031 used by the Minnowboard & the RTL8211E used by
> > the MIPS Boston development board, will stop generating the RX clock
> > when the ethernet link is down (eg. the ethernet cable is unplugged).
> >
> > Various pieces of functionality in the EG20T MAC, ranging from basics
> > like completing a MAC reset to programming MAC addresses, rely upon the
> > RX clock being provided. When the clock is not provided these pieces of
> > functionality simply never complete, and the busy bits that indicate
> > they're in progress remain set indefinitely.
> >
> > The pch_gbe driver currently requires that the RX clock is always
> > provided, and attempts to enforce this by disabling the hibernation
> > feature of the AR8031 PHY to keep it generating the RX clock. This patch
> > moves us away from this model by only configuring the MAC when the PHY
> > indicates that the ethernet link is up. When the link is up we should be
> > able to safely expect that the RX clock is being provided, and therefore
> > safely reset & configure the MAC.
>
> What we ended up doing in the bcmgenet driver is loop back the RX and TX
> clocks such that we always have a clock that we can use to perform any
> MAC operation, including reset.
>
> Is this an option here?
That sounds like a nice solution, but I don't see a way to do it in the
EG20T datasheet[1].
> You might also want to split the allocation from the actual
> initialization if this is not done already.
Some of the buffer allocation is still happening in pch_gbe_up() rather
than when the link actually comes up, but there is more that could
probably be moved.
Thanks,
Paul
[1] https://www.intel.com/content/www/us/en/intelligent-systems/queens-bay/platform-controller-hub-eg20t-datasheet.html
^ permalink raw reply
* Re: [PATCH rdma-next 08/12] overflow.h: Add arithmetic shift helper
From: Jason Gunthorpe @ 2018-06-27 18:10 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Leon Romanovsky, Doug Ledford, Kees Cook, Leon Romanovsky,
RDMA mailing list, Hadar Hen Zion, Matan Barak, Michael J Ruhl,
Noa Osherovich, Raed Salem, Yishai Hadas, Saeed Mahameed,
linux-netdev, linux-kernel
In-Reply-To: <CAKwiHFgchr+6HYOZ4e4e1vzL9cFabe6eonNNM8NTWZypazcuKA@mail.gmail.com>
On Wed, Jun 27, 2018 at 11:36:03AM +0200, Rasmus Villemoes wrote:
> OK. The requirement of everything having the same type for the
> check_*_overflow when gccs builtins are not available was mostly a
> consequence of my inability to implement completely type-generic
> versions (but also to enforce some sanity, so people don't do
> check_add_overflow( s8, size_t, int*)). There's no gcc builtin for
> shift, but if it's relatively simple to one allowing a and *d to have
> different types, then why not. It's of course particularly convenient
> to allow a bare "1" (i.e. int) as a while having *d have some random
> type.
Yes
> Wouldn't check_shift_overflow(-1, 4, &someint) just put -16 in someint
> and report no overflow? That's what I'd expect, if negative values are
> to be supported at all.
I would say that is not a desired outcome, bitshift is defined on
bits, if the caller wanted something defined as signed multiply they
should use multiply.
IMHO, nobody writes 'a << b' expecting sign preservation..
> Well, the types you can check at compile-time, the values not, so you
> still have to define the result, i.e. contents of *d, for negative
> values (even if we decide that "overflow" should always be signalled in
> that case).
Why do a need to define a 'result' beyond whatever the not-undefined
behavior shift expression produces?
> What about more like this?
> check_shift_overflow(a, s, d) ({
> // Shift is always performed on the machine's largest
> unsigned
> u64 _a = a;
> typeof(s) _s = s;
> typeof(d) _d = d;
> // Make s safe against UB
> unsigned int _to_shift = _s >= 0 && _s < 8*sizeof(*d) : _s ? 0;
> *_d = (_a << _to_shift);
> // s is malformed
> (_to_shift != _s ||
> // d is a signed type and became negative
> *_d < 0 ||
> // a is a signed type and was negative
> _a < 0 ||
> // Not invertable means a was truncated during
> shifting
> (*_d >> _to_shift) != a))
> })
> I'm not seeing a UB with this?
>
> Something like that might work, but you're not there yet. In
> particular, your test for whether a is negative is thwarted by using
> u64 for _a and testing _a < 0...
Oops, yes that was intended to be 'a', and of course we need to
capture it..
Leon? Seems like agreement, Can you work with this version?
#include <stdint.h>
#include <stdbool.h>
#include <assert.h>
#define u64 uint64_t
/*
* Compute *d = (a << s)
*
* Returns true if '*d' cannot hold the result or 'a << s' doesn't make sense.
* - 'a << s' causes bits to be lost when stored in d
* - 's' is garbage (eg negative) or so large that a << s is guarenteed to be 0
* - 'a' is negative
* - 'a << s' sets the sign bit, if any, in '*d'
* *d is not defined if false is returned.
*/
#define check_shift_overflow(a, s, d) \
({ \
typeof(a) _a = a; \
typeof(s) _s = s; \
typeof(d) _d = d; \
u64 _a_full = _a; \
unsigned int _to_shift = \
_s >= 0 && _s < 8 * sizeof(*d) ? _s : 0; \
\
*_d = (_a_full << _to_shift); \
\
(_to_shift != _s || *_d < 0 || _a < 0 || \
(*_d >> _to_shift) != a); \
})
int main(int argc, const char *argv[])
{
int32_t s32;
uint32_t u32;
assert(check_shift_overflow(1, 0, &s32) == false && s32 == (1 << 0));
assert(check_shift_overflow(1, 1, &s32) == false && s32 == (1 << 1));
assert(check_shift_overflow(1, 30, &s32) == false && s32 == (1 << 30));
assert(check_shift_overflow(1, 31, &s32) == true);
assert(check_shift_overflow(1, 32, &s32) == true);
assert(check_shift_overflow(-1, 1, &s32) == true);
assert(check_shift_overflow(-1, 0, &s32) == true);
assert(check_shift_overflow(1, 0, &u32) == false && u32 == (1 << 0));
assert(check_shift_overflow(1, 1, &u32) == false && u32 == (1 << 1));
assert(check_shift_overflow(1, 30, &u32) == false && u32 == (1 << 30));
assert(check_shift_overflow(1, 31, &u32) == false && u32 == (1UL << 31));
assert(check_shift_overflow(1, 32, &u32) == true);
assert(check_shift_overflow(-1, 1, &u32) == true);
assert(check_shift_overflow(-1, 0, &u32) == true);
assert(check_shift_overflow(0xFFFFFFFF, 0, &u32) == false && u32 == (0xFFFFFFFFUL << 0));
assert(check_shift_overflow(0xFFFFFFFF, 1, &u32) == true);
assert(check_shift_overflow(0xFFFFFFFF, 0, &s32) == true);
assert(check_shift_overflow(0xFFFFFFFF, 1, &s32) == true);
}
Thanks,
Jason
^ permalink raw reply
* Re: [PATCH v7 09/11] net: pch_gbe: Convert to mdiobus and phylib
From: Paul Burton @ 2018-06-27 18:09 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, David S . Miller
In-Reply-To: <20180627175144.GG885@lunn.ch>
Hi Andrew,
On Wed, Jun 27, 2018 at 07:51:44PM +0200, Andrew Lunn wrote:
> > @@ -5,7 +5,8 @@
> > config PCH_GBE
> > tristate "OKI SEMICONDUCTOR IOH(ML7223/ML7831) GbE"
> > depends on PCI && (X86_32 || COMPILE_TEST)
> > - select MII
> > + select PHYLIB
> > + imply AT803X_PHY if X86_32
> > select PTP_1588_CLOCK_PCH
> > select NET_PTP_CLASSIFY
>
> That is unusual. I don't think any other MAC driver does this.
>
> If the AT803X driver is not available, it will fall back to the
> generic PHY driver. That means RGMII delays will not get set
> correctly, no interrupts, no wol, and no workaround for the 8030.
>
> Are any of these relevant to your board?
Well, my board uses an RTL8211E PHY, doesn't support suspending so WoL
isn't applicable & with this series isn't yet using PHY interrupts
(though that would be ideal as a later addition). So for my board I
enable CONFIG_REALTEK_PHY & I don't have CONFIG_AT8031X_PHY enabled.
It seems the Minnowboard uses the AT8031 PHY, but it's not the only X86
board that includes the EG20T & not all of those use the AT8031. For
example I have access to an Aaeon NanoCOM-TC module[1] which uses the
EG20T & pch_gbe, but its datasheet lists an RTL8211CL PHY (which is
presumably misconfigured by current kernels, though at least for basic
network access is functional).
The idea behind using imply was that it allows kernel configurations
that have up to now only supported the AT8031 PHY via pch_gbe's custom
code to automatically continue to support that PHY, but also allows
support for it to be disabled for systems that do not use that PHY (for
example mine or the Aaeon system).
Would you prefer that the MAC driver instead selects the PHY drivers for
all PHYs known to have been used with the MAC? Or would you be happy if
I added the equivalent in patch 11:
imply REALTEK_PHY if MIPS
Though perhaps REALTEK_PHY would be good to enable for X86_32 too to
cover that Aaeon system...
Thanks,
Paul
[1] http://www.aaeon.com/en/p/com-express-modules-nanocom-tc
^ permalink raw reply
* Re: [RFC bpf-next 6/6] samples/bpf: Add meta data hash example to xdp_redirect_cpu
From: Saeed Mahameed @ 2018-06-27 18:04 UTC (permalink / raw)
To: saeedm@dev.mellanox.co.il, brouer@redhat.com
Cc: alexander.h.duyck@intel.com, peter.waskiewicz.jr@intel.com,
Rony Efraim, borkmann@iogearbox.net, Tariq Toukan,
neerav.parikh@intel.com, Opher Reviv,
alexei.starovoitov@gmail.com, pjwaskiewicz@gmail.com,
netdev@vger.kernel.org, ttoukan.linux@gmail.com
In-Reply-To: <20180627125914.1a3b52db@redhat.com>
On Wed, 2018-06-27 at 12:59 +0200, Jesper Dangaard Brouer wrote:
> On Tue, 26 Jun 2018 19:46:15 -0700
> Saeed Mahameed <saeedm@dev.mellanox.co.il> wrote:
>
> > Add a new program (prog_num = 4) that will not parse packets and
> > will
> > use the meta data hash to spread/redirect traffic into different
> > cpus.
>
> You cannot "steal" prognum 4, as it is already used for
> "xdp_prognum4_ddos_filter_pktgen". Please append your new prog as
> #5.
>
Sure.
> > For the new program we set on bpf_set_link_xdp_fd:
> > xdp_flags |= XDP_FLAGS_META_HASH | XDP_FLAGS_META_VLAN;
> >
> > On mlx5 it will succeed since mlx5 already supports these flags.
> >
> > The new program will read the value of the hash from the data_meta
> > pointer from the xdp_md and will use it to compute the destination
> > cpu.
> >
> > Note: I didn't test this patch to show redirect works with the
> > hash!
> > I only used it to see that the hash and vlan values are set
> > correctly
> > by the driver and can be seen by the xdp program.
> >
> > * I faced some difficulties to read the hash value using the helper
> > functions defined in the previous patches, but once i used the same
> > logic
> > with out these functions it worked ! Will have to figure this out
> > later.
> >
> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> > ---
> > samples/bpf/xdp_redirect_cpu_kern.c | 67
> > +++++++++++++++++++++++++++++
> > samples/bpf/xdp_redirect_cpu_user.c | 7 +++
> > 2 files changed, 74 insertions(+)
> >
> > diff --git a/samples/bpf/xdp_redirect_cpu_kern.c
> > b/samples/bpf/xdp_redirect_cpu_kern.c
> > index 303e9e7161f3..d6b3f55f342a 100644
> > --- a/samples/bpf/xdp_redirect_cpu_kern.c
> > +++ b/samples/bpf/xdp_redirect_cpu_kern.c
> > @@ -376,6 +376,73 @@ int xdp_prognum3_proto_separate(struct xdp_md
> > *ctx)
> > return bpf_redirect_map(&cpu_map, cpu_dest, 0);
> > }
> >
> > +#if 0
> > +xdp_md_info_arr mdi = {
> > + [XDP_DATA_META_HASH] = {.offset = 0, .present = 1},
> > + [XDP_DATA_META_VLAN] = {.offset = sizeof(struct
> > xdp_md_hash), .present = 1},
> > +};
> > +#endif
>
> Sorry, no global variables avail in the generated BPF byte-code.
>
Yea i found out the hard way :), but for my final solution i would like
to somehow share the same static md info array between netdev and bpf
program, so this code was just experimental.
> > +SEC("xdp_cpu_map4_hash_separate")
> > +int xdp_prognum4_hash_separate(struct xdp_md *ctx)
> > +{
> > + void *data_meta = (void *)(long)ctx->data_meta;
> > + void *data_end = (void *)(long)ctx->data_end;
> > + void *data = (void *)(long)ctx->data;
> > + struct xdp_md_hash *hash;
> > + struct xdp_md_vlan *vlan;
> > + struct datarec *rec;
> > + u32 cpu_dest = 0;
> > + u32 cpu_idx = 0;
> > + u32 *cpu_lookup;
> > + u32 key = 0;
> > +
> > + /* Count RX packet in map */
> > + rec = bpf_map_lookup_elem(&rx_cnt, &key);
> > + if (!rec)
> > + return XDP_ABORTED;
> > + rec->processed++;
> > +
> > + /* for some reason this code fails to be verified */
> > +#if 0
> > + hash = xdp_data_meta_get_hash(mdi, data_meta);
>
> This will not work, because it is not implemented as a proper
> BPF-helper call.
>
> First, you currently store the xdp_md_info_arr inside the driver,
> which
> makes it hard for a helper to access this. For helper access, we
> could
> store this in xdp_rxq_info.
>
Good Idea!
> Second, in your design it looks like you are introducing a helper per
> possible item in xdp_md_info_arr. I think we can reduce this to a
> single helper, that takes a XDP_DATA_META_xxx flag, and returns an
> offset. (The helper could return a direct pointer, but I don't think
> the verfier can handle that, as it need to "see" this is related to
> data_meta pointer, and that we do the proper boundry checks.).
>
We can update the verifier to allow access to any offset between
data_meta and data_meta + offset(last meta data) + sizeof(last meta
data) ?
> The BPF prog already have direct memory access to the data_meta area,
> and all it really need is an offset. Sure, the XDP/bpf programmer
> could just calculate these offsets as constants, and remember to load
> the XDP prog with the flags that corresponds to the calculated
> offsets.
>
> But I think we can do something even smarter...
>
> It should be possible to convert/patch the BPF instructions, of the
> helper call that returns an offset, to instead avoid the call and
> either (1) provide the offset as a constant/IMM or (2) make BPF inst
> doing the lookup in xdp_md_info_arr.
>
I vote (2).
>
> > + if (hash + 1 > data)
> > + return XDP_ABORTED;
> > +
> > + vlan = xdp_data_meta_get_vlan(mdi, data_meta);
> > + if (vlan + 1 > data)
> > + return XDP_ABORTED;
> > +#endif
> > +
> > + /* Work around for the above code */
> > + hash = data_meta; /* since we know hash will appear first
> > */
> > + if (hash + 1 > data)
> > + return XDP_ABORTED;
> > +
> > +#if 0
> > + // Just for testing
> > + /* We know that vlan will appear after the hash */
> > + vlan = (void *)((char *)data_meta + sizeof(*hash));
> > + if (vlan + 1 > data) {
> > + return XDP_ABORTED;
> > + }
> > +#endif
>
>
^ permalink raw reply
* [PATCH iproute2] man: Fix typos on tc-cbs
From: Jesus Sanchez-Palencia @ 2018-06-27 17:50 UTC (permalink / raw)
To: netdev; +Cc: Jesus Sanchez-Palencia
Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
---
man/man8/tc-cbs.8 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/man/man8/tc-cbs.8 b/man/man8/tc-cbs.8
index 32e1e0d4..ad1d8821 100644
--- a/man/man8/tc-cbs.8
+++ b/man/man8/tc-cbs.8
@@ -28,7 +28,7 @@ defined rate limiting method to the traffic.
This queueing discipline is intended to be used by TSN (Time Sensitive
Networking) applications, the CBS parameters are derived directly by
what is described by the Annex L of the IEEE 802.1Q-2014
-Sepcification. The algorithm and how it affects the latency are
+Specification. The algorithm and how it affects the latency are
detailed there.
CBS is meant to be installed under another qdisc that maps packet
@@ -60,7 +60,7 @@ packet size, which is then used for calculating the idleslope.
sendslope
Sendslope is the rate of credits that is depleted (it should be a
negative number of kilobits per second) when a transmission is
-ocurring. It can be calculated as follows, (IEEE 802.1Q-2014 Section
+occurring. It can be calculated as follows, (IEEE 802.1Q-2014 Section
8.6.8.2 item g):
sendslope = idleslope - port_transmit_rate
--
2.17.1
^ permalink raw reply related
* Re: [RFC bpf-next 2/6] net: xdp: RX meta data infrastructure
From: Saeed Mahameed @ 2018-06-27 17:55 UTC (permalink / raw)
To: saeedm@dev.mellanox.co.il, brouer@redhat.com
Cc: alexander.h.duyck@intel.com, peter.waskiewicz.jr@intel.com,
Rony Efraim, borkmann@iogearbox.net, Tariq Toukan,
neerav.parikh@intel.com, Opher Reviv,
alexei.starovoitov@gmail.com, pjwaskiewicz@gmail.com,
netdev@vger.kernel.org, ttoukan.linux@gmail.com
In-Reply-To: <20180627161517.31f1f7af@redhat.com>
On Wed, 2018-06-27 at 16:15 +0200, Jesper Dangaard Brouer wrote:
> On Tue, 26 Jun 2018 19:46:11 -0700
> Saeed Mahameed <saeedm@dev.mellanox.co.il> wrote:
>
> > diff --git a/include/net/xdp.h b/include/net/xdp.h
> > index 2deea7166a34..afe302613ae1 100644
> > --- a/include/net/xdp.h
> > +++ b/include/net/xdp.h
> > @@ -138,6 +138,12 @@ xdp_set_data_meta_invalid(struct xdp_buff
> > *xdp)
> > xdp->data_meta = xdp->data + 1;
> > }
> >
> > +static __always_inline void
> > +xdp_reset_data_meta(struct xdp_buff *xdp)
> > +{
> > + xdp->data_meta = xdp->data_hard_start;
> > +}
>
> This is WRONG ... it should be:
>
> xdp->data_meta = xdp->data;
>
maybe the name of the function is not suitable for the use case.
i need to set xdp->data_meta = xdp->data in the driver to start storing
meta data.
^ permalink raw reply
* Re: [PATCH v7 11/11] net: pch_gbe: Allow build on MIPS platforms
From: Andrew Lunn @ 2018-06-27 17:54 UTC (permalink / raw)
To: Paul Burton; +Cc: netdev, David S . Miller
In-Reply-To: <20180627000612.27263-12-paul.burton@mips.com>
On Tue, Jun 26, 2018 at 05:06:12PM -0700, Paul Burton wrote:
> Allow the pch_gbe driver to be built on MIPS platforms, allowing its use
> on the MIPS Boston development board.
>
> Signed-off-by: Paul Burton <paul.burton@mips.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
>
> ---
>
> Changes in v7: None
>
> drivers/net/ethernet/oki-semi/pch_gbe/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig b/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig
> index 5276f4ff3b63..8e3630b9a9d1 100644
> --- a/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig
> +++ b/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig
> @@ -4,7 +4,7 @@
>
> config PCH_GBE
> tristate "OKI SEMICONDUCTOR IOH(ML7223/ML7831) GbE"
> - depends on PCI && (X86_32 || COMPILE_TEST)
> + depends on PCI && (X86_32 || MIPS || COMPILE_TEST)
Same question here as for the PTP driver.
Andrew
^ permalink raw reply
* Re: [PATCH v7 06/11] net: pch_gbe: Only enable MAC when PHY link is active
From: Florian Fainelli @ 2018-06-27 17:54 UTC (permalink / raw)
To: Paul Burton, netdev; +Cc: David S . Miller, Andrew Lunn
In-Reply-To: <20180627000612.27263-7-paul.burton@mips.com>
On 06/26/2018 05:06 PM, Paul Burton wrote:
> When using a PHY connected via RGMII, as the pch_gbe driver presumes is
> the case, the RX clock is provided by the PHY to the MAC. Various PHYs,
> including both the AR8031 used by the Minnowboard & the RTL8211E used by
> the MIPS Boston development board, will stop generating the RX clock
> when the ethernet link is down (eg. the ethernet cable is unplugged).
>
> Various pieces of functionality in the EG20T MAC, ranging from basics
> like completing a MAC reset to programming MAC addresses, rely upon the
> RX clock being provided. When the clock is not provided these pieces of
> functionality simply never complete, and the busy bits that indicate
> they're in progress remain set indefinitely.
>
> The pch_gbe driver currently requires that the RX clock is always
> provided, and attempts to enforce this by disabling the hibernation
> feature of the AR8031 PHY to keep it generating the RX clock. This patch
> moves us away from this model by only configuring the MAC when the PHY
> indicates that the ethernet link is up. When the link is up we should be
> able to safely expect that the RX clock is being provided, and therefore
> safely reset & configure the MAC.
What we ended up doing in the bcmgenet driver is loop back the RX and TX
clocks such that we always have a clock that we can use to perform any
MAC operation, including reset.
Is this an option here? You might also want to split the allocation from
the actual initialization if this is not done already.
>
> Signed-off-by: Paul Burton <paul.burton@mips.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> ---
>
> Changes in v7: New patch
>
> .../ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 44 +++++++++----------
> 1 file changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> index eb290c1edce0..721ce29b6467 100644
> --- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> +++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> @@ -1837,7 +1837,6 @@ static int pch_gbe_request_irq(struct pch_gbe_adapter *adapter)
> int pch_gbe_up(struct pch_gbe_adapter *adapter)
> {
> struct net_device *netdev = adapter->netdev;
> - struct pch_gbe_tx_ring *tx_ring = adapter->tx_ring;
> struct pch_gbe_rx_ring *rx_ring = adapter->rx_ring;
> int err = -EINVAL;
>
> @@ -1847,14 +1846,6 @@ int pch_gbe_up(struct pch_gbe_adapter *adapter)
> goto out;
> }
>
> - /* hardware has been reset, we need to reload some things */
> - pch_gbe_set_multi(netdev);
> -
> - pch_gbe_setup_tctl(adapter);
> - pch_gbe_configure_tx(adapter);
> - pch_gbe_setup_rctl(adapter);
> - pch_gbe_configure_rx(adapter);
> -
> err = pch_gbe_request_irq(adapter);
> if (err) {
> netdev_err(netdev,
> @@ -1867,18 +1858,9 @@ int pch_gbe_up(struct pch_gbe_adapter *adapter)
> "Error: can't bring device up - alloc rx buffers pool failed\n");
> goto freeirq;
> }
> - pch_gbe_alloc_tx_buffers(adapter, tx_ring);
> - pch_gbe_alloc_rx_buffers(adapter, rx_ring, rx_ring->count);
> adapter->tx_queue_len = netdev->tx_queue_len;
> - pch_gbe_enable_dma_rx(&adapter->hw);
> - pch_gbe_enable_mac_rx(&adapter->hw);
>
> mod_timer(&adapter->watchdog_timer, jiffies);
> -
> - napi_enable(&adapter->napi);
> - pch_gbe_irq_enable(adapter);
> - netif_start_queue(adapter->netdev);
> -
> return 0;
>
> freeirq:
> @@ -1930,6 +1912,8 @@ static void pch_gbe_watchdog(struct timer_list *t)
> {
> struct pch_gbe_adapter *adapter = from_timer(adapter, t,
> watchdog_timer);
> + struct pch_gbe_rx_ring *rx_ring = adapter->rx_ring;
> + struct pch_gbe_tx_ring *tx_ring = adapter->tx_ring;
> struct net_device *netdev = adapter->netdev;
> struct pch_gbe_hw *hw = &adapter->hw;
>
> @@ -1950,12 +1934,32 @@ static void pch_gbe_watchdog(struct timer_list *t)
> }
> hw->mac.link_speed = ethtool_cmd_speed(&cmd);
> hw->mac.link_duplex = cmd.duplex;
> +
> + pch_gbe_reset(adapter);
> +
> /* Set the RGMII control. */
> pch_gbe_set_rgmii_ctrl(adapter, hw->mac.link_speed,
> hw->mac.link_duplex);
> /* Set the communication mode */
> pch_gbe_set_mode(adapter, hw->mac.link_speed,
> hw->mac.link_duplex);
> +
> + pch_gbe_set_multi(netdev);
> + pch_gbe_setup_tctl(adapter);
> + pch_gbe_configure_tx(adapter);
> + pch_gbe_setup_rctl(adapter);
> + pch_gbe_configure_rx(adapter);
> +
> + pch_gbe_alloc_tx_buffers(adapter, tx_ring);
> + pch_gbe_alloc_rx_buffers(adapter, rx_ring, rx_ring->count);
> +
> + pch_gbe_enable_dma_rx(&adapter->hw);
> + pch_gbe_enable_mac_rx(&adapter->hw);
> +
> + napi_enable(&adapter->napi);
> + pch_gbe_irq_enable(adapter);
> + netif_start_queue(adapter->netdev);
> +
> netdev_dbg(netdev,
> "Link is Up %d Mbps %s-Duplex\n",
> hw->mac.link_speed,
> @@ -2568,7 +2572,6 @@ static int pch_gbe_probe(struct pci_dev *pdev,
> (ETH_HLEN + ETH_FCS_LEN);
>
> pch_gbe_mac_load_mac_addr(&adapter->hw);
> - pch_gbe_mac_reset_hw(&adapter->hw);
>
> /* setup the private structure */
> ret = pch_gbe_sw_init(adapter);
> @@ -2610,9 +2613,6 @@ static int pch_gbe_probe(struct pci_dev *pdev,
> adapter->wake_up_evt = PCH_GBE_WL_INIT_SETTING;
> dev_info(&pdev->dev, "MAC address : %pM\n", netdev->dev_addr);
>
> - /* reset the hardware with the new settings */
> - pch_gbe_reset(adapter);
> -
> ret = register_netdev(netdev);
> if (ret)
> goto err_free_adapter;
>
--
Florian
^ permalink raw reply
* Re: [PATCH v7 06/11] net: pch_gbe: Only enable MAC when PHY link is active
From: Paul Burton @ 2018-06-27 17:54 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, David S . Miller
In-Reply-To: <20180627173014.GD885@lunn.ch>
Hi Andrew,
On Wed, Jun 27, 2018 at 07:30:14PM +0200, Andrew Lunn wrote:
> On Tue, Jun 26, 2018 at 05:06:07PM -0700, Paul Burton wrote:
> > When using a PHY connected via RGMII, as the pch_gbe driver presumes is
> > the case, the RX clock is provided by the PHY to the MAC. Various PHYs,
> > including both the AR8031 used by the Minnowboard & the RTL8211E used by
> > the MIPS Boston development board, will stop generating the RX clock
> > when the ethernet link is down (eg. the ethernet cable is unplugged).
> >
> > Various pieces of functionality in the EG20T MAC, ranging from basics
> > like completing a MAC reset to programming MAC addresses, rely upon the
> > RX clock being provided. When the clock is not provided these pieces of
> > functionality simply never complete, and the busy bits that indicate
> > they're in progress remain set indefinitely.
> >
> > The pch_gbe driver currently requires that the RX clock is always
> > provided, and attempts to enforce this by disabling the hibernation
> > feature of the AR8031 PHY to keep it generating the RX clock. This patch
> > moves us away from this model by only configuring the MAC when the PHY
> > indicates that the ethernet link is up. When the link is up we should be
> > able to safely expect that the RX clock is being provided, and therefore
> > safely reset & configure the MAC.
>
> Hi Paul
>
> I like the concept, but the implementation is not clear. Maybe it just
> needs more details in the commit message. What has the watchdog got to
> do with link up?
pch_gbe_watchdog() polls for the link coming up or going down, so that's
where we find out that the link is up.
> And what happens on link down? Does the MAC need shutting down? I
> don't see such code here.
Well, depending upon the PHY the RX clock might stop which will prevent
parts of the MAC from functioning properly. Exactly which parts I don't
really know - the EG20T documentation is vague & unclear. I do know
that:
- We won't receive packets any more, of course. This should be fine
without any extra handling because we just won't see any futher DMA
complete interrupts (or the associated bit set when polling).
- A MAC reset won't complete - ie. the pch_gbe_wait_clr_bit() in
pch_gbe_reset()/pch_gbe_reset_hw() will time out. This I think
should be OK because after this patch we won't generally reset the
MAC when the link is down anyway, except perhaps the PCI error_state
case in pch_gbe_down(). I'm not sure what the reset there is for...
- Masking or unmasking MAC address registers won't complete - ie. the
pch_gbe_wait_clr_bit() in pch_gbe_mac_mar_set() or
pch_gbe_set_multi() will time out. This is again when the link is
already known to be up, although there is a case in
__pch_gbe_suspend() which is setting up WoL that I'm not so sure
about...
Thanks,
Paul
^ permalink raw reply
* Re: [PATCH v7 10/11] ptp: pch: Allow build on MIPS platforms
From: Andrew Lunn @ 2018-06-27 17:53 UTC (permalink / raw)
To: Paul Burton; +Cc: netdev, David S . Miller
In-Reply-To: <20180627000612.27263-11-paul.burton@mips.com>
On Tue, Jun 26, 2018 at 05:06:11PM -0700, Paul Burton wrote:
> Allow the ptp_pch driver to be built on MIPS platforms in preparation
> for use on the MIPS Boston board.
>
> Signed-off-by: Paul Burton <paul.burton@mips.com>
> Acked-by: Richard Cochran <richardcochran@gmail.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> ---
>
> Changes in v7: None
>
> drivers/ptp/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
> index d137c480db46..fd5f2c6c18ba 100644
> --- a/drivers/ptp/Kconfig
> +++ b/drivers/ptp/Kconfig
> @@ -90,7 +90,7 @@ config DP83640_PHY
>
> config PTP_1588_CLOCK_PCH
> tristate "Intel PCH EG20T as PTP clock"
> - depends on X86_32 || COMPILE_TEST
> + depends on X86_32 || MIPS || COMPILE_TEST
Hi Paul
Is there anything in the code which stops it working on S390? ARM?
X86_64? Can we just remove this depends?
Andrew
^ permalink raw reply
* Re: [PATCH v7 09/11] net: pch_gbe: Convert to mdiobus and phylib
From: Andrew Lunn @ 2018-06-27 17:51 UTC (permalink / raw)
To: Paul Burton; +Cc: netdev, David S . Miller
In-Reply-To: <20180627000612.27263-10-paul.burton@mips.com>
> @@ -5,7 +5,8 @@
> config PCH_GBE
> tristate "OKI SEMICONDUCTOR IOH(ML7223/ML7831) GbE"
> depends on PCI && (X86_32 || COMPILE_TEST)
> - select MII
> + select PHYLIB
> + imply AT803X_PHY if X86_32
> select PTP_1588_CLOCK_PCH
> select NET_PTP_CLASSIFY
That is unusual. I don't think any other MAC driver does this.
If the AT803X driver is not available, it will fall back to the
generic PHY driver. That means RGMII delays will not get set
correctly, no interrupts, no wol, and no workaround for the 8030.
Are any of these relevant to your board?
> @@ -197,16 +151,14 @@ static void pch_gbe_get_regs(struct net_device *netdev,
> struct pch_gbe_hw *hw = &adapter->hw;
> struct pci_dev *pdev = adapter->pdev;
> u32 *regs_buff = p;
> - u16 i, tmp;
> + u16 i;
>
> regs->version = 0x1000000 | (__u32)pdev->revision << 16 | pdev->device;
> for (i = 0; i < PCH_GBE_MAC_REGS_LEN; i++)
> *regs_buff++ = ioread32(&hw->reg->INT_ST + i);
> /* PHY register */
> - for (i = 0; i < PCH_GBE_PHY_REGS_LEN; i++) {
> - pch_gbe_phy_read_reg_miic(&adapter->hw, i, &tmp);
> - *regs_buff++ = tmp;
> - }
> + for (i = 0; i < PCH_GBE_PHY_REGS_LEN; i++)
> + *regs_buff++ = phy_read(adapter->phydev, i);
> }
In general, that is not safe. Some PHYs have pages, and you have no
idea what page is currently selected. If you don't need it, i would
drop this. There are other ways to get access to phy registers, like
miitool, which should do a better job.
Andrew
^ permalink raw reply
* Re: [Patch net-next v3 1/3] net: introduce helper dev_change_tx_queue_len()
From: Cong Wang @ 2018-06-27 17:41 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Linux Kernel Network Developers, John Fastabend
In-Reply-To: <e2d5e8ea-0096-1c2e-b7d4-76c1172900c3@gmail.com>
On Wed, Jun 27, 2018 at 9:14 AM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 01/25/2018 06:26 PM, Cong Wang wrote:
> > This patch promotes the local change_tx_queue_len() to a core
> > helper function, dev_change_tx_queue_len(), so that rtnetlink
> > and net-sysfs could share the code. This also prepares for the
> > following patch.
> >
> > Note, the -EFAULT in the original code doesn't make sense,
> > we should propagate the errno from notifiers.
> >
> > Cc: John Fastabend <john.fastabend@gmail.com>
> > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> > ---
> > include/linux/netdevice.h | 1 +
> > net/core/dev.c | 28 ++++++++++++++++++++++++++++
> > net/core/net-sysfs.c | 25 +------------------------
> > net/core/rtnetlink.c | 18 +++++-------------
> > 4 files changed, 35 insertions(+), 37 deletions(-)
> >
>
> Hi Cong
>
> What about using dev_change_tx_queue_len() helper from SIOCSIFTXQLEN path in
> net/core/dev_ioctl.c ?
>
> This would make sure we call dev_qdisc_change_tx_queue_len() in this case.
>
Good catch! Will send a patch for net-next.
Thanks.
^ permalink raw reply
* Official email
From: F Betrisey @ 2018-06-27 14:17 UTC (permalink / raw)
I’m counsel to late Henry who died in an auto crash with his immediate
family in February 2014, and left an estate of $18 million dollars. You
are entitled to his estate, Henry share same last name with you. Please
contact me, thank you.
^ permalink raw reply
* Re: [PATCH rdma-next 08/12] overflow.h: Add arithmetic shift helper
From: Leon Romanovsky @ 2018-06-27 17:39 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Jason Gunthorpe, Doug Ledford, Kees Cook, RDMA mailing list,
Hadar Hen Zion, Matan Barak, Michael J Ruhl, Noa Osherovich,
Raed Salem, Yishai Hadas, Saeed Mahameed, linux-netdev,
linux-kernel
In-Reply-To: <CAKwiHFgchr+6HYOZ4e4e1vzL9cFabe6eonNNM8NTWZypazcuKA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 939 bytes --]
On Wed, Jun 27, 2018 at 11:36:03AM +0200, Rasmus Villemoes wrote:
> On 26 June 2018 at 19:54, Jason Gunthorpe <jgg@mellanox.com> wrote:
>
> > On Tue, Jun 26, 2018 at 10:07:07AM +0200, Rasmus Villemoes wrote:
> > > On 25 June 2018 at 19:11, Jason Gunthorpe <[1]jgg@mellanox.com>
> > wrote:
> > >
> >
> > When thinking about signed cases.. The explicit u64 cast, and
> > implict promotion to typeof(d), produce something counter intuitive,
> > eg:
> >
> > (u64)(s32)-1 == 0xffffffffffffffff
> >
> > Which would result in a shift oucome that is not what anyone would
> > expect, IMHO... So the first version isn't what I'd expect either..
> >
>
> Wouldn't check_shift_overflow(-1, 4, &someint) just put -16 in someint and
> report no overflow? That's what I'd expect, if negative values are to be
> supported at all.
Most if not all the times we don't do shifts on negative values, so I
don't think that we should support them.
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* [PATCH v2 net-next 6/6] net sched actions: avoid bitwise operation on signed value in pedit
From: Roman Mashak @ 2018-06-27 17:33 UTC (permalink / raw)
To: davem; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak
In-Reply-To: <1530120815-13736-1-git-send-email-mrv@mojatatu.com>
Since char can be unsigned or signed, and bitwise operators may have
implementation-dependent results when performed on signed operands,
declare 'u8 *' operand instead.
Suggested-by: Davide Caratti <dcaratti@redhat.com>
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
net/sched/act_pedit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index caa6927a992c..ab151346d3d4 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -311,7 +311,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
}
if (tkey->offmask) {
- char *d, _d;
+ u8 *d, _d;
if (!offset_valid(skb, hoffset + tkey->at)) {
pr_info("tc action pedit 'at' offset %d out of bounds\n",
--
2.7.4
^ permalink raw reply related
* [PATCH v2 net-next 5/6] net sched actions: fix misleading text strings in pedit action
From: Roman Mashak @ 2018-06-27 17:33 UTC (permalink / raw)
To: davem; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak
In-Reply-To: <1530120815-13736-1-git-send-email-mrv@mojatatu.com>
Change "tc filter pedit .." to "tc actions pedit .." in error
messages to clearly refer to pedit action.
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
net/sched/act_pedit.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 3b775f54cee5..caa6927a992c 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -305,7 +305,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
rc = pedit_skb_hdr_offset(skb, htype, &hoffset);
if (rc) {
- pr_info("tc filter pedit bad header type specified (0x%x)\n",
+ pr_info("tc action pedit bad header type specified (0x%x)\n",
htype);
goto bad;
}
@@ -314,7 +314,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
char *d, _d;
if (!offset_valid(skb, hoffset + tkey->at)) {
- pr_info("tc filter pedit 'at' offset %d out of bounds\n",
+ pr_info("tc action pedit 'at' offset %d out of bounds\n",
hoffset + tkey->at);
goto bad;
}
@@ -326,12 +326,12 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
}
if (offset % 4) {
- pr_info("tc filter pedit offset must be on 32 bit boundaries\n");
+ pr_info("tc action pedit offset must be on 32 bit boundaries\n");
goto bad;
}
if (!offset_valid(skb, hoffset + offset)) {
- pr_info("tc filter pedit offset %d out of bounds\n",
+ pr_info("tc action pedit offset %d out of bounds\n",
hoffset + offset);
goto bad;
}
@@ -349,7 +349,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
val = (*ptr + tkey->val) & ~tkey->mask;
break;
default:
- pr_info("tc filter pedit bad command (%d)\n",
+ pr_info("tc action pedit bad command (%d)\n",
cmd);
goto bad;
}
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox