* Re: [PATCH net-next v3] bindings: net: stmmac: add missing note about LPI interrupt
From: Joao Pinto @ 2017-04-19 15:22 UTC (permalink / raw)
To: Niklas Cassel, Rob Herring, Mark Rutland, David S. Miller,
Joao Pinto, Niklas Cassel, Alexandre TORGUE, Giuseppe CAVALLARO,
Thierry Reding, Eric Engestrom
Cc: netdev, devicetree, linux-kernel
In-Reply-To: <20170418123955.21335-1-niklass@axis.com>
Hi Niklas,
Às 1:39 PM de 4/18/2017, Niklas Cassel escreveu:
> From: Niklas Cassel <niklas.cassel@axis.com>
>
> The hardware has a LPI interrupt.
> There is already code in the stmmac driver to parse and handle the
> interrupt. However, this information was missing from the DT binding.
>
> At the same time, improve the description of the existing interrupts.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
> Documentation/devicetree/bindings/net/stmmac.txt | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
> index f652b0c384ce..c3a7be6615c5 100644
> --- a/Documentation/devicetree/bindings/net/stmmac.txt
> +++ b/Documentation/devicetree/bindings/net/stmmac.txt
> @@ -7,9 +7,12 @@ Required properties:
> - interrupt-parent: Should be the phandle for the interrupt controller
> that services interrupts for this device
> - interrupts: Should contain the STMMAC interrupts
> -- interrupt-names: Should contain the interrupt names "macirq"
> - "eth_wake_irq" if this interrupt is supported in the "interrupts"
> - property
> +- interrupt-names: Should contain a list of interrupt names corresponding to
> + the interrupts in the interrupts property, if available.
> + Valid interrupt names are:
> + - "macirq" (combined signal for various interrupt events)
> + - "eth_wake_irq" (the interrupt to manage the remote wake-up packet detection)
> + - "eth_lpi" (the interrupt that occurs when Tx or Rx enters/exits LPI state)
> - phy-mode: See ethernet.txt file in the same directory.
> - snps,reset-gpio gpio number for phy reset.
> - snps,reset-active-low boolean flag to indicate if phy reset is active low.
> @@ -152,8 +155,8 @@ Examples:
> compatible = "st,spear600-gmac";
> reg = <0xe0800000 0x8000>;
> interrupt-parent = <&vic1>;
> - interrupts = <24 23>;
> - interrupt-names = "macirq", "eth_wake_irq";
> + interrupts = <24 23 22>;
> + interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
> mac-address = [000000000000]; /* Filled in by U-Boot */
> max-frame-size = <3800>;
> phy-mode = "gmii";
>
Acked-By: Joao Pinto <jpinto@synopsys.com>
Regards!
^ permalink raw reply
* Re: XDP question: best API for returning/setting egress port?
From: Jesper Dangaard Brouer @ 2017-04-19 15:24 UTC (permalink / raw)
To: Daniel Borkmann
Cc: John Fastabend, Daniel Borkmann, Alexei Starovoitov,
Alexei Starovoitov, netdev@vger.kernel.org,
xdp-newbies@vger.kernel.org, brouer
In-Reply-To: <58F75917.1050409@iogearbox.net>
On Wed, 19 Apr 2017 14:33:27 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 04/19/2017 02:00 PM, Jesper Dangaard Brouer wrote:
> > On Tue, 18 Apr 2017 13:54:45 -0700
> > John Fastabend <john.fastabend@gmail.com> wrote:
> >> On 17-04-18 12:58 PM, Jesper Dangaard Brouer wrote:
> >>>
> >>> As I argued in NetConf presentation[1] (from slide #9) we need a port
> >>> mapping table (instead of using ifindex'es). Both for supporting
> >>> other "port" types than net_devices (think sockets), and for
> >>> sandboxing what XDP can bypass.
> >>>
> >>> I want to create a new XDP action called XDP_REDIRECT, that instruct
> >>> XDP to send the xdp_buff to another "port" (get translated into a
> >>> net_device, or something else depending on internal port type).
> >>>
> >>> Looking at the userspace/eBPF interface, I'm wondering what is the
> >>> best API for "returning" this port number from eBPF?
> >>>
> >>> The options I see is:
> >>>
> >>> 1) Split-up the u32 action code, and e.g let the high-16-bit be the
> >>> port number and lower-16bit the (existing) action verdict.
> >>>
> >>> Pros: Simple API
> >>> Cons: Number of ports limited to 64K
> >>>
> >>> 2) Extend both xdp_buff + xdp_md to contain a (u32) port number, allow
> >>> eBPF to update xdp_md->port.
> >>>
> >>> Pros: Larger number of ports.
> >>> Cons: This require some ebpf translation steps between xdp_buff <-> xdp_md.
> >>> (see xdp_convert_ctx_access)
> >>>
> >>> 3) Extend only xdp_buff and create bpf_helper that set port in xdp_buff.
> >>>
> >>> Pros: Hides impl details, and allows helper to give eBPF code feedback
> >>> (on e.g. if port doesn't exist any longer)
> >>> Cons: Helper function call likely slower?
> >>
> >> How about doing this the same way redirect is done in the tc case? I have this
> >> patch under test,
> >>
> >> https://github.com/jrfastab/linux/commit/e78f5425d5e3c305b4170ddd85c61c2e15359fee
> >
> > I have been looking at this approach, which is close to option #3 above.
> >
> > The problem with your implementation that you use a per-cpu store.
> > This creates the problem of storing state between packets. First packet
> > can call helper bpf_xdp_redirect() setting an ifindex, but program can
> > still return XDP_PASS. Next packet can call XDP_REDIRECT and use the
> > ifindex set from the first packet. IMHO this is a problematic API to
> > expose.
> >
> > I do see that the TC interface that uses the same approach, via helper
> > bpf_redirect(). Maybe it have the same API problem? Looking at
> > sch_handle_ingress() I don't see this is handled (e.g. by always
> > clearing this_cpu_ptr(redirect_info)->ifindex = 0).
>
> It's cleared in {skb,xdp}_do_redirect() right after fetching the
> ifindex. I think this approach is just fine. The example described
> above is a misuse of the API by a buggy program calling bpf_xdp_redirect()
> and returning XDP_PASS while another time it returns XDP_REDIRECT
> without the bpf_xdp_redirect() helper, sounds very exotic, but it's
> as buggy as, say, a program doing the csum update wrong, a program
> writing the wrong data to the packet, doing adjust head on the wrong
> header offset, jumping into the wrong tail call entry and other things.
For TC I guess it is fine to keep it as is, because it is needed to
avoid extending skb. IHMO for XDP I see no reason to keep a
per-cpu-store (which besides will be slower), simply update
xdp_buff.port should be sufficient (which is only relevant for this
packet).
As noted in option#3, my concern is that calling a helper function call
will be slower, than simply returning the needed port info?
Maybe some bpf experts can tell me if such helper call could be
optimized out with some bpf magic?
> I think encoding this into an action code is rather limiting, f.e.
> where would we place a flags argument if needed in future? Would
> that mean, we need a XDP_REDIRECT2 return code that also allows for
> encoding flags?
Nope, it will be extensible.
We can start with:
struct xdp_ret {
union {
__u32 act;
struct {
__u16 action;
__u16 port;
};
};
And later change it to:
struct xdp_ret {
union {
__u32 act;
struct {
__u8 action;
__u8 flags;
__u16 port;
};
};
If actions does not go above 255. I would prefer that we start with
the latter, else people would argue that we need to extend the
structure like:
struct xdp_ret {
union {
__u32 act;
struct {
union {
__u16 action;
struct {
__u8 action2;
__u8 flags;
};
};
__u16 port;
};
};
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH net-next v4 1/2] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
From: Jamal Hadi Salim @ 2017-04-19 15:37 UTC (permalink / raw)
To: Jiri Pirko; +Cc: davem, netdev, eric.dumazet, xiyou.wangcong
In-Reply-To: <20170419131323.GE3357@nanopsycho.orion>
On 17-04-19 09:13 AM, Jiri Pirko wrote:
> Wed, Apr 19, 2017 at 03:03:59PM CEST, jhs@mojatatu.com wrote:
>> On 17-04-19 08:36 AM, Jiri Pirko wrote:
>>> Wed, Apr 19, 2017 at 01:57:29PM CEST, jhs@mojatatu.com wrote:
>>>> From: Jamal Hadi Salim <jhs@mojatatu.com>
>>
>>>> include/uapi/linux/rtnetlink.h | 21 +++++++++++++++++++--
>>>> net/sched/act_api.c | 43 ++++++++++++++++++++++++++++++++----------
>>>> 3 files changed, 53 insertions(+), 12 deletions(-)
>>>> +#define TCAA_MAX (__TCAA_MAX - 1)
>>>> #define TA_RTA(r) ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct tcamsg))))
>>>> #define TA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct tcamsg))
>>>> -#define TCA_ACT_TAB 1 /* attr type must be >=1 */
>>>> -#define TCAA_MAX 1
>>>> +#define TCA_ACT_TAB TCAA_ACT_TAB
>>>
>>> This is mess. What does "TCAA" stand for?
>>
>> TC Actions Attributes. What would you call it? I could have
>> called it TCA_ROOT etc. But maybe a comment to just call it
>> TC Actions Attributes would be enough?
>
> TCA_DUMP_X
>
> it is only for dumping. Naming it "attribute" seems weird. Same as if
> you have: int variable_something;
>
Jiri, this is not just for dumping. We are describing high level
attributes for tc actions.
>
>>
>>> I suggest some more meaningful naming of the enum items and define
>>> TCA_ACT_TAB and TCAA_MAX to the new values in order to maintain UAPI
>>
>>
>> Thats what the above does (for UAPI) maintainance, no?
>
> It does it for TCA_ACT_TAB. We need to do it for both TCA_ACT_TAB and TCAA_MAX
>
TCAA_XXX is the namespace selected. You dont like that name and
adding DUMP doesnt make sense to me. How about TCA_ACT_ROOT?
>>
>>> Also, please put X_MAX = __X_MAX - 1 into enum
>>
>> That is diverting from the norm which defines it outside
>> of the enum. A good reason could be: You, Jiri, plan to go and
>> cleanup all the netlink stuff which uses this style.
>> Or you think we should start a trend which leads us
>> to a new clean style.
>
> I would start now. I can take of the follow-up patch to change the rest.
>
It is a _lot_ of code to change! Note:
This is all the UAPI visible code (the same coding style for 20 years).
I am worried about this part.
>>>
>>>> +/* tcamsg flags stored in attribute TCAA_ACT_FLAGS
>>>> + *
>>>> + * ACT_LARGE_DUMP_ON user->kernel to request for larger than TCA_ACT_MAX_PRIO
>>>> + * actions in a dump. All dump responses will contain the number of actions
>>>> + * being dumped stored in for user app's consumption in TCAA_ACT_COUNT
>>>> + *
>>>> + */
>>>> +#define ACT_LARGE_DUMP_ON (1 << 0)
>>>
>>> Use "BIT(0)"
>>>
>>
>> Same question as before.
>
> Same answer :)
>
I will change this one - it is a lot simpler coding style
wide/wise than the other one.
>> Are you planning to cleanup the rest of the code which
>> So you are using 8 bits for one flag which requires one bit?
>> + the TLV header? Sounds like overkill.
>> Note: We dont need more than 1 or 2 bits for this case.
>> Even 32 bits is overkill for what I am doing.
>> When do i need to extend a single bit representation?
>
> I don't see any problem adding couple of bytes if it increases cleannes
> and easy extendability.
>
How do you extend one bit? Seriously. If i want to add another bit I
will add one more to existing bit map not 64 (T + L + 8bits + pad).
If i ran out of space i will add a new TLV.
>>>
>>>> struct net *net = sock_net(skb->sk);
>>>> - struct nlattr *tca[TCA_ACT_MAX + 1];
>>>> + struct nlattr *tca[TCAA_MAX + 1];
>>>
>>> This is certainly wrong.
>>>
>>
>> Why is it wrong?
>
> Because you use existing TCA_ACT_ attr enum.
>
Is there a programming mistake or you just dont like the name?
AFAIK, and based on my testing that code is correct.
>>>> - ret = nlmsg_parse(n, sizeof(struct tcamsg), tca, TCA_ACT_MAX, NULL,
>>>> + ret = nlmsg_parse(n, sizeof(struct tcamsg), tca, TCAA_MAX, tcaa_policy,
>>>
>>> This is certainly wrong.
>>>
>>
>> Same question as above.
>
> Same answer.
>
And same question still.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH net-next 1/2 v2] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
From: Jamal Hadi Salim @ 2017-04-19 15:43 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, netdev, jiri, xiyou.wangcong
In-Reply-To: <1492609668.10587.164.camel@edumazet-glaptop3.roam.corp.google.com>
On 17-04-19 09:47 AM, Eric Dumazet wrote:
> Try to _not_ use 32768 bytes for the recvmsg() sizes, but 4KB
>
> You pack XXX actions until 4KB skb is full.
>
> Then code does :
>
> nla_put_u32(skb, TCAA_ACT_COUNT, cb->args[1])
>
> This might fail, then you
>
> goto out_module_put;
>
>
> Then we are stuck ?
>
> What am I missing ?
>
>
Ok, I understand you now ;->
You are saying we need make sure there is at
least 64 bits reserved so we can stick this TLV in there.
We may have to review a lot of other dumping code in general
to make sure this doesnt happen. Thanks Eric.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH net-next v4 2/2] net sched actions: add time filter for action dumping
From: Jamal Hadi Salim @ 2017-04-19 15:47 UTC (permalink / raw)
To: Jiri Pirko; +Cc: davem, netdev, eric.dumazet, xiyou.wangcong
In-Reply-To: <20170419141419.GF3357@nanopsycho.orion>
On 17-04-19 10:14 AM, Jiri Pirko wrote:
> Wed, Apr 19, 2017 at 03:11:26PM CEST, jhs@mojatatu.com wrote:
> So it is hard coded. That confused me. I believe it would be good to
> provide actual tc patch that does the proper changes and include the
> example of that usage in the description - instead of "hackedtc"
>
Yes, the user sets it. I can make the diff available next time.
I didnt want to go on a discussion how to pass the command line
parameter in tc.
> TIME_DELTA?
>
sure.
Let me wait more comments then I will push the new update
by tommorow.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH net-next v4 1/2] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
From: Jiri Pirko @ 2017-04-19 15:54 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: davem, netdev, eric.dumazet, xiyou.wangcong
In-Reply-To: <e2a1e201-4830-e600-e668-91f83bcc9b32@mojatatu.com>
Wed, Apr 19, 2017 at 05:37:15PM CEST, jhs@mojatatu.com wrote:
>On 17-04-19 09:13 AM, Jiri Pirko wrote:
>> Wed, Apr 19, 2017 at 03:03:59PM CEST, jhs@mojatatu.com wrote:
>> > On 17-04-19 08:36 AM, Jiri Pirko wrote:
>> > > Wed, Apr 19, 2017 at 01:57:29PM CEST, jhs@mojatatu.com wrote:
>> > > > From: Jamal Hadi Salim <jhs@mojatatu.com>
>> >
>> > > > include/uapi/linux/rtnetlink.h | 21 +++++++++++++++++++--
>> > > > net/sched/act_api.c | 43 ++++++++++++++++++++++++++++++++----------
>> > > > 3 files changed, 53 insertions(+), 12 deletions(-)
>
>> > > > +#define TCAA_MAX (__TCAA_MAX - 1)
>> > > > #define TA_RTA(r) ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct tcamsg))))
>> > > > #define TA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct tcamsg))
>> > > > -#define TCA_ACT_TAB 1 /* attr type must be >=1 */
>> > > > -#define TCAA_MAX 1
>> > > > +#define TCA_ACT_TAB TCAA_ACT_TAB
>> > >
>> > > This is mess. What does "TCAA" stand for?
>> >
>> > TC Actions Attributes. What would you call it? I could have
>> > called it TCA_ROOT etc. But maybe a comment to just call it
>> > TC Actions Attributes would be enough?
>>
>> TCA_DUMP_X
>>
>> it is only for dumping. Naming it "attribute" seems weird. Same as if
>> you have: int variable_something;
>>
>
>Jiri, this is not just for dumping. We are describing high level
>attributes for tc actions.
This is already present:
enum {
TCA_ACT_UNSPEC,
TCA_ACT_KIND,
TCA_ACT_OPTIONS,
TCA_ACT_INDEX,
TCA_ACT_STATS,
TCA_ACT_PAD,
TCA_ACT_COOKIE,
__TCA_ACT_MAX
};
This is nested inside the dump message (TCAA_MAX, TCA_ACT_TAB)
Looks like you are mixing these 2.
>
>>
>> >
>> > > I suggest some more meaningful naming of the enum items and define
>> > > TCA_ACT_TAB and TCAA_MAX to the new values in order to maintain UAPI
>> >
>> >
>> > Thats what the above does (for UAPI) maintainance, no?
>>
>> It does it for TCA_ACT_TAB. We need to do it for both TCA_ACT_TAB and TCAA_MAX
>>
>
>TCAA_XXX is the namespace selected. You dont like that name and
>adding DUMP doesnt make sense to me. How about TCA_ACT_ROOT?
>
>> >
>> > > Also, please put X_MAX = __X_MAX - 1 into enum
>> >
>> > That is diverting from the norm which defines it outside
>> > of the enum. A good reason could be: You, Jiri, plan to go and
>> > cleanup all the netlink stuff which uses this style.
>> > Or you think we should start a trend which leads us
>> > to a new clean style.
>>
>> I would start now. I can take of the follow-up patch to change the rest.
>>
>
>It is a _lot_ of code to change! Note:
>This is all the UAPI visible code (the same coding style for 20 years).
>I am worried about this part.
We'll see. Lets do it in a sensitive way, in steps. But for new things,
I think it is good not to stick with old and outlived habits.
>
>> > >
>> > > > +/* tcamsg flags stored in attribute TCAA_ACT_FLAGS
>> > > > + *
>> > > > + * ACT_LARGE_DUMP_ON user->kernel to request for larger than TCA_ACT_MAX_PRIO
>> > > > + * actions in a dump. All dump responses will contain the number of actions
>> > > > + * being dumped stored in for user app's consumption in TCAA_ACT_COUNT
>> > > > + *
>> > > > + */
>> > > > +#define ACT_LARGE_DUMP_ON (1 << 0)
>> > >
>> > > Use "BIT(0)"
>> > >
>> >
>> > Same question as before.
>>
>> Same answer :)
>>
>
>I will change this one - it is a lot simpler coding style
>wide/wise than the other one.
>
>> > Are you planning to cleanup the rest of the code which
>
>> > So you are using 8 bits for one flag which requires one bit?
>> > + the TLV header? Sounds like overkill.
>> > Note: We dont need more than 1 or 2 bits for this case.
>> > Even 32 bits is overkill for what I am doing.
>> > When do i need to extend a single bit representation?
>>
>> I don't see any problem adding couple of bytes if it increases cleannes
>> and easy extendability.
>>
>
>How do you extend one bit? Seriously. If i want to add another bit I
>will add one more to existing bit map not 64 (T + L + 8bits + pad).
>If i ran out of space i will add a new TLV.
Netlink is TLV, should be used as TLV. I don't see how you can run out
any space. You tend to use Netlink in some weird hybrid mode, with only
argument being space. I think that couple of bytes wasted is not
a problem at all...
>
>> > >
>> > > > struct net *net = sock_net(skb->sk);
>> > > > - struct nlattr *tca[TCA_ACT_MAX + 1];
>> > > > + struct nlattr *tca[TCAA_MAX + 1];
>> > >
>> > > This is certainly wrong.
>> > >
>> >
>> > Why is it wrong?
>>
>> Because you use existing TCA_ACT_ attr enum.
>>
>
>Is there a programming mistake or you just dont like the name?
>AFAIK, and based on my testing that code is correct.
See my first comment. I think that you mix 2 things together.
>
>> > > > - ret = nlmsg_parse(n, sizeof(struct tcamsg), tca, TCA_ACT_MAX, NULL,
>> > > > + ret = nlmsg_parse(n, sizeof(struct tcamsg), tca, TCAA_MAX, tcaa_policy,
>> > >
>> > > This is certainly wrong.
>> > >
>> >
>> > Same question as above.
>>
>> Same answer.
>>
>
>And same question still.
>
>cheers,
>jamal
^ permalink raw reply
* Re: [PATCH net-next v4 2/2] net sched actions: add time filter for action dumping
From: Jiri Pirko @ 2017-04-19 15:55 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: davem, netdev, eric.dumazet, xiyou.wangcong
In-Reply-To: <c05c6e23-2008-2860-4aa9-e318a20db4d6@mojatatu.com>
Wed, Apr 19, 2017 at 05:47:21PM CEST, jhs@mojatatu.com wrote:
>On 17-04-19 10:14 AM, Jiri Pirko wrote:
>> Wed, Apr 19, 2017 at 03:11:26PM CEST, jhs@mojatatu.com wrote:
>
>> So it is hard coded. That confused me. I believe it would be good to
>> provide actual tc patch that does the proper changes and include the
>> example of that usage in the description - instead of "hackedtc"
>>
>
>Yes, the user sets it. I can make the diff available next time.
>I didnt want to go on a discussion how to pass the command line
>parameter in tc.
I think it is very good to send kernel and userspace part together.
Makes people to easily figure out what is going on and helps with
review.
>
>> TIME_DELTA?
>>
>
>sure.
>
>Let me wait more comments then I will push the new update
>by tommorow.
>
>cheers,
>jamal
^ permalink raw reply
* Re: [PATCH net-next v4 1/2] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
From: Jamal Hadi Salim @ 2017-04-19 16:07 UTC (permalink / raw)
To: Jiri Pirko; +Cc: davem, netdev, eric.dumazet, xiyou.wangcong
In-Reply-To: <20170419155400.GH3357@nanopsycho.orion>
On 17-04-19 11:54 AM, Jiri Pirko wrote:
> Wed, Apr 19, 2017 at 05:37:15PM CEST, jhs@mojatatu.com wrote:
>> On 17-04-19 09:13 AM, Jiri Pirko wrote:
>>> Wed, Apr 19, 2017 at 03:03:59PM CEST, jhs@mojatatu.com wrote:
>>>> On 17-04-19 08:36 AM, Jiri Pirko wrote:
>>>>> Wed, Apr 19, 2017 at 01:57:29PM CEST, jhs@mojatatu.com wrote:
>>>>>> From: Jamal Hadi Salim <jhs@mojatatu.com>
>>>>
>>>>>> include/uapi/linux/rtnetlink.h | 21 +++++++++++++++++++--
>>>>>> net/sched/act_api.c | 43 ++++++++++++++++++++++++++++++++----------
>>>>>> 3 files changed, 53 insertions(+), 12 deletions(-)
>>
>>>>>> +#define TCAA_MAX (__TCAA_MAX - 1)
>>>>>> #define TA_RTA(r) ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct tcamsg))))
>>>>>> #define TA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct tcamsg))
>>>>>> -#define TCA_ACT_TAB 1 /* attr type must be >=1 */
>>>>>> -#define TCAA_MAX 1
>>>>>> +#define TCA_ACT_TAB TCAA_ACT_TAB
>>>>>
>>>>> This is mess. What does "TCAA" stand for?
>>>>
>>>> TC Actions Attributes. What would you call it? I could have
>>>> called it TCA_ROOT etc. But maybe a comment to just call it
>>>> TC Actions Attributes would be enough?
>>>
>>> TCA_DUMP_X
>>>
>>> it is only for dumping. Naming it "attribute" seems weird. Same as if
>>> you have: int variable_something;
>>>
>>
>> Jiri, this is not just for dumping. We are describing high level
>> attributes for tc actions.
>
> This is already present:
> enum {
> TCA_ACT_UNSPEC,
> TCA_ACT_KIND,
> TCA_ACT_OPTIONS,
> TCA_ACT_INDEX,
> TCA_ACT_STATS,
> TCA_ACT_PAD,
> TCA_ACT_COOKIE,
> __TCA_ACT_MAX
> };
>
> This is nested inside the dump message (TCAA_MAX, TCA_ACT_TAB)
>
> Looks like you are mixing these 2.
>
No - That space is per action. The space i am defining is
above that in the the hierarchy. There used to be only
one attribute there (TCA_ACT_TAB) and now we are making
it more generic.
>
>
>> It is a _lot_ of code to change! Note:
>> This is all the UAPI visible code (the same coding style for 20 years).
>> I am worried about this part.
>
> We'll see. Lets do it in a sensitive way, in steps. But for new things,
> I think it is good not to stick with old and outlived habits.
>
>
I know you have the muscle to get it done - so fine, i will start
with this one change.
>
> Netlink is TLV, should be used as TLV. I don't see how you can run out
> any space. You tend to use Netlink in some weird hybrid mode, with only
> argument being space. I think that couple of bytes wasted is not
> a problem at all...
>
You are not making sense to me still.
What you describe as "a couple of bytes" adds up when you have
a large number of objects. I am trying to cut down on data back
and forth from user/kernel and a bitmap is a well understood entity.
Even if i did use a TLV - when i am representing flags which require one
bit each - it makes sense to use a bitmask encapsulated in a TLV. Not
to waste a TLV per bit.
cheers,
jamal
^ permalink raw reply
* Re: net: heap out-of-bounds in fib6_clean_node/rt6_fill_node/fib6_age/fib6_prune_clone
From: David Ahern @ 2017-04-19 16:09 UTC (permalink / raw)
To: Andrey Konovalov, Dmitry Vyukov
Cc: Eric Dumazet, Mahesh Bandewar, Eric Dumazet, David Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev, LKML, Cong Wang, syzkaller
In-Reply-To: <CAAeHK+zx6W_yxnoEQ2Pc1AT4uLXqYZaoi5oQBeuCntdGS38TQg@mail.gmail.com>
On 4/18/17 2:43 PM, Andrey Konovalov wrote:
> Hi!
>
> I've finally managed to reproduce one of the crashes on commit
> 4f7d029b9bf009fbee76bb10c0c4351a1870d2f3 (4.11-rc7).
>
> I'm not sure if this bug has the same root cause as the first one
> reported in this thread, but it definitely has to do with ipv6
> routing.
>
> C reproducer, syzkaller program and my .config are attached.
built a kernel with that config. booted the vm. ran the program. nada.
strace is showing:
clone(child_stack=0x72ffb0,
flags=CLONE_NEWUTS|CLONE_NEWUSER|CLONE_NEWPID|CLONE_NEWNET) = -1 EINVAL
(Invalid argument)
^ permalink raw reply
* Re: net: heap out-of-bounds in fib6_clean_node/rt6_fill_node/fib6_age/fib6_prune_clone
From: Andrey Konovalov @ 2017-04-19 16:12 UTC (permalink / raw)
To: David Ahern
Cc: Dmitry Vyukov, Eric Dumazet, Mahesh Bandewar, Eric Dumazet,
David Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev, LKML, Cong Wang, syzkaller
In-Reply-To: <580392d8-fa33-67bb-c5d6-27d262f1646a@cumulusnetworks.com>
[-- Attachment #1: Type: text/plain, Size: 845 bytes --]
On Wed, Apr 19, 2017 at 6:09 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
> On 4/18/17 2:43 PM, Andrey Konovalov wrote:
>> Hi!
>>
>> I've finally managed to reproduce one of the crashes on commit
>> 4f7d029b9bf009fbee76bb10c0c4351a1870d2f3 (4.11-rc7).
>>
>> I'm not sure if this bug has the same root cause as the first one
>> reported in this thread, but it definitely has to do with ipv6
>> routing.
>>
>> C reproducer, syzkaller program and my .config are attached.
>
> built a kernel with that config. booted the vm. ran the program. nada.
>
> strace is showing:
>
> clone(child_stack=0x72ffb0,
> flags=CLONE_NEWUTS|CLONE_NEWUSER|CLONE_NEWPID|CLONE_NEWNET) = -1 EINVAL
> (Invalid argument)
>
That's weird. I usually see this when I have CONFIG_USER_NS disabled.
Anyway, I just finished simplifying the reproducer. Give this one a try.
[-- Attachment #2: ip6_rt_cache_alloc-crash-poc2.c --]
[-- Type: text/x-csrc, Size: 1586 bytes --]
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <sys/mount.h>
#include <sys/prctl.h>
#include <sys/resource.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/time.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <linux/capability.h>
#include <linux/if.h>
#include <linux/if_tun.h>
#include <linux/kvm.h>
#include <linux/sched.h>
#include <net/if_arp.h>
#include <netinet/in.h>
#include <linux/ipv6_route.h>
#include <assert.h>
#include <dirent.h>
#include <errno.h>
#include <fcntl.h>
#include <grp.h>
#include <pthread.h>
#include <setjmp.h>
#include <signal.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
void loop() {
int s0 = socket(AF_INET6, SOCK_STREAM, 0x84);
struct in6_rtmsg rtmsg;
memset(&rtmsg.rtmsg_dst, 0, sizeof(rtmsg.rtmsg_dst));
rtmsg.rtmsg_src = in6addr_loopback;
memset(&rtmsg.rtmsg_gateway, 0, sizeof(rtmsg.rtmsg_gateway));
rtmsg.rtmsg_type = 0x4;
rtmsg.rtmsg_dst_len = 0x0000;
rtmsg.rtmsg_src_len = 0x7;
rtmsg.rtmsg_metric = 0x100;
rtmsg.rtmsg_info = 0x80000001;
rtmsg.rtmsg_flags = 0x40000000;
rtmsg.rtmsg_ifindex = if_nametoindex("gre0");
ioctl(s0, SIOCADDRT, &rtmsg);
int s1 = socket(AF_INET6, SOCK_RAW|SOCK_NONBLOCK, IPPROTO_RAW);
struct sockaddr_in6 si;
memset(&si, 0, sizeof(si));
char buffer[1];
sendto(s1, &buffer[0], 0, MSG_CONFIRM, (const struct sockaddr *)&si, sizeof(si));
}
int main() {
unshare(CLONE_NEWUSER);
unshare(CLONE_NEWNET);
loop();
return 0;
}
^ permalink raw reply
* Re: [PATCH net-next v4 1/2] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
From: Jiri Pirko @ 2017-04-19 16:17 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: davem, netdev, eric.dumazet, xiyou.wangcong
In-Reply-To: <56488055-1f4b-ee10-2357-5ce80f1fbc50@mojatatu.com>
Wed, Apr 19, 2017 at 06:07:48PM CEST, jhs@mojatatu.com wrote:
>On 17-04-19 11:54 AM, Jiri Pirko wrote:
>> Wed, Apr 19, 2017 at 05:37:15PM CEST, jhs@mojatatu.com wrote:
>> > On 17-04-19 09:13 AM, Jiri Pirko wrote:
>> > > Wed, Apr 19, 2017 at 03:03:59PM CEST, jhs@mojatatu.com wrote:
>> > > > On 17-04-19 08:36 AM, Jiri Pirko wrote:
>> > > > > Wed, Apr 19, 2017 at 01:57:29PM CEST, jhs@mojatatu.com wrote:
>> > > > > > From: Jamal Hadi Salim <jhs@mojatatu.com>
>> > > >
>> > > > > > include/uapi/linux/rtnetlink.h | 21 +++++++++++++++++++--
>> > > > > > net/sched/act_api.c | 43 ++++++++++++++++++++++++++++++++----------
>> > > > > > 3 files changed, 53 insertions(+), 12 deletions(-)
>> >
>> > > > > > +#define TCAA_MAX (__TCAA_MAX - 1)
>> > > > > > #define TA_RTA(r) ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct tcamsg))))
>> > > > > > #define TA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct tcamsg))
>> > > > > > -#define TCA_ACT_TAB 1 /* attr type must be >=1 */
>> > > > > > -#define TCAA_MAX 1
>> > > > > > +#define TCA_ACT_TAB TCAA_ACT_TAB
>> > > > >
>> > > > > This is mess. What does "TCAA" stand for?
>> > > >
>> > > > TC Actions Attributes. What would you call it? I could have
>> > > > called it TCA_ROOT etc. But maybe a comment to just call it
>> > > > TC Actions Attributes would be enough?
>> > >
>> > > TCA_DUMP_X
>> > >
>> > > it is only for dumping. Naming it "attribute" seems weird. Same as if
>> > > you have: int variable_something;
>> > >
>> >
>> > Jiri, this is not just for dumping. We are describing high level
>> > attributes for tc actions.
>>
>> This is already present:
>> enum {
>> TCA_ACT_UNSPEC,
>> TCA_ACT_KIND,
>> TCA_ACT_OPTIONS,
>> TCA_ACT_INDEX,
>> TCA_ACT_STATS,
>> TCA_ACT_PAD,
>> TCA_ACT_COOKIE,
>> __TCA_ACT_MAX
>> };
>>
>> This is nested inside the dump message (TCAA_MAX, TCA_ACT_TAB)
>>
>> Looks like you are mixing these 2.
>>
>
>No - That space is per action. The space i am defining is
>above that in the the hierarchy. There used to be only
>one attribute there (TCA_ACT_TAB) and now we are making
>it more generic.
Right. That's what I say. And that higher level is used only for
dumping. That's why I suggested TCA_ACT_DUMP prefix.
>
>>
>>
>
>> > It is a _lot_ of code to change! Note:
>> > This is all the UAPI visible code (the same coding style for 20 years).
>> > I am worried about this part.
>>
>> We'll see. Lets do it in a sensitive way, in steps. But for new things,
>> I think it is good not to stick with old and outlived habits.
>>
>>
>
>I know you have the muscle to get it done - so fine, i will start
>with this one change.
>
>>
>> Netlink is TLV, should be used as TLV. I don't see how you can run out
>> any space. You tend to use Netlink in some weird hybrid mode, with only
>> argument being space. I think that couple of bytes wasted is not
>> a problem at all...
>>
>
>You are not making sense to me still.
>What you describe as "a couple of bytes" adds up when you have
>a large number of objects. I am trying to cut down on data back
>and forth from user/kernel and a bitmap is a well understood entity.
The attributes in question are per-message, not per-object
>
>Even if i did use a TLV - when i am representing flags which require one
>bit each - it makes sense to use a bitmask encapsulated in a TLV. Not
>to waste a TLV per bit.
That is the only correct way. For example, in future the kernel may
report in extended ack that it does not support some specific attribute
user passed. If you pack it all in one attr, that would not be possible.
Also, what prevents user from passing random data on bit flag positions
that you are not using? Current kernel would ignore it. Next kernel will
do something different according to the flag bit. That is UAPI break.
Essentialy the same thing what DaveM said about the struct. You have to
define this completely, at the beginning, not possible to use the unused
bits for other things in the future.
^ permalink raw reply
* Re: [PATCH v3 1/2] net: phy: Fix PHY AN done state machine for interrupt driven PHYs
From: Florian Fainelli @ 2017-04-19 16:28 UTC (permalink / raw)
To: Roger Quadros, andrew
Cc: davem, kyle.roeschley, nsekhar, netdev, linux-kernel,
stable # v4 . 9+, Sergei Shtylyov, Andrew F. Davis,
Alexander Kochetkov
In-Reply-To: <203afff4-7dd7-e185-000a-1689a97dfb67@ti.com>
On 04/11/2017 04:17 AM, Roger Quadros wrote:
> Hi,
>
> On 28/03/17 13:05, Roger Quadros wrote:
>> +Andrew Davis & Sekhar.
>>
>> Hi,
>>
>> Andrew Davis posted a few comments offline which I'm clarifying here.
>>
>> On 27/03/17 14:59, Roger Quadros wrote:
>>> The Ethernet link on an interrupt driven PHY was not coming up if the
>>> Ethernet cable was plugged before the Ethernet interface was brought up.
>>>
>>> The PHY state machine seems to be stuck from RUNNING to AN state
>>> with no new interrupts from the PHY. So it doesn't know when the
>>> PHY Auto-negotiation has been completed and doesn't transition to RUNNING
>>> state with ANEG done thus netif_carrier_on() is never called.
>>>
>>> NOTE: genphy_config_aneg() will not restart PHY Auto-negotiation of
>>> advertisement parameters didn't change.
>>
>> Is phy->config_aneg expected to *always* restart auto-negotiation even if
>> advertisement parameters didn't change?
>> If so then we'll need to fix genphy_config_aneg().
>>
>>>
>>> Fix this by scheduling the PHY state machine in phy_start_aneg().
>>> There is no way of knowing in phy.c whether auto-negotiation was
>>> restarted or not by the PHY driver so we just wait for the next
>>> poll/interrupt to update the PHY state machine.
>>>
>>> Fixes: 3c293f4e08b5 ("net: phy: Trigger state machine on state change and not polling.")
>>> Cc: stable <stable@vger.kernel.org> # v4.9+
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>> v3: Fix typo in commit message
>>>
>>> drivers/net/phy/phy.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>> index 1be69d8..49dedf8 100644
>>> --- a/drivers/net/phy/phy.c
>>> +++ b/drivers/net/phy/phy.c
>>> @@ -630,6 +630,10 @@ int phy_start_aneg(struct phy_device *phydev)
>>>
>>> out_unlock:
>>> mutex_unlock(&phydev->lock);
>>> + if (!err && phy_interrupt_is_valid(phydev))
>>> + queue_delayed_work(system_power_efficient_wq,
>>> + &phydev->state_queue, HZ);
>>> +
>>> return err;
>>> }
>>> EXPORT_SYMBOL(phy_start_aneg);
>>>
>>
>> There is still room for optimization for interrupt driven PHYs as I still
>> see a delay of 1 second between "ifconfig ethx up" and link status coming up
>> if cable was already plugged in. In fact if Auto-negotiation was already completed
>> and not required to be restarted, the PHY state machine should have move from
>> AN to RUNNING instantly without expecting a PHY interrupt.
>>
>> How can we get rid of the unnecessary delay in the case where auto-negotiation
>> is not restarted?
>> Should we check for phy_aneg_done() immediately after issuing a phy_start_aneg()
>> in phy_state_machine() and switch from PHY_AN to PHY_RUNNING?
That sounds like a good idea yes. It seems to me like Alexander's patch
actually takes care of that:
http://patchwork.ozlabs.org/patch/752288/
Let's try to merge threads/recipients so we can shoot for a fix to be
included soon.
Thanks!
>>
>> This should avoid the need to re-schedule the state machine in phy_start_angeg().
>
> Any comments on my questions?
>
> cheers,
> -roger
>
--
Florian
^ permalink raw reply
* Re: net: heap out-of-bounds in fib6_clean_node/rt6_fill_node/fib6_age/fib6_prune_clone
From: David Ahern @ 2017-04-19 16:29 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Dmitry Vyukov, Eric Dumazet, Mahesh Bandewar, Eric Dumazet,
David Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev, LKML, Cong Wang, syzkaller
In-Reply-To: <CAAeHK+zZb=K4X9=NmSWQJL8bpgMbXoG+h4RasAyYDr+C5pkVYA@mail.gmail.com>
On 4/19/17 10:12 AM, Andrey Konovalov wrote:
> That's weird. I usually see this when I have CONFIG_USER_NS disabled.
I bungled the movement of .config between servers. reproduced. will
investigate.
^ permalink raw reply
* [PATCH net-next 0/2] ip_tunnel: Allow policy-based routing through tunnels
From: Craig Gallek @ 2017-04-19 16:30 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Craig Gallek <kraig@google.com>
iproute2 changes to follow. Example usage:
ip link add gre-test type gre local 10.0.0.1 remote 10.0.0.2 fwmark 0x4
ip -detail link show gre-test
...
ip link set gre-test type gre fwmark 0
Craig Gallek (2):
ip6_tunnel: Allow policy-based routing through tunnels
ip_tunnel: Allow policy-based routing through tunnels
include/net/ip6_tunnel.h | 2 ++
include/net/ip_tunnels.h | 5 +++--
include/uapi/linux/if_tunnel.h | 3 +++
net/ipv4/ip_gre.c | 24 +++++++++++++++++-------
net/ipv4/ip_tunnel.c | 27 +++++++++++++++++----------
net/ipv4/ip_vti.c | 20 +++++++++++++++-----
net/ipv4/ipip.c | 24 +++++++++++++++++-------
net/ipv6/ip6_gre.c | 14 +++++++++++++-
net/ipv6/ip6_tunnel.c | 15 ++++++++++++++-
net/ipv6/ip6_vti.c | 10 +++++++++-
net/ipv6/sit.c | 37 ++++++++++++++++++++++++-------------
11 files changed, 134 insertions(+), 47 deletions(-)
--
2.12.2.816.g2cccc81164-goog
^ permalink raw reply
* [PATCH net-next 1/2] ip6_tunnel: Allow policy-based routing through tunnels
From: Craig Gallek @ 2017-04-19 16:30 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20170419163054.31173-1-kraigatgoog@gmail.com>
From: Craig Gallek <kraig@google.com>
This feature allows the administrator to set an fwmark for
packets traversing a tunnel. This allows the use of independent
routing tables for tunneled packets without the use of iptables.
Signed-off-by: Craig Gallek <kraig@google.com>
---
include/net/ip6_tunnel.h | 2 ++
include/uapi/linux/if_tunnel.h | 3 +++
net/ipv6/ip6_gre.c | 14 +++++++++++++-
net/ipv6/ip6_tunnel.c | 15 ++++++++++++++-
net/ipv6/ip6_vti.c | 10 +++++++++-
5 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/include/net/ip6_tunnel.h b/include/net/ip6_tunnel.h
index 1b1cf33cbfb0..08fbc7f7d8d7 100644
--- a/include/net/ip6_tunnel.h
+++ b/include/net/ip6_tunnel.h
@@ -33,6 +33,8 @@ struct __ip6_tnl_parm {
__be16 o_flags;
__be32 i_key;
__be32 o_key;
+
+ __u32 fwmark;
};
/* IPv6 tunnel */
diff --git a/include/uapi/linux/if_tunnel.h b/include/uapi/linux/if_tunnel.h
index 92f3c8677523..6792d1967d31 100644
--- a/include/uapi/linux/if_tunnel.h
+++ b/include/uapi/linux/if_tunnel.h
@@ -75,6 +75,7 @@ enum {
IFLA_IPTUN_ENCAP_SPORT,
IFLA_IPTUN_ENCAP_DPORT,
IFLA_IPTUN_COLLECT_METADATA,
+ IFLA_IPTUN_FWMARK,
__IFLA_IPTUN_MAX,
};
#define IFLA_IPTUN_MAX (__IFLA_IPTUN_MAX - 1)
@@ -132,6 +133,7 @@ enum {
IFLA_GRE_ENCAP_DPORT,
IFLA_GRE_COLLECT_METADATA,
IFLA_GRE_IGNORE_DF,
+ IFLA_GRE_FWMARK,
__IFLA_GRE_MAX,
};
@@ -147,6 +149,7 @@ enum {
IFLA_VTI_OKEY,
IFLA_VTI_LOCAL,
IFLA_VTI_REMOTE,
+ IFLA_VTI_FWMARK,
__IFLA_VTI_MAX,
};
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 6fcb7cb49bb2..8d128ba79b66 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -544,6 +544,8 @@ static inline int ip6gre_xmit_ipv4(struct sk_buff *skb, struct net_device *dev)
& IPV6_TCLASS_MASK;
if (t->parms.flags & IP6_TNL_F_USE_ORIG_FWMARK)
fl6.flowi6_mark = skb->mark;
+ else
+ fl6.flowi6_mark = t->parms.fwmark;
fl6.flowi6_uid = sock_net_uid(dev_net(dev), NULL);
@@ -603,6 +605,8 @@ static inline int ip6gre_xmit_ipv6(struct sk_buff *skb, struct net_device *dev)
fl6.flowlabel |= ip6_flowlabel(ipv6h);
if (t->parms.flags & IP6_TNL_F_USE_ORIG_FWMARK)
fl6.flowi6_mark = skb->mark;
+ else
+ fl6.flowi6_mark = t->parms.fwmark;
fl6.flowi6_uid = sock_net_uid(dev_net(dev), NULL);
@@ -780,6 +784,7 @@ static int ip6gre_tnl_change(struct ip6_tnl *t,
t->parms.o_key = p->o_key;
t->parms.i_flags = p->i_flags;
t->parms.o_flags = p->o_flags;
+ t->parms.fwmark = p->fwmark;
dst_cache_reset(&t->dst_cache);
ip6gre_tnl_link_config(t, set_mtu);
return 0;
@@ -1249,6 +1254,9 @@ static void ip6gre_netlink_parms(struct nlattr *data[],
if (data[IFLA_GRE_FLAGS])
parms->flags = nla_get_u32(data[IFLA_GRE_FLAGS]);
+
+ if (data[IFLA_GRE_FWMARK])
+ parms->fwmark = nla_get_u32(data[IFLA_GRE_FWMARK]);
}
static int ip6gre_tap_init(struct net_device *dev)
@@ -1470,6 +1478,8 @@ static size_t ip6gre_get_size(const struct net_device *dev)
nla_total_size(2) +
/* IFLA_GRE_ENCAP_DPORT */
nla_total_size(2) +
+ /* IFLA_GRE_FWMARK */
+ nla_total_size(4) +
0;
}
@@ -1490,7 +1500,8 @@ static int ip6gre_fill_info(struct sk_buff *skb, const struct net_device *dev)
nla_put_u8(skb, IFLA_GRE_TTL, p->hop_limit) ||
nla_put_u8(skb, IFLA_GRE_ENCAP_LIMIT, p->encap_limit) ||
nla_put_be32(skb, IFLA_GRE_FLOWINFO, p->flowinfo) ||
- nla_put_u32(skb, IFLA_GRE_FLAGS, p->flags))
+ nla_put_u32(skb, IFLA_GRE_FLAGS, p->flags) ||
+ nla_put_u32(skb, IFLA_GRE_FWMARK, p->fwmark))
goto nla_put_failure;
if (nla_put_u16(skb, IFLA_GRE_ENCAP_TYPE,
@@ -1525,6 +1536,7 @@ static const struct nla_policy ip6gre_policy[IFLA_GRE_MAX + 1] = {
[IFLA_GRE_ENCAP_FLAGS] = { .type = NLA_U16 },
[IFLA_GRE_ENCAP_SPORT] = { .type = NLA_U16 },
[IFLA_GRE_ENCAP_DPORT] = { .type = NLA_U16 },
+ [IFLA_GRE_FWMARK] = { .type = NLA_U32 },
};
static struct rtnl_link_ops ip6gre_link_ops __read_mostly = {
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 75fac933c209..ad15d38b41e8 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -1256,6 +1256,8 @@ ip4ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
& IPV6_TCLASS_MASK;
if (t->parms.flags & IP6_TNL_F_USE_ORIG_FWMARK)
fl6.flowi6_mark = skb->mark;
+ else
+ fl6.flowi6_mark = t->parms.fwmark;
}
fl6.flowi6_uid = sock_net_uid(dev_net(dev), NULL);
@@ -1338,6 +1340,8 @@ ip6ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
fl6.flowlabel |= ip6_flowlabel(ipv6h);
if (t->parms.flags & IP6_TNL_F_USE_ORIG_FWMARK)
fl6.flowi6_mark = skb->mark;
+ else
+ fl6.flowi6_mark = t->parms.fwmark;
}
fl6.flowi6_uid = sock_net_uid(dev_net(dev), NULL);
@@ -1467,6 +1471,7 @@ ip6_tnl_change(struct ip6_tnl *t, const struct __ip6_tnl_parm *p)
t->parms.flowinfo = p->flowinfo;
t->parms.link = p->link;
t->parms.proto = p->proto;
+ t->parms.fwmark = p->fwmark;
dst_cache_reset(&t->dst_cache);
ip6_tnl_link_config(t);
return 0;
@@ -1918,6 +1923,9 @@ static void ip6_tnl_netlink_parms(struct nlattr *data[],
if (data[IFLA_IPTUN_COLLECT_METADATA])
parms->collect_md = true;
+
+ if (data[IFLA_IPTUN_FWMARK])
+ parms->fwmark = nla_get_u32(data[IFLA_IPTUN_FWMARK]);
}
static bool ip6_tnl_netlink_encap_parms(struct nlattr *data[],
@@ -2054,6 +2062,8 @@ static size_t ip6_tnl_get_size(const struct net_device *dev)
nla_total_size(2) +
/* IFLA_IPTUN_COLLECT_METADATA */
nla_total_size(0) +
+ /* IFLA_IPTUN_FWMARK */
+ nla_total_size(4) +
0;
}
@@ -2069,7 +2079,8 @@ static int ip6_tnl_fill_info(struct sk_buff *skb, const struct net_device *dev)
nla_put_u8(skb, IFLA_IPTUN_ENCAP_LIMIT, parm->encap_limit) ||
nla_put_be32(skb, IFLA_IPTUN_FLOWINFO, parm->flowinfo) ||
nla_put_u32(skb, IFLA_IPTUN_FLAGS, parm->flags) ||
- nla_put_u8(skb, IFLA_IPTUN_PROTO, parm->proto))
+ nla_put_u8(skb, IFLA_IPTUN_PROTO, parm->proto) ||
+ nla_put_u32(skb, IFLA_IPTUN_FWMARK, parm->fwmark))
goto nla_put_failure;
if (nla_put_u16(skb, IFLA_IPTUN_ENCAP_TYPE, tunnel->encap.type) ||
@@ -2081,6 +2092,7 @@ static int ip6_tnl_fill_info(struct sk_buff *skb, const struct net_device *dev)
if (parm->collect_md)
if (nla_put_flag(skb, IFLA_IPTUN_COLLECT_METADATA))
goto nla_put_failure;
+
return 0;
nla_put_failure:
@@ -2109,6 +2121,7 @@ static const struct nla_policy ip6_tnl_policy[IFLA_IPTUN_MAX + 1] = {
[IFLA_IPTUN_ENCAP_SPORT] = { .type = NLA_U16 },
[IFLA_IPTUN_ENCAP_DPORT] = { .type = NLA_U16 },
[IFLA_IPTUN_COLLECT_METADATA] = { .type = NLA_FLAG },
+ [IFLA_IPTUN_FWMARK] = { .type = NLA_U32 },
};
static struct rtnl_link_ops ip6_link_ops __read_mostly = {
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index 3d8a3b63b4fd..d67ef56454b2 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -657,6 +657,7 @@ vti6_tnl_change(struct ip6_tnl *t, const struct __ip6_tnl_parm *p)
t->parms.i_key = p->i_key;
t->parms.o_key = p->o_key;
t->parms.proto = p->proto;
+ t->parms.fwmark = p->fwmark;
dst_cache_reset(&t->dst_cache);
vti6_link_config(t);
return 0;
@@ -933,6 +934,9 @@ static void vti6_netlink_parms(struct nlattr *data[],
if (data[IFLA_VTI_OKEY])
parms->o_key = nla_get_be32(data[IFLA_VTI_OKEY]);
+
+ if (data[IFLA_VTI_FWMARK])
+ parms->fwmark = nla_get_u32(data[IFLA_VTI_FWMARK]);
}
static int vti6_newlink(struct net *src_net, struct net_device *dev,
@@ -998,6 +1002,8 @@ static size_t vti6_get_size(const struct net_device *dev)
nla_total_size(4) +
/* IFLA_VTI_OKEY */
nla_total_size(4) +
+ /* IFLA_VTI_FWMARK */
+ nla_total_size(4) +
0;
}
@@ -1010,7 +1016,8 @@ static int vti6_fill_info(struct sk_buff *skb, const struct net_device *dev)
nla_put_in6_addr(skb, IFLA_VTI_LOCAL, &parm->laddr) ||
nla_put_in6_addr(skb, IFLA_VTI_REMOTE, &parm->raddr) ||
nla_put_be32(skb, IFLA_VTI_IKEY, parm->i_key) ||
- nla_put_be32(skb, IFLA_VTI_OKEY, parm->o_key))
+ nla_put_be32(skb, IFLA_VTI_OKEY, parm->o_key) ||
+ nla_put_u32(skb, IFLA_VTI_FWMARK, parm->fwmark))
goto nla_put_failure;
return 0;
@@ -1024,6 +1031,7 @@ static const struct nla_policy vti6_policy[IFLA_VTI_MAX + 1] = {
[IFLA_VTI_REMOTE] = { .len = sizeof(struct in6_addr) },
[IFLA_VTI_IKEY] = { .type = NLA_U32 },
[IFLA_VTI_OKEY] = { .type = NLA_U32 },
+ [IFLA_VTI_FWMARK] = { .type = NLA_U32 },
};
static struct rtnl_link_ops vti6_link_ops __read_mostly = {
--
2.12.2.816.g2cccc81164-goog
^ permalink raw reply related
* [PATCH net-next 2/2] ip_tunnel: Allow policy-based routing through tunnels
From: Craig Gallek @ 2017-04-19 16:30 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20170419163054.31173-1-kraigatgoog@gmail.com>
From: Craig Gallek <kraig@google.com>
This feature allows the administrator to set an fwmark for
packets traversing a tunnel. This allows the use of independent
routing tables for tunneled packets without the use of iptables.
There is no concept of per-packet routing decisions through IPv4
tunnels, so this implementation does not need to work with
per-packet route lookups as the v6 implementation may
(with IP6_TNL_F_USE_ORIG_FWMARK).
Further, since the v4 tunnel ioctls share datastructures
(which can not be trivially modified) with the kernel's internal
tunnel configuration structures, the mark attribute must be stored
in the tunnel structure itself and passed as a parameter when
creating or changing tunnel attributes.
Signed-off-by: Craig Gallek <kraig@google.com>
---
include/net/ip_tunnels.h | 5 +++--
net/ipv4/ip_gre.c | 24 +++++++++++++++++-------
net/ipv4/ip_tunnel.c | 27 +++++++++++++++++----------
net/ipv4/ip_vti.c | 20 +++++++++++++++-----
net/ipv4/ipip.c | 24 +++++++++++++++++-------
net/ipv6/sit.c | 37 ++++++++++++++++++++++++-------------
6 files changed, 93 insertions(+), 44 deletions(-)
diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index 95056796657c..520809912f03 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -132,6 +132,7 @@ struct ip_tunnel {
unsigned int prl_count; /* # of entries in PRL */
unsigned int ip_tnl_net_id;
struct gro_cells gro_cells;
+ __u32 fwmark;
bool collect_md;
bool ignore_df;
};
@@ -273,9 +274,9 @@ int ip_tunnel_rcv(struct ip_tunnel *tunnel, struct sk_buff *skb,
const struct tnl_ptk_info *tpi, struct metadata_dst *tun_dst,
bool log_ecn_error);
int ip_tunnel_changelink(struct net_device *dev, struct nlattr *tb[],
- struct ip_tunnel_parm *p);
+ struct ip_tunnel_parm *p, __u32 fwmark);
int ip_tunnel_newlink(struct net_device *dev, struct nlattr *tb[],
- struct ip_tunnel_parm *p);
+ struct ip_tunnel_parm *p, __u32 fwmark);
void ip_tunnel_setup(struct net_device *dev, unsigned int net_id);
struct ip_tunnel_encap_ops {
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index c9c1cb635d9a..e90c80a548ad 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -829,7 +829,8 @@ static int ipgre_tap_validate(struct nlattr *tb[], struct nlattr *data[])
static int ipgre_netlink_parms(struct net_device *dev,
struct nlattr *data[],
struct nlattr *tb[],
- struct ip_tunnel_parm *parms)
+ struct ip_tunnel_parm *parms,
+ __u32 *fwmark)
{
struct ip_tunnel *t = netdev_priv(dev);
@@ -886,6 +887,9 @@ static int ipgre_netlink_parms(struct net_device *dev,
t->ignore_df = !!nla_get_u8(data[IFLA_GRE_IGNORE_DF]);
}
+ if (data[IFLA_GRE_FWMARK])
+ *fwmark = nla_get_u32(data[IFLA_GRE_FWMARK]);
+
return 0;
}
@@ -957,6 +961,7 @@ static int ipgre_newlink(struct net *src_net, struct net_device *dev,
{
struct ip_tunnel_parm p;
struct ip_tunnel_encap ipencap;
+ __u32 fwmark = 0;
int err;
if (ipgre_netlink_encap_parms(data, &ipencap)) {
@@ -967,31 +972,32 @@ static int ipgre_newlink(struct net *src_net, struct net_device *dev,
return err;
}
- err = ipgre_netlink_parms(dev, data, tb, &p);
+ err = ipgre_netlink_parms(dev, data, tb, &p, &fwmark);
if (err < 0)
return err;
- return ip_tunnel_newlink(dev, tb, &p);
+ return ip_tunnel_newlink(dev, tb, &p, fwmark);
}
static int ipgre_changelink(struct net_device *dev, struct nlattr *tb[],
struct nlattr *data[])
{
+ struct ip_tunnel *t = netdev_priv(dev);
struct ip_tunnel_parm p;
struct ip_tunnel_encap ipencap;
+ __u32 fwmark = t->fwmark;
int err;
if (ipgre_netlink_encap_parms(data, &ipencap)) {
- struct ip_tunnel *t = netdev_priv(dev);
err = ip_tunnel_encap_setup(t, &ipencap);
if (err < 0)
return err;
}
- err = ipgre_netlink_parms(dev, data, tb, &p);
+ err = ipgre_netlink_parms(dev, data, tb, &p, &fwmark);
if (err < 0)
return err;
- return ip_tunnel_changelink(dev, tb, &p);
+ return ip_tunnel_changelink(dev, tb, &p, fwmark);
}
static size_t ipgre_get_size(const struct net_device *dev)
@@ -1029,6 +1035,8 @@ static size_t ipgre_get_size(const struct net_device *dev)
nla_total_size(0) +
/* IFLA_GRE_IGNORE_DF */
nla_total_size(1) +
+ /* IFLA_GRE_FWMARK */
+ nla_total_size(4) +
0;
}
@@ -1049,7 +1057,8 @@ static int ipgre_fill_info(struct sk_buff *skb, const struct net_device *dev)
nla_put_u8(skb, IFLA_GRE_TTL, p->iph.ttl) ||
nla_put_u8(skb, IFLA_GRE_TOS, p->iph.tos) ||
nla_put_u8(skb, IFLA_GRE_PMTUDISC,
- !!(p->iph.frag_off & htons(IP_DF))))
+ !!(p->iph.frag_off & htons(IP_DF))) ||
+ nla_put_u32(skb, IFLA_GRE_FWMARK, t->fwmark))
goto nla_put_failure;
if (nla_put_u16(skb, IFLA_GRE_ENCAP_TYPE,
@@ -1093,6 +1102,7 @@ static const struct nla_policy ipgre_policy[IFLA_GRE_MAX + 1] = {
[IFLA_GRE_ENCAP_DPORT] = { .type = NLA_U16 },
[IFLA_GRE_COLLECT_METADATA] = { .type = NLA_FLAG },
[IFLA_GRE_IGNORE_DF] = { .type = NLA_U8 },
+ [IFLA_GRE_FWMARK] = { .type = NLA_U32 },
};
static struct rtnl_link_ops ipgre_link_ops __read_mostly = {
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 823abaef006b..b878ecbc0608 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -293,7 +293,8 @@ static struct net_device *__ip_tunnel_create(struct net *net,
static inline void init_tunnel_flow(struct flowi4 *fl4,
int proto,
__be32 daddr, __be32 saddr,
- __be32 key, __u8 tos, int oif)
+ __be32 key, __u8 tos, int oif,
+ __u32 mark)
{
memset(fl4, 0, sizeof(*fl4));
fl4->flowi4_oif = oif;
@@ -302,6 +303,7 @@ static inline void init_tunnel_flow(struct flowi4 *fl4,
fl4->flowi4_tos = tos;
fl4->flowi4_proto = proto;
fl4->fl4_gre_key = key;
+ fl4->flowi4_mark = mark;
}
static int ip_tunnel_bind_dev(struct net_device *dev)
@@ -322,7 +324,8 @@ static int ip_tunnel_bind_dev(struct net_device *dev)
init_tunnel_flow(&fl4, iph->protocol, iph->daddr,
iph->saddr, tunnel->parms.o_key,
- RT_TOS(iph->tos), tunnel->parms.link);
+ RT_TOS(iph->tos), tunnel->parms.link,
+ tunnel->fwmark);
rt = ip_route_output_key(tunnel->net, &fl4);
if (!IS_ERR(rt)) {
@@ -578,7 +581,7 @@ void ip_md_tunnel_xmit(struct sk_buff *skb, struct net_device *dev, u8 proto)
tos = ipv6_get_dsfield((const struct ipv6hdr *)inner_iph);
}
init_tunnel_flow(&fl4, proto, key->u.ipv4.dst, key->u.ipv4.src, 0,
- RT_TOS(tos), tunnel->parms.link);
+ RT_TOS(tos), tunnel->parms.link, tunnel->fwmark);
if (tunnel->encap.type != TUNNEL_ENCAP_NONE)
goto tx_error;
rt = ip_route_output_key(tunnel->net, &fl4);
@@ -707,7 +710,8 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
}
init_tunnel_flow(&fl4, protocol, dst, tnl_params->saddr,
- tunnel->parms.o_key, RT_TOS(tos), tunnel->parms.link);
+ tunnel->parms.o_key, RT_TOS(tos), tunnel->parms.link,
+ tunnel->fwmark);
if (ip_tunnel_encap(skb, tunnel, &protocol, &fl4) < 0)
goto tx_error;
@@ -795,7 +799,8 @@ static void ip_tunnel_update(struct ip_tunnel_net *itn,
struct ip_tunnel *t,
struct net_device *dev,
struct ip_tunnel_parm *p,
- bool set_mtu)
+ bool set_mtu,
+ __u32 fwmark)
{
ip_tunnel_del(itn, t);
t->parms.iph.saddr = p->iph.saddr;
@@ -812,10 +817,11 @@ static void ip_tunnel_update(struct ip_tunnel_net *itn,
t->parms.iph.tos = p->iph.tos;
t->parms.iph.frag_off = p->iph.frag_off;
- if (t->parms.link != p->link) {
+ if (t->parms.link != p->link || t->fwmark != fwmark) {
int mtu;
t->parms.link = p->link;
+ t->fwmark = fwmark;
mtu = ip_tunnel_bind_dev(dev);
if (set_mtu)
dev->mtu = mtu;
@@ -893,7 +899,7 @@ int ip_tunnel_ioctl(struct net_device *dev, struct ip_tunnel_parm *p, int cmd)
if (t) {
err = 0;
- ip_tunnel_update(itn, t, dev, p, true);
+ ip_tunnel_update(itn, t, dev, p, true, 0);
} else {
err = -ENOENT;
}
@@ -1066,7 +1072,7 @@ void ip_tunnel_delete_net(struct ip_tunnel_net *itn, struct rtnl_link_ops *ops)
EXPORT_SYMBOL_GPL(ip_tunnel_delete_net);
int ip_tunnel_newlink(struct net_device *dev, struct nlattr *tb[],
- struct ip_tunnel_parm *p)
+ struct ip_tunnel_parm *p, __u32 fwmark)
{
struct ip_tunnel *nt;
struct net *net = dev_net(dev);
@@ -1087,6 +1093,7 @@ int ip_tunnel_newlink(struct net_device *dev, struct nlattr *tb[],
nt->net = net;
nt->parms = *p;
+ nt->fwmark = fwmark;
err = register_netdevice(dev);
if (err)
goto out;
@@ -1105,7 +1112,7 @@ int ip_tunnel_newlink(struct net_device *dev, struct nlattr *tb[],
EXPORT_SYMBOL_GPL(ip_tunnel_newlink);
int ip_tunnel_changelink(struct net_device *dev, struct nlattr *tb[],
- struct ip_tunnel_parm *p)
+ struct ip_tunnel_parm *p, __u32 fwmark)
{
struct ip_tunnel *t;
struct ip_tunnel *tunnel = netdev_priv(dev);
@@ -1137,7 +1144,7 @@ int ip_tunnel_changelink(struct net_device *dev, struct nlattr *tb[],
}
}
- ip_tunnel_update(itn, t, dev, p, !tb[IFLA_MTU]);
+ ip_tunnel_update(itn, t, dev, p, !tb[IFLA_MTU], fwmark);
return 0;
}
EXPORT_SYMBOL_GPL(ip_tunnel_changelink);
diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index 8b14f1404c8f..40977413fd48 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -471,7 +471,8 @@ static int vti_tunnel_validate(struct nlattr *tb[], struct nlattr *data[])
}
static void vti_netlink_parms(struct nlattr *data[],
- struct ip_tunnel_parm *parms)
+ struct ip_tunnel_parm *parms,
+ __u32 *fwmark)
{
memset(parms, 0, sizeof(*parms));
@@ -497,24 +498,29 @@ static void vti_netlink_parms(struct nlattr *data[],
if (data[IFLA_VTI_REMOTE])
parms->iph.daddr = nla_get_in_addr(data[IFLA_VTI_REMOTE]);
+ if (data[IFLA_VTI_FWMARK])
+ *fwmark = nla_get_u32(data[IFLA_VTI_FWMARK]);
}
static int vti_newlink(struct net *src_net, struct net_device *dev,
struct nlattr *tb[], struct nlattr *data[])
{
struct ip_tunnel_parm parms;
+ __u32 fwmark = 0;
- vti_netlink_parms(data, &parms);
- return ip_tunnel_newlink(dev, tb, &parms);
+ vti_netlink_parms(data, &parms, &fwmark);
+ return ip_tunnel_newlink(dev, tb, &parms, fwmark);
}
static int vti_changelink(struct net_device *dev, struct nlattr *tb[],
struct nlattr *data[])
{
+ struct ip_tunnel *t = netdev_priv(dev);
+ __u32 fwmark = t->fwmark;
struct ip_tunnel_parm p;
- vti_netlink_parms(data, &p);
- return ip_tunnel_changelink(dev, tb, &p);
+ vti_netlink_parms(data, &p, &fwmark);
+ return ip_tunnel_changelink(dev, tb, &p, fwmark);
}
static size_t vti_get_size(const struct net_device *dev)
@@ -530,6 +536,8 @@ static size_t vti_get_size(const struct net_device *dev)
nla_total_size(4) +
/* IFLA_VTI_REMOTE */
nla_total_size(4) +
+ /* IFLA_VTI_FWMARK */
+ nla_total_size(4) +
0;
}
@@ -543,6 +551,7 @@ static int vti_fill_info(struct sk_buff *skb, const struct net_device *dev)
nla_put_be32(skb, IFLA_VTI_OKEY, p->o_key);
nla_put_in_addr(skb, IFLA_VTI_LOCAL, p->iph.saddr);
nla_put_in_addr(skb, IFLA_VTI_REMOTE, p->iph.daddr);
+ nla_put_u32(skb, IFLA_VTI_FWMARK, t->fwmark);
return 0;
}
@@ -553,6 +562,7 @@ static const struct nla_policy vti_policy[IFLA_VTI_MAX + 1] = {
[IFLA_VTI_OKEY] = { .type = NLA_U32 },
[IFLA_VTI_LOCAL] = { .len = FIELD_SIZEOF(struct iphdr, saddr) },
[IFLA_VTI_REMOTE] = { .len = FIELD_SIZEOF(struct iphdr, daddr) },
+ [IFLA_VTI_FWMARK] = { .type = NLA_U32 },
};
static struct rtnl_link_ops vti_link_ops __read_mostly = {
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index 00d4229b6954..1e441c6f2160 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -390,7 +390,8 @@ static int ipip_tunnel_validate(struct nlattr *tb[], struct nlattr *data[])
}
static void ipip_netlink_parms(struct nlattr *data[],
- struct ip_tunnel_parm *parms, bool *collect_md)
+ struct ip_tunnel_parm *parms, bool *collect_md,
+ __u32 *fwmark)
{
memset(parms, 0, sizeof(*parms));
@@ -428,6 +429,9 @@ static void ipip_netlink_parms(struct nlattr *data[],
if (data[IFLA_IPTUN_COLLECT_METADATA])
*collect_md = true;
+
+ if (data[IFLA_IPTUN_FWMARK])
+ *fwmark = nla_get_u32(data[IFLA_IPTUN_FWMARK]);
}
/* This function returns true when ENCAP attributes are present in the nl msg */
@@ -470,6 +474,7 @@ static int ipip_newlink(struct net *src_net, struct net_device *dev,
struct ip_tunnel *t = netdev_priv(dev);
struct ip_tunnel_parm p;
struct ip_tunnel_encap ipencap;
+ __u32 fwmark = 0;
if (ipip_netlink_encap_parms(data, &ipencap)) {
int err = ip_tunnel_encap_setup(t, &ipencap);
@@ -478,26 +483,27 @@ static int ipip_newlink(struct net *src_net, struct net_device *dev,
return err;
}
- ipip_netlink_parms(data, &p, &t->collect_md);
- return ip_tunnel_newlink(dev, tb, &p);
+ ipip_netlink_parms(data, &p, &t->collect_md, &fwmark);
+ return ip_tunnel_newlink(dev, tb, &p, fwmark);
}
static int ipip_changelink(struct net_device *dev, struct nlattr *tb[],
struct nlattr *data[])
{
+ struct ip_tunnel *t = netdev_priv(dev);
struct ip_tunnel_parm p;
struct ip_tunnel_encap ipencap;
bool collect_md;
+ __u32 fwmark = t->fwmark;
if (ipip_netlink_encap_parms(data, &ipencap)) {
- struct ip_tunnel *t = netdev_priv(dev);
int err = ip_tunnel_encap_setup(t, &ipencap);
if (err < 0)
return err;
}
- ipip_netlink_parms(data, &p, &collect_md);
+ ipip_netlink_parms(data, &p, &collect_md, &fwmark);
if (collect_md)
return -EINVAL;
@@ -505,7 +511,7 @@ static int ipip_changelink(struct net_device *dev, struct nlattr *tb[],
(!(dev->flags & IFF_POINTOPOINT) && p.iph.daddr))
return -EINVAL;
- return ip_tunnel_changelink(dev, tb, &p);
+ return ip_tunnel_changelink(dev, tb, &p, fwmark);
}
static size_t ipip_get_size(const struct net_device *dev)
@@ -535,6 +541,8 @@ static size_t ipip_get_size(const struct net_device *dev)
nla_total_size(2) +
/* IFLA_IPTUN_COLLECT_METADATA */
nla_total_size(0) +
+ /* IFLA_IPTUN_FWMARK */
+ nla_total_size(4) +
0;
}
@@ -550,7 +558,8 @@ static int ipip_fill_info(struct sk_buff *skb, const struct net_device *dev)
nla_put_u8(skb, IFLA_IPTUN_TOS, parm->iph.tos) ||
nla_put_u8(skb, IFLA_IPTUN_PROTO, parm->iph.protocol) ||
nla_put_u8(skb, IFLA_IPTUN_PMTUDISC,
- !!(parm->iph.frag_off & htons(IP_DF))))
+ !!(parm->iph.frag_off & htons(IP_DF))) ||
+ nla_put_u32(skb, IFLA_IPTUN_FWMARK, tunnel->fwmark))
goto nla_put_failure;
if (nla_put_u16(skb, IFLA_IPTUN_ENCAP_TYPE,
@@ -585,6 +594,7 @@ static const struct nla_policy ipip_policy[IFLA_IPTUN_MAX + 1] = {
[IFLA_IPTUN_ENCAP_SPORT] = { .type = NLA_U16 },
[IFLA_IPTUN_ENCAP_DPORT] = { .type = NLA_U16 },
[IFLA_IPTUN_COLLECT_METADATA] = { .type = NLA_FLAG },
+ [IFLA_IPTUN_FWMARK] = { .type = NLA_U32 },
};
static struct rtnl_link_ops ipip_link_ops __read_mostly = {
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 99853c6e33a8..61e5902f0687 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -881,11 +881,12 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,
goto tx_error;
}
- rt = ip_route_output_ports(tunnel->net, &fl4, NULL,
- dst, tiph->saddr,
- 0, 0,
- IPPROTO_IPV6, RT_TOS(tos),
- tunnel->parms.link);
+ flowi4_init_output(&fl4, tunnel->parms.link, tunnel->fwmark,
+ RT_TOS(tos), RT_SCOPE_UNIVERSE, IPPROTO_IPV6,
+ 0, dst, tiph->saddr, 0, 0,
+ sock_net_uid(tunnel->net, NULL));
+ rt = ip_route_output_flow(tunnel->net, &fl4, NULL);
+
if (IS_ERR(rt)) {
dev->stats.tx_carrier_errors++;
goto tx_error_icmp;
@@ -1071,7 +1072,8 @@ static void ipip6_tunnel_bind_dev(struct net_device *dev)
}
}
-static void ipip6_tunnel_update(struct ip_tunnel *t, struct ip_tunnel_parm *p)
+static void ipip6_tunnel_update(struct ip_tunnel *t, struct ip_tunnel_parm *p,
+ __u32 fwmark)
{
struct net *net = t->net;
struct sit_net *sitn = net_generic(net, sit_net_id);
@@ -1085,8 +1087,9 @@ static void ipip6_tunnel_update(struct ip_tunnel *t, struct ip_tunnel_parm *p)
ipip6_tunnel_link(sitn, t);
t->parms.iph.ttl = p->iph.ttl;
t->parms.iph.tos = p->iph.tos;
- if (t->parms.link != p->link) {
+ if (t->parms.link != p->link || t->fwmark != fwmark) {
t->parms.link = p->link;
+ t->fwmark = fwmark;
ipip6_tunnel_bind_dev(t->dev);
}
dst_cache_reset(&t->dst_cache);
@@ -1220,7 +1223,7 @@ ipip6_tunnel_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
t = netdev_priv(dev);
}
- ipip6_tunnel_update(t, &p);
+ ipip6_tunnel_update(t, &p, t->fwmark);
}
if (t) {
@@ -1418,7 +1421,8 @@ static int ipip6_validate(struct nlattr *tb[], struct nlattr *data[])
}
static void ipip6_netlink_parms(struct nlattr *data[],
- struct ip_tunnel_parm *parms)
+ struct ip_tunnel_parm *parms,
+ __u32 *fwmark)
{
memset(parms, 0, sizeof(*parms));
@@ -1457,6 +1461,8 @@ static void ipip6_netlink_parms(struct nlattr *data[],
if (data[IFLA_IPTUN_PROTO])
parms->iph.protocol = nla_get_u8(data[IFLA_IPTUN_PROTO]);
+ if (data[IFLA_IPTUN_FWMARK])
+ *fwmark = nla_get_u32(data[IFLA_IPTUN_FWMARK]);
}
/* This function returns true when ENCAP attributes are present in the nl msg */
@@ -1549,7 +1555,7 @@ static int ipip6_newlink(struct net *src_net, struct net_device *dev,
return err;
}
- ipip6_netlink_parms(data, &nt->parms);
+ ipip6_netlink_parms(data, &nt->parms, &nt->fwmark);
if (ipip6_tunnel_locate(net, &nt->parms, 0))
return -EEXIST;
@@ -1577,6 +1583,7 @@ static int ipip6_changelink(struct net_device *dev, struct nlattr *tb[],
#ifdef CONFIG_IPV6_SIT_6RD
struct ip_tunnel_6rd ip6rd;
#endif
+ __u32 fwmark = t->fwmark;
int err;
if (dev == sitn->fb_tunnel_dev)
@@ -1588,7 +1595,7 @@ static int ipip6_changelink(struct net_device *dev, struct nlattr *tb[],
return err;
}
- ipip6_netlink_parms(data, &p);
+ ipip6_netlink_parms(data, &p, &fwmark);
if (((dev->flags & IFF_POINTOPOINT) && !p.iph.daddr) ||
(!(dev->flags & IFF_POINTOPOINT) && p.iph.daddr))
@@ -1602,7 +1609,7 @@ static int ipip6_changelink(struct net_device *dev, struct nlattr *tb[],
} else
t = netdev_priv(dev);
- ipip6_tunnel_update(t, &p);
+ ipip6_tunnel_update(t, &p, fwmark);
#ifdef CONFIG_IPV6_SIT_6RD
if (ipip6_netlink_6rd_parms(data, &ip6rd))
@@ -1649,6 +1656,8 @@ static size_t ipip6_get_size(const struct net_device *dev)
nla_total_size(2) +
/* IFLA_IPTUN_ENCAP_DPORT */
nla_total_size(2) +
+ /* IFLA_IPTUN_FWMARK */
+ nla_total_size(4) +
0;
}
@@ -1665,7 +1674,8 @@ static int ipip6_fill_info(struct sk_buff *skb, const struct net_device *dev)
nla_put_u8(skb, IFLA_IPTUN_PMTUDISC,
!!(parm->iph.frag_off & htons(IP_DF))) ||
nla_put_u8(skb, IFLA_IPTUN_PROTO, parm->iph.protocol) ||
- nla_put_be16(skb, IFLA_IPTUN_FLAGS, parm->i_flags))
+ nla_put_be16(skb, IFLA_IPTUN_FLAGS, parm->i_flags) ||
+ nla_put_u32(skb, IFLA_IPTUN_FWMARK, tunnel->fwmark))
goto nla_put_failure;
#ifdef CONFIG_IPV6_SIT_6RD
@@ -1715,6 +1725,7 @@ static const struct nla_policy ipip6_policy[IFLA_IPTUN_MAX + 1] = {
[IFLA_IPTUN_ENCAP_FLAGS] = { .type = NLA_U16 },
[IFLA_IPTUN_ENCAP_SPORT] = { .type = NLA_U16 },
[IFLA_IPTUN_ENCAP_DPORT] = { .type = NLA_U16 },
+ [IFLA_IPTUN_FWMARK] = { .type = NLA_U32 },
};
static void ipip6_dellink(struct net_device *dev, struct list_head *head)
--
2.12.2.816.g2cccc81164-goog
^ permalink raw reply related
* Re: [PATCH v2 2/2] net: phy: Don't miss phy_suspend() on PHY_HALTED for PHYs with interrupts
From: Florian Fainelli @ 2017-04-19 16:31 UTC (permalink / raw)
To: Roger Quadros
Cc: andrew, davem, nsekhar, netdev, linux-kernel, stable # v4 . 9+,
Alexander Kochetkov
In-Reply-To: <1490180524-28675-3-git-send-email-rogerq@ti.com>
On 03/22/2017 04:02 AM, Roger Quadros wrote:
> phy_suspend() doesn't get called as part of phy_stop() for PHYs
> using interrupts because the phy state machine is never triggered
> after a phy_stop().
>
> Explicitly trigger the PHY state machine in phy_stop() so that it can
> see the new PHY state (HALTED) and suspend the PHY.
>
> As most PHYLIB consumers will call phy_stop() with rtnl_lock() held
> from ndo_close() so we use don't wait for workqueue cancellation in
> phy_trigger_machine() by passing false for the 'sync' argument.
Sorry for this long delay in responding. I am not exactly sure if this
thing to do here. phy_stop() does not have a requirement to suspend the
PHY as it is currently defined. You may want to manually suspend the PHY
after a phy_stop() by explicitly calling phy_suspend().
Let me think about it some more.
>
> Fixes: 3c293f4e08b5 ("net: phy: Trigger state machine on state change and not polling.")
> Cc: stable <stable@vger.kernel.org> # v4.9+
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> drivers/net/phy/phy.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 49dedf8..ab14e7b 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -907,6 +907,7 @@ void phy_stop(struct phy_device *phydev)
> * of rtnl_lock(), but PHY_HALTED shall guarantee phy_change()
> * will not reenable interrupts.
> */
> + phy_trigger_machine(phydev, false);
> }
> EXPORT_SYMBOL(phy_stop);
>
>
--
Florian
^ permalink raw reply
* Re: [PATCH] net: phy: fix auto-negotiation stall due to unavailable interrupt
From: Florian Fainelli @ 2017-04-19 16:32 UTC (permalink / raw)
To: Alexander Kochetkov, netdev, linux-kernel, andrew, Roger Quadros,
nsekhar, kyle.roeschley
In-Reply-To: <24E19B31-CCD8-4032-B17B-D226D4DB881D@gmail.com>
On 04/19/2017 07:05 AM, Alexander Kochetkov wrote:
> Just found similar problem fixed in another PHY. See commit 99f81afc139c
> ("phy: micrel: Disable auto negotiation on startup»)
This specific commit was really a "this works for me, but I have not
investigated what needs fixing in PHYLIB".
Roger has been submitting a couple of patches to fix the same issue and
yours seems to be better in that it also addresses his concerns that
after his fix there was still a 1s delay (the HZ parameter to the
queue_delayed_work call) being seen:
http://patchwork.ozlabs.org/patch/743773/
Roger can you also test Alexander's patch?
>
>> 19 апр. 2017 г., в 16:46, Alexander Kochetkov <al.kochet@gmail.com> написал(а):
>>
>> The problem I fix related to SMSC LAN8710/LAN8720 PHY handled using
>> interrupts. During power-up cycle the PHY do auto-negotiation, generate
>> interrupt and set BMSR_ANEGCOMPLETE flag. Interrupt is handled by PHY
>> state machine but doesn't update link because PHY is in PHY_READY state.
>> After some time MAC bring up and connect with PHY. It start PHY using
>> phy_start(). During startup PHY change state to PHY_AN but doesn't
>> set BMCR_ANRESTART flag due to genphy_config_aneg() doesn't update MII_BMCR
>> because there no new to advertising. As a result, state machine wait for
>> interrupt from PHY and nether get "link is up". Because BMSR_ANEGCOMPLETE
>> already set the patch schedule check link without waiting interrupt.
>> In case genphy_config_aneg() update MII_BMCR and set BMCR_ANRESTART
>> flag, BMSR_ANEGCOMPLETE will be cleared and state machine will continue
>> on auto-negotiation interrupt.
>>
>> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
>> ---
>> drivers/net/phy/phy.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>> index 7cc1b7d..da8f03d 100644
>> --- a/drivers/net/phy/phy.c
>> +++ b/drivers/net/phy/phy.c
>> @@ -1169,6 +1169,18 @@ void phy_state_machine(struct work_struct *work)
>> if (phydev->irq == PHY_POLL)
>> queue_delayed_work(system_power_efficient_wq, &phydev->state_queue,
>> PHY_STATE_TIME * HZ);
>> +
>> + /* Re-schedule a PHY state machine to check PHY status because
>> + * negotiation already done and aneg interrupt may not be generated.
>> + */
>> + if (needs_aneg && (phydev->irq > 0) && (phydev->state == PHY_AN)) {
>> + err = phy_aneg_done(phydev);
>> + if (err > 0)
>> + queue_delayed_work(system_power_efficient_wq,
>> + &phydev->state_queue, 0);
>> + if (err < 0)
>> + phy_error(phydev);
>> + }
>> }
>>
>> /**
>> --
>> 1.7.9.5
>>
>
--
Florian
^ permalink raw reply
* [PATCH net-next] Replace 2 jiffies with sysctl netdev_budget_usecs to enable softirq tuning
From: Matthew Whitehead @ 2017-04-19 16:37 UTC (permalink / raw)
To: netdev; +Cc: Matthew Whitehead
Constants used for tuning are generally a bad idea, especially as hardware
changes over time. Replace the constant 2 jiffies with sysctl variable
netdev_budget_usecs to enable sysadmins to tune the softirq processing.
Also document the variable.
For example, a very fast machine might tune this to 1000 microseconds,
while my regression testing 486DX-25 needs it to be 4000 microseconds on
a nearly idle network to prevent time_squeeze from being incremented.
Version 2: changed jiffies to microseconds for predictable units.
Signed-off-by: Matthew Whitehead <tedheadster@gmail.com>
---
Documentation/sysctl/net.txt | 11 ++++++++++-
include/linux/netdevice.h | 1 +
include/uapi/linux/sysctl.h | 1 +
kernel/sysctl_binary.c | 1 +
net/core/dev.c | 4 +++-
net/core/sysctl_net_core.c | 8 ++++++++
6 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/Documentation/sysctl/net.txt b/Documentation/sysctl/net.txt
index 2ebabc9..14db18c 100644
--- a/Documentation/sysctl/net.txt
+++ b/Documentation/sysctl/net.txt
@@ -188,7 +188,16 @@ netdev_budget
Maximum number of packets taken from all interfaces in one polling cycle (NAPI
poll). In one polling cycle interfaces which are registered to polling are
-probed in a round-robin manner.
+probed in a round-robin manner. Also, a polling cycle may not exceed
+netdev_budget_usecs microseconds, even if netdev_budget has not been
+exhausted.
+
+netdev_budget_usecs
+---------------------
+
+Maximum number of microseconds in one NAPI polling cycle. Polling
+will exit when either netdev_budget_usecs have elapsed during the
+poll cycle or the number of packets processed reaches netdev_budget.
netdev_max_backlog
------------------
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 97456b25..2fc72a5 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3305,6 +3305,7 @@ static __always_inline int ____dev_forward_skb(struct net_device *dev,
void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev);
extern int netdev_budget;
+extern unsigned int netdev_budget_usecs;
/* Called by rtnetlink.c:rtnl_unlock() */
void netdev_run_todo(void);
diff --git a/include/uapi/linux/sysctl.h b/include/uapi/linux/sysctl.h
index d2b1215..a1f1f25 100644
--- a/include/uapi/linux/sysctl.h
+++ b/include/uapi/linux/sysctl.h
@@ -274,6 +274,7 @@ enum
NET_CORE_AEVENT_ETIME=20,
NET_CORE_AEVENT_RSEQTH=21,
NET_CORE_WARNINGS=22,
+ NET_CORE_BUDGET_USECS=23,
};
/* /proc/sys/net/ethernet */
diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c
index ece4b17..4ee3e49 100644
--- a/kernel/sysctl_binary.c
+++ b/kernel/sysctl_binary.c
@@ -197,6 +197,7 @@ struct bin_table {
{ CTL_INT, NET_CORE_AEVENT_ETIME, "xfrm_aevent_etime" },
{ CTL_INT, NET_CORE_AEVENT_RSEQTH, "xfrm_aevent_rseqth" },
{ CTL_INT, NET_CORE_WARNINGS, "warnings" },
+ { CTL_INT, NET_CORE_BUDGET_USECS, "netdev_budget_usecs" },
{},
};
diff --git a/net/core/dev.c b/net/core/dev.c
index 533a6d6..78627e5 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3441,6 +3441,7 @@ int dev_queue_xmit_accel(struct sk_buff *skb, void *accel_priv)
int netdev_tstamp_prequeue __read_mostly = 1;
int netdev_budget __read_mostly = 300;
+unsigned int __read_mostly netdev_budget_usecs = 2000;
int weight_p __read_mostly = 64; /* old backlog weight */
int dev_weight_rx_bias __read_mostly = 1; /* bias for backlog weight */
int dev_weight_tx_bias __read_mostly = 1; /* bias for output_queue quota */
@@ -5310,7 +5311,8 @@ static int napi_poll(struct napi_struct *n, struct list_head *repoll)
static __latent_entropy void net_rx_action(struct softirq_action *h)
{
struct softnet_data *sd = this_cpu_ptr(&softnet_data);
- unsigned long time_limit = jiffies + 2;
+ unsigned long time_limit = jiffies +
+ usecs_to_jiffies(netdev_budget_usecs);
int budget = netdev_budget;
LIST_HEAD(list);
LIST_HEAD(repoll);
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 7f9cc40..ea23254 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -452,6 +452,14 @@ static int proc_do_rss_key(struct ctl_table *table, int write,
.extra1 = &one,
.extra2 = &max_skb_frags,
},
+ {
+ .procname = "netdev_budget_usecs",
+ .data = &netdev_budget_usecs,
+ .maxlen = sizeof(unsigned int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = &zero,
+ },
{ }
};
--
1.8.3.1
^ permalink raw reply related
* Re: export pcie_flr and remove copies of it in drivers V2
From: Leon Romanovsky @ 2017-04-19 16:40 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Christoph Hellwig, Bjorn Helgaas, Giovanni Cabiddu,
Salvatore Benedetto, Mike Marciniszyn, Dennis Dalessandro,
Derek Chickles, Satanand Burla, Felix Manlunas, Raghu Vatsavayi,
Jeff Kirsher, linux-pci-u79uwXL29TY76Z2rM5mHXA,
qat-linux-ral2JQCrhuEAvxtiuMwx3w,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170419053737.GG14088-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1091 bytes --]
On Wed, Apr 19, 2017 at 08:37:37AM +0300, Leon Romanovsky wrote:
> On Tue, Apr 18, 2017 at 01:36:12PM -0500, Bjorn Helgaas wrote:
> > On Fri, Apr 14, 2017 at 09:11:24PM +0200, Christoph Hellwig wrote:
> > > Hi all,
> > >
> > > this exports the PCI layer pcie_flr helper, and removes various opencoded
> > > copies of it.
> > >
> > > Changes since V1:
> > > - rebase on top of the pci/virtualization branch
> > > - fixed the probe case in __pci_dev_reset
> > > - added ACKs from Bjorn
> >
> > Applied the first three patches:
> >
> > bc13871ef35a PCI: Export pcie_flr()
> > e641c375d414 PCI: Call pcie_flr() from reset_intel_82599_sfp_virtfn()
> > 40e0901ea4bf PCI: Call pcie_flr() from reset_chelsio_generic_dev()
> >
>
> Bjorn,
>
> How do you suggest to proceed with other patches? They should be applied
> to your tree either, because they depend on "bc13871ef35a PCI: Export
> pcie_flr()".
I finally caught the old emails and found the answer by myself.
http://marc.info/?l=linux-rdma&m=149218545228343&w=2
Thanks
>
> Thanks
>
>
> > to pci/virtualization for v4.12, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] net: phy: fix auto-negotiation stall due to unavailable interrupt
From: Alexander Kochetkov @ 2017-04-19 16:53 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, linux-kernel, andrew, Roger Quadros, nsekhar,
kyle.roeschley
In-Reply-To: <754f7cfe-6914-570a-d0e5-4a26b6787281@gmail.com>
> 19 апр. 2017 г., в 19:32, Florian Fainelli <f.fainelli@gmail.com> написал(а):
>
> http://patchwork.ozlabs.org/patch/743773/
>
> Roger can you also test Alexander's patch?
If MAC use phy_start_aneg() instead of phy_start() my patch will not work as
expected. Roger, if patch don’t work for you please check what MAC bring up PHY using
phy_start():
http://patchwork.ozlabs.org/patch/752308/
Is it correct to start PHY inside MAC probe using phy_start_aneg()? Or phy_start() must be used?
And probably this tags should be added for my patch:
Fixes: 3c293f4e08b5 ("net: phy: Trigger state machine on state change and not polling.")
Cc: stable <stable@vger.kernel.org> # v4.9+
Because I bisected to commit 529ed1275263 ("net: phy: phy drivers
should not set SUPPORTED_[Asym_]Pause») that looks pretty good.
Also, there is another issue I found. link_timeout doesn’t work for interrupt driven PHY.
It is possible to implement timer to handle this case.
Florian, what do you think? Should this be fixed?
Alexander.
^ permalink raw reply
* [PATCH net 0/6] net: use skb_cow_head() to deal with cloned skbs
From: Eric Dumazet @ 2017-04-19 16:59 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, James Hughes, Eric Dumazet, Eric Dumazet
James Hughes found an issue with smsc95xx driver. Same problematic code
is found in other drivers.
Eric Dumazet (6):
smsc75xx: use skb_cow_head() to deal with cloned skbs
cx82310_eth: use skb_cow_head() to deal with cloned skbs
sr9700: use skb_cow_head() to deal with cloned skbs
lan78xx: use skb_cow_head() to deal with cloned skbs
ch9200: use skb_cow_head() to deal with cloned skbs
kaweth: use skb_cow_head() to deal with cloned skbs
drivers/net/usb/ch9200.c | 9 ++-------
drivers/net/usb/cx82310_eth.c | 7 ++-----
drivers/net/usb/kaweth.c | 18 ++++++------------
drivers/net/usb/lan78xx.c | 9 ++-------
drivers/net/usb/smsc75xx.c | 8 ++------
drivers/net/usb/sr9700.c | 9 ++-------
6 files changed, 16 insertions(+), 44 deletions(-)
--
2.12.2.816.g2cccc81164-goog
^ permalink raw reply
* [PATCH net 5/6] ch9200: use skb_cow_head() to deal with cloned skbs
From: Eric Dumazet @ 2017-04-19 16:59 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, James Hughes, Eric Dumazet, Eric Dumazet, Matthew Garrett
In-Reply-To: <20170419165926.30631-1-edumazet@google.com>
We need to ensure there is enough headroom to push extra header,
but we also need to check if we are allowed to change headers.
skb_cow_head() is the proper helper to deal with this.
Fixes: 4a476bd6d1d9 ("usbnet: New driver for QinHeng CH9200 devices")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: James Hughes <james.hughes@raspberrypi.org>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
---
drivers/net/usb/ch9200.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/net/usb/ch9200.c b/drivers/net/usb/ch9200.c
index 8a40202c0a1732850da1b2eb64af21470b9c85b4..c4f1c363e24b89404c6834312074f8a4451ded50 100644
--- a/drivers/net/usb/ch9200.c
+++ b/drivers/net/usb/ch9200.c
@@ -254,14 +254,9 @@ static struct sk_buff *ch9200_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
tx_overhead = 0x40;
len = skb->len;
- if (skb_headroom(skb) < tx_overhead) {
- struct sk_buff *skb2;
-
- skb2 = skb_copy_expand(skb, tx_overhead, 0, flags);
+ if (skb_cow_head(skb, tx_overhead)) {
dev_kfree_skb_any(skb);
- skb = skb2;
- if (!skb)
- return NULL;
+ return NULL;
}
__skb_push(skb, tx_overhead);
--
2.12.2.816.g2cccc81164-goog
^ permalink raw reply related
* [PATCH net 1/6] smsc75xx: use skb_cow_head() to deal with cloned skbs
From: Eric Dumazet @ 2017-04-19 16:59 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, James Hughes, Eric Dumazet, Eric Dumazet
In-Reply-To: <20170419165926.30631-1-edumazet@google.com>
We need to ensure there is enough headroom to push extra header,
but we also need to check if we are allowed to change headers.
skb_cow_head() is the proper helper to deal with this.
Fixes: d0cad871703b ("smsc75xx: SMSC LAN75xx USB gigabit ethernet adapter driver")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: James Hughes <james.hughes@raspberrypi.org>
---
drivers/net/usb/smsc75xx.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 0b17b40d7a4fa2653caf21406c4a6b3b45d868b0..190de9a90f7387c5070c7f589aa18bb7d05ac5d7 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -2203,13 +2203,9 @@ static struct sk_buff *smsc75xx_tx_fixup(struct usbnet *dev,
{
u32 tx_cmd_a, tx_cmd_b;
- if (skb_headroom(skb) < SMSC75XX_TX_OVERHEAD) {
- struct sk_buff *skb2 =
- skb_copy_expand(skb, SMSC75XX_TX_OVERHEAD, 0, flags);
+ if (skb_cow_head(skb, SMSC75XX_TX_OVERHEAD)) {
dev_kfree_skb_any(skb);
- skb = skb2;
- if (!skb)
- return NULL;
+ return NULL;
}
tx_cmd_a = (u32)(skb->len & TX_CMD_A_LEN) | TX_CMD_A_FCS;
--
2.12.2.816.g2cccc81164-goog
^ permalink raw reply related
* [PATCH net 3/6] sr9700: use skb_cow_head() to deal with cloned skbs
From: Eric Dumazet @ 2017-04-19 16:59 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, James Hughes, Eric Dumazet, Eric Dumazet
In-Reply-To: <20170419165926.30631-1-edumazet@google.com>
We need to ensure there is enough headroom to push extra header,
but we also need to check if we are allowed to change headers.
skb_cow_head() is the proper helper to deal with this.
Fixes: c9b37458e956 ("USB2NET : SR9700 : One chip USB 1.1 USB2NET SR9700Device Driver Support")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: James Hughes <james.hughes@raspberrypi.org>
---
drivers/net/usb/sr9700.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/net/usb/sr9700.c b/drivers/net/usb/sr9700.c
index 4a1e9c489f1f455388ffee289d65e1d6b36cba42..aadfe1d1c37ee67e2a7d17c759db12dad248c41d 100644
--- a/drivers/net/usb/sr9700.c
+++ b/drivers/net/usb/sr9700.c
@@ -456,14 +456,9 @@ static struct sk_buff *sr9700_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
len = skb->len;
- if (skb_headroom(skb) < SR_TX_OVERHEAD) {
- struct sk_buff *skb2;
-
- skb2 = skb_copy_expand(skb, SR_TX_OVERHEAD, 0, flags);
+ if (skb_cow_head(skb, SR_TX_OVERHEAD)) {
dev_kfree_skb_any(skb);
- skb = skb2;
- if (!skb)
- return NULL;
+ return NULL;
}
__skb_push(skb, SR_TX_OVERHEAD);
--
2.12.2.816.g2cccc81164-goog
^ 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