Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] r8169: add enable_aspm parameter
From: Heiner Kallweit @ 2019-07-08 18:27 UTC (permalink / raw)
  To: AceLan Kao, Realtek linux nic maintainers, David S. Miller,
	netdev, linux-kernel
In-Reply-To: <20190708063751.16234-1-acelan.kao@canonical.com>

On 08.07.2019 08:37, AceLan Kao wrote:
> We have many commits in the driver which enable and then disable ASPM
> function over and over again.
>    commit b75bb8a5b755 ("r8169: disable ASPM again")
>    commit 0866cd15029b ("r8169: enable ASPM on RTL8106E")
>    commit 94235460f9ea ("r8169: Align ASPM/CLKREQ setting function with vendor driver")
>    commit aa1e7d2c31ef ("r8169: enable ASPM on RTL8168E-VL")
>    commit f37658da21aa ("r8169: align ASPM entry latency setting with vendor driver")
>    commit a99790bf5c7f ("r8169: Reinstate ASPM Support")
>    commit 671646c151d4 ("r8169: Don't disable ASPM in the driver")
>    commit 4521e1a94279 ("Revert "r8169: enable internal ASPM and clock request settings".")
>    commit d64ec841517a ("r8169: enable internal ASPM and clock request settings")
> 
> This function is very important for production, and if we can't come out
> a solution to make both happy, I'd suggest we add a parameter in the
> driver to toggle it.
> 
The usage of a module parameter to control ASPM is discouraged.
There have been more such attempts in the past that have been declined.

Pending with the PCI maintainers is a series adding ASPM control
via sysfs, see here: https://www.spinics.net/lists/linux-pci/msg83228.html

Also more details than just stating "it's important for production"
would have been appreciated in the commit message, e.g. which
power-savings you can achieve with ASPM on which systems.

^ permalink raw reply

* Re: [PATCH] net: hisilicon: Add an tx_desc to adapt HI13X1_GMAC
From: David Miller @ 2019-07-08 18:18 UTC (permalink / raw)
  To: xiaojiangfeng
  Cc: yisen.zhuang, salil.mehta, dingtianhong, robh+dt, mark.rutland,
	netdev, devicetree, linux-kernel, leeyou.li, xiekunxun,
	jianping.liu, nixiaoming
In-Reply-To: <20190707.221805.2104668553072088371.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Sun, 07 Jul 2019 22:18:05 -0700 (PDT)

> From: Jiangfeng Xiao <xiaojiangfeng@huawei.com>
> Date: Fri, 5 Jul 2019 14:10:03 +0800
> 
>> HI13X1 changed the offsets and bitmaps for tx_desc
>> registers in the same peripheral device on different
>> models of the hip04_eth.
>> 
>> Signed-off-by: Jiangfeng Xiao <xiaojiangfeng@huawei.com>
> 
> Applied.

Actually I didn't apply this because I can't see that HI13X1_GMAC
kconfig knob anywhere in the tree at all.

^ permalink raw reply

* Re: [PATCH net-next v6 04/15] ethtool: introduce ethtool netlink interface
From: Johannes Berg @ 2019-07-08 18:12 UTC (permalink / raw)
  To: Michal Kubecek, netdev
  Cc: Jiri Pirko, David Miller, Jakub Kicinski, Andrew Lunn,
	Florian Fainelli, John Linville, Stephen Hemminger, linux-kernel
In-Reply-To: <20190708172729.GC24474@unicorn.suse.cz>

On Mon, 2019-07-08 at 19:27 +0200, Michal Kubecek wrote:
> 
> Second reason is that with 8-bit genetlink command/message id, the space
> is not as infinite as it might seem.

FWIW, there isn't really any good reason for this, we have like 16
reserved bits in the genl header.

OTOH, having a LOT of ops will certainly cost space in the kernel
image...

johannes


^ permalink raw reply

* Re: [PATCH ghak90 V6 02/10] audit: add container id
From: Richard Guy Briggs @ 2019-07-08 18:12 UTC (permalink / raw)
  To: Paul Moore
  Cc: Tycho Andersen, Serge E. Hallyn, containers, linux-api,
	Linux-Audit Mailing List, linux-fsdevel, LKML, netdev,
	netfilter-devel, sgrubb, omosnace, dhowells, simo, Eric Paris,
	ebiederm, nhorman
In-Reply-To: <CAHC9VhT5HPt9rCJoDutdvA3r1Y1GOHfpXe2eJ54atNC1=Vd8LA@mail.gmail.com>

On 2019-05-30 19:26, Paul Moore wrote:
> On Thu, May 30, 2019 at 5:29 PM Tycho Andersen <tycho@tycho.ws> wrote:
> > On Thu, May 30, 2019 at 03:29:32PM -0400, Paul Moore wrote:
> > >
> > > [REMINDER: It is an "*audit* container ID" and not a general
> > > "container ID" ;)  Smiley aside, I'm not kidding about that part.]
> >
> > This sort of seems like a distinction without a difference; presumably
> > audit is going to want to differentiate between everything that people
> > in userspace call a container. So you'll have to support all this
> > insanity anyway, even if it's "not a container ID".
> 
> That's not quite right.  Audit doesn't care about what a container is,
> or is not, it also doesn't care if the "audit container ID" actually
> matches the ID used by the container engine in userspace and I think
> that is a very important line to draw.  Audit is simply given a value
> which it calls the "audit container ID", it ensures that the value is
> inherited appropriately (e.g. children inherit their parent's audit
> container ID), and it uses the value in audit records to provide some
> additional context for log analysis.  The distinction isn't limited to
> the value itself, but also to how it is used; it is an "audit
> container ID" and not a "container ID" because this value is
> exclusively for use by the audit subsystem.  We are very intentionally
> not adding a generic container ID to the kernel.  If the kernel does
> ever grow a general purpose container ID we will be one of the first
> ones in line to make use of it, but we are not going to be the ones to
> generically add containers to the kernel.  Enough people already hate
> audit ;)
> 
> > > I'm not interested in supporting/merging something that isn't useful;
> > > if this doesn't work for your use case then we need to figure out what
> > > would work.  It sounds like nested containers are much more common in
> > > the lxc world, can you elaborate a bit more on this?
> > >
> > > As far as the possible solutions you mention above, I'm not sure I
> > > like the per-userns audit container IDs, I'd much rather just emit the
> > > necessary tracking information via the audit record stream and let the
> > > log analysis tools figure it out.  However, the bigger question is how
> > > to limit (re)setting the audit container ID when you are in a non-init
> > > userns.  For reasons already mentioned, using capable() is a non
> > > starter for everything but the initial userns, and using ns_capable()
> > > is equally poor as it essentially allows any userns the ability to
> > > munge it's audit container ID (obviously not good).  It appears we
> > > need a different method for controlling access to the audit container
> > > ID.
> >
> > One option would be to make it a string, and have it be append only.
> > That should be safe with no checks.
> >
> > I know there was a long thread about what type to make this thing. I
> > think you could accomplish the append-only-ness with a u64 if you had
> > some rule about only allowing setting lower order bits than those that
> > are already set. With 4 bits for simplicity:
> >
> > 1100         # initial container id
> > 1100 -> 1011 # not allowed
> > 1100 -> 1101 # allowed, but now 1101 is set in stone since there are
> >              # no lower order bits left
> >
> > There are probably fancier ways to do it if you actually understand
> > math :)
> 
>  ;)
> 
> > Since userns nesting is limited to 32 levels (right now, IIRC), and
> > you have 64 bits, this might be reasonable. You could just teach
> > container engines to use the first say N bits for themselves, with a 1
> > bit for the barrier at the end.
> 
> I like the creativity, but I worry that at some point these
> limitations are going to be raised (limits have a funny way of doing
> that over time) and we will be in trouble.  I say "trouble" because I
> want to be able to quickly do an audit container ID comparison and
> we're going to pay a penalty for these larger values (we'll need this
> when we add multiple auditd support and the requisite record routing).
> 
> Thinking about this makes me also realize we probably need to think a
> bit longer about audit container ID conflicts between orchestrators.
> Right now we just take the value that is given to us by the
> orchestrator, but if we want to allow multiple container orchestrators
> to work without some form of cooperation in userspace (I think we have
> to assume the orchestrators will not talk to each other) we likely
> need to have some way to block reuse of an audit container ID.  We
> would either need to prevent the orchestrator from explicitly setting
> an audit container ID to a currently in use value, or instead generate
> the audit container ID in the kernel upon an event triggered by the
> orchestrator (e.g. a write to a /proc file).  I suspect we should
> start looking at the idr code, I think we will need to make use of it.

To address this, I'd suggest that it is enforced to only allow the
setting of descendants and to maintain a master list of audit container
identifiers (with a hash table if necessary later) that includes the
container owner.

This also allows the orchestrator/engine to inject processes into
existing containers by checking that the audit container identifier is
only used again by the same owner.

I have working code for both.

> paul moore
> www.paul-moore.com

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply

* Re: [PATCH ghak90 V6 02/10] audit: add container id
From: Richard Guy Briggs @ 2019-07-08 18:05 UTC (permalink / raw)
  To: Paul Moore
  Cc: Serge E. Hallyn, Tycho Andersen, containers, linux-api,
	Linux-Audit Mailing List, linux-fsdevel, LKML, netdev,
	netfilter-devel, sgrubb, omosnace, dhowells, simo, Eric Paris,
	ebiederm, nhorman
In-Reply-To: <CAHC9VhThLiQzGYRUWmSuVfOC6QCDmA75BDB7Eg7V8HX4x7ymQg@mail.gmail.com>

On 2019-05-30 15:29, Paul Moore wrote:
> On Thu, May 30, 2019 at 1:09 PM Serge E. Hallyn <serge@hallyn.com> wrote:
> > On Wed, May 29, 2019 at 06:39:48PM -0400, Paul Moore wrote:
> > > On Wed, May 29, 2019 at 6:28 PM Tycho Andersen <tycho@tycho.ws> wrote:
> > > > On Wed, May 29, 2019 at 12:03:58PM -0400, Paul Moore wrote:
> > > > > On Wed, May 29, 2019 at 11:34 AM Tycho Andersen <tycho@tycho.ws> wrote:
> > > > > > On Wed, May 29, 2019 at 11:29:05AM -0400, Paul Moore wrote:
> > > > > > > On Wed, May 29, 2019 at 10:57 AM Tycho Andersen <tycho@tycho.ws> wrote:
> > > > > > > > On Mon, Apr 08, 2019 at 11:39:09PM -0400, Richard Guy Briggs wrote:
> 
> ...
> 
> > > > > > > The current thinking
> > > > > > > is that you would only change the audit container ID from one
> > > > > > > set/inherited value to another if you were nesting containers, in
> > > > > > > which case the nested container orchestrator would need to be granted
> > > > > > > CAP_AUDIT_CONTROL (which everyone to date seems to agree is a workable
> > > > > > > compromise).
> > > >
> > > > won't work in user namespaced containers, because they will never be
> > > > capable(CAP_AUDIT_CONTROL); so I don't think this will work for
> > > > nesting as is. But maybe nobody cares :)
> > >
> > > That's fun :)
> > >
> > > To be honest, I've never been a big fan of supporting nested
> > > containers from an audit perspective, so I'm not really too upset
> > > about this.  The k8s/cri-o folks seem okay with this, or at least I
> > > haven't heard any objections; lxc folks, what do you have to say?
> >
> > I actually thought the answer to this (when last I looked, "some time" ago)
> > was that userspace should track an audit message saying "task X in
> > container Y is changing its auditid to Z", and then decide to also track Z.
> > This should be doable, but a lot of extra work in userspace.
> >
> > Per-userns containerids would also work.  So task X1 is in containerid
> > 1 on the host and creates a new task Y in new userns;  it continues to
> > be reported in init_user_ns as containerid 1 forever;  but in its own
> > userns it can request to be known as some other containerid.  Audit
> > socks would be per-userns, allowing root in a container to watch for
> > audit events in its own (and descendent) namespaces.
> >
> > But again I'm sure we've gone over all this in the last few years.
> >
> > I suppose we can look at this as a "first step", and talk about
> > making it user-ns-nestable later.  But agreed it's not useful in a
> > lot of situations as is.
> 
> [REMINDER: It is an "*audit* container ID" and not a general
> "container ID" ;)  Smiley aside, I'm not kidding about that part.]
> 
> I'm not interested in supporting/merging something that isn't useful;
> if this doesn't work for your use case then we need to figure out what
> would work.  It sounds like nested containers are much more common in
> the lxc world, can you elaborate a bit more on this?
> 
> As far as the possible solutions you mention above, I'm not sure I
> like the per-userns audit container IDs, I'd much rather just emit the
> necessary tracking information via the audit record stream and let the
> log analysis tools figure it out.  However, the bigger question is how
> to limit (re)setting the audit container ID when you are in a non-init
> userns.  For reasons already mentioned, using capable() is a non
> starter for everything but the initial userns, and using ns_capable()
> is equally poor as it essentially allows any userns the ability to
> munge it's audit container ID (obviously not good).  It appears we
> need a different method for controlling access to the audit container
> ID.

We're not quite ready yet for multiple audit daemons and possibly not
yet for audit namespaces, but this is starting to look a lot like the
latter.

If we can't trust ns_capable() then why are we passing on
CAP_AUDIT_CONTROL?  It is being passed down and not stripped purposely
by the orchestrator/engine.  If ns_capable() isn't inherited how is it
gained otherwise?  Can it be inserted by cotainer image?  I think the
answer is "no".  Either we trust ns_capable() or we have audit
namespaces (recommend based on user namespace) (or both).

At this point I would say we are at an impasse unless we trust
ns_capable() or we implement audit namespaces.

I don't think another mechanism to trust nested orchestrators/engines
will buy us anything.

Am I missing something?

> Punting this to a LSM hook is an obvious thing to do, and something we
> might want to do anyway, but currently audit doesn't rely on the LSM
> for proper/safe operation and I'm not sure I want to change that now.
> 
> The next obvious thing is to create some sort of access control knob
> in audit itself.  Perhaps an auditctl operation that would allow the
> administrator to specify which containers, via their corresponding
> audit container IDs, are allowed to change their audit container ID?
> The permission granting would need to be done in the init userns, but
> it would allow containers with a non-init userns the ability to change
> their audit container ID.  We would probably still want a
> ns_capable(CAP_AUDIT_CONTROL) restriction in this case.

This auditctl knob of which you speak is an additional API, not changing
the existing proposed one.

> Does anyone else have any other ideas?
> 
> -- 
> paul moore
> www.paul-moore.com

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply

* Re: [PATCH] rtw88/pci: Rearrange the memory usage for skb in RX ISR
From: Larry Finger @ 2019-07-08 18:01 UTC (permalink / raw)
  To: Jian-Hong Pan, Yan-Hsuan Chuang, Kalle Valo, David S . Miller
  Cc: linux-wireless, netdev, linux-kernel, linux, Daniel Drake, stable
In-Reply-To: <20190708063252.4756-1-jian-hong@endlessm.com>

On 7/8/19 1:32 AM, Jian-Hong Pan wrote:
> diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
> index cfe05ba7280d..1bfc99ae6b84 100644
> --- a/drivers/net/wireless/realtek/rtw88/pci.c
> +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> @@ -786,6 +786,15 @@ static void rtw_pci_rx_isr(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
>   		rx_desc = skb->data;
>   		chip->ops->query_rx_desc(rtwdev, rx_desc, &pkt_stat, &rx_status);
>   
> +		/* discard current skb if the new skb cannot be allocated as a
> +		 * new one in rx ring later
> +		 * */
> +		new = dev_alloc_skb(RTK_PCI_RX_BUF_SIZE);
> +		if (WARN(!new, "rx routine starvation\n")) {
> +			new = skb;
> +			goto next_rp;

This should probably be a WARN_ONCE() rather than WARN(), otherwise the logs 
will be flooded once this condition triggers.

Larry

^ permalink raw reply

* Re: [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API
From: Andrii Nakryiko @ 2019-07-08 17:58 UTC (permalink / raw)
  To: Matt Hart
  Cc: Andrii Nakryiko, Alexei Starovoitov, bpf, Daniel Borkmann,
	Kernel Team, Networking, Stanislav Fomichev
In-Reply-To: <CAH+k93FQkiwRXwgRGrUJEpmAGZBL03URKDmx8uVA9MnLrDKn0Q@mail.gmail.com>

On Mon, Jul 8, 2019 at 8:11 AM Matt Hart <matthew.hart@linaro.org> wrote:
>
> Hi all,
>
> I bisected a perf build error on ARMv7 to this patch:
> libbpf.c: In function ‘perf_event_open_probe’:
> libbpf.c:4112:17: error: cast from pointer to integer of different
> size [-Werror=pointer-to-int-cast]
>   attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
>                  ^
>
> Is this a known issue?

No, thanks for reporting!

It should be

attr.config1 = (uint64_t)(uintptr_t)(void *)name;

to avoid warning on 32-bit architectures.

I'll post a fix later today, but if you could verify this fixes
warning for you, I'd really appreciate that! Thanks!

^ permalink raw reply

* Re: [PATCH net-next iproute2 2/3] tc: Introduce tc ct action
From: Marcelo Ricardo Leitner @ 2019-07-08 17:54 UTC (permalink / raw)
  To: Paul Blakey
  Cc: Jiri Pirko, Roi Dayan, Yossi Kuperman, Oz Shlomo, netdev,
	David Miller, Aaron Conole, Zhike Wang, Justin Pettit,
	John Hurley, Rony Efraim, nst-kernel, Simon Horman
In-Reply-To: <1562489628-5925-3-git-send-email-paulb@mellanox.com>

On Sun, Jul 07, 2019 at 11:53:47AM +0300, Paul Blakey wrote:
> New tc action to send packets to conntrack module, commit
> them, and set a zone, labels, mark, and nat on the connection.
> 
> It can also clear the packet's conntrack state by using clear.
> 
> Usage:
>    ct clear
>    ct commit [force] [zone] [mark] [label] [nat]

Isn't the 'commit' also optional? More like
    ct [commit [force]] [zone] [mark] [label] [nat]

>    ct [nat] [zone]
> 
> Signed-off-by: Paul Blakey <paulb@mellanox.com>
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Signed-off-by: Yossi Kuperman <yossiku@mellanox.com>
> Acked-by: Jiri Pirko <jiri@mellanox.com>
> Acked-by: Roi Dayan <roid@mellanox.com>
> ---
...
> +static void
> +usage(void)
> +{
> +	fprintf(stderr,
> +		"Usage: ct clear\n"
> +		"	ct commit [force] [zone ZONE] [mark MASKED_MARK] [label MASKED_LABEL] [nat NAT_SPEC]\n"

Ditto here then.

> +		"	ct [nat] [zone ZONE]\n"
> +		"Where: ZONE is the conntrack zone table number\n"
> +		"	NAT_SPEC is {src|dst} addr addr1[-addr2] [port port1[-port2]]\n"
> +		"\n");
> +	exit(-1);
> +}
...

The validation below doesn't enforce that commit must be there for
such case.

> +static int
> +parse_ct(struct action_util *a, int *argc_p, char ***argv_p, int tca_id,
> +		struct nlmsghdr *n)
> +{
> +	struct tc_ct sel = {};
> +	char **argv = *argv_p;
> +	struct rtattr *tail;
> +	int argc = *argc_p;
> +	int ct_action = 0;
> +	int ret;
> +
> +	tail = addattr_nest(n, MAX_MSG, tca_id);
> +
> +	if (argc && matches(*argv, "ct") == 0)
> +		NEXT_ARG_FWD();
> +
> +	while (argc > 0) {
> +		if (matches(*argv, "zone") == 0) {
> +			NEXT_ARG();
> +
> +			if (ct_parse_u16(*argv,
> +					 TCA_CT_ZONE, TCA_CT_UNSPEC, n)) {
> +				fprintf(stderr, "ct: Illegal \"zone\"\n");
> +				return -1;
> +			}
> +		} else if (matches(*argv, "nat") == 0) {
> +			ct_action |= TCA_CT_ACT_NAT;
> +
> +			NEXT_ARG();
> +			if (matches(*argv, "src") == 0)
> +				ct_action |= TCA_CT_ACT_NAT_SRC;
> +			else if (matches(*argv, "dst") == 0)
> +				ct_action |= TCA_CT_ACT_NAT_DST;
> +			else
> +				continue;
> +
> +			NEXT_ARG();
> +			if (matches(*argv, "addr") != 0)
> +				usage();
> +
> +			NEXT_ARG();
> +			ret = ct_parse_nat_addr_range(*argv, n);
> +			if (ret) {
> +				fprintf(stderr, "ct: Illegal nat address range\n");
> +				return -1;
> +			}
> +
> +			NEXT_ARG_FWD();
> +			if (matches(*argv, "port") != 0)
> +				continue;
> +
> +			NEXT_ARG();
> +			ret = ct_parse_nat_port_range(*argv, n);
> +			if (ret) {
> +				fprintf(stderr, "ct: Illegal nat port range\n");
> +				return -1;
> +			}
> +		} else if (matches(*argv, "clear") == 0) {
> +			ct_action |= TCA_CT_ACT_CLEAR;
> +		} else if (matches(*argv, "commit") == 0) {
> +			ct_action |= TCA_CT_ACT_COMMIT;
> +		} else if (matches(*argv, "force") == 0) {
> +			ct_action |= TCA_CT_ACT_FORCE;
> +		} else if (matches(*argv, "index") == 0) {
> +			NEXT_ARG();
> +			if (get_u32(&sel.index, *argv, 10)) {
> +				fprintf(stderr, "ct: Illegal \"index\"\n");
> +				return -1;
> +			}
> +		} else if (matches(*argv, "mark") == 0) {
> +			NEXT_ARG();
> +
> +			ret = ct_parse_mark(*argv, n);
> +			if (ret) {
> +				fprintf(stderr, "ct: Illegal \"mark\"\n");
> +				return -1;
> +			}
> +		} else if (matches(*argv, "label") == 0) {
> +			NEXT_ARG();
> +
> +			ret = ct_parse_labels(*argv, n);
> +			if (ret) {
> +				fprintf(stderr, "ct: Illegal \"label\"\n");
> +				return -1;
> +			}
> +		} else if (matches(*argv, "help") == 0) {
> +			usage();
> +		} else {
> +			break;
> +		}
> +		NEXT_ARG_FWD();
> +	}
> +
> +	if (ct_action & TCA_CT_ACT_CLEAR &&
> +	    ct_action & ~TCA_CT_ACT_CLEAR) {
> +		fprintf(stderr, "ct: clear can only be used alone\n");
> +		return -1;
> +	}
> +
> +	if (ct_action & TCA_CT_ACT_NAT_SRC &&
> +	    ct_action & TCA_CT_ACT_NAT_DST) {
> +		fprintf(stderr, "ct: src and dst nat can't be used together\n");
> +		return -1;
> +	}
> +
> +	if ((ct_action & TCA_CT_ACT_COMMIT) &&
> +	    (ct_action & TCA_CT_ACT_NAT) &&
> +	    !(ct_action & (TCA_CT_ACT_NAT_SRC | TCA_CT_ACT_NAT_DST))) {
> +		fprintf(stderr, "ct: commit and nat must set src or dst\n");
> +		return -1;
> +	}
> +
> +	if (!(ct_action & TCA_CT_ACT_COMMIT) &&
> +	    (ct_action & (TCA_CT_ACT_NAT_SRC | TCA_CT_ACT_NAT_DST))) {
> +		fprintf(stderr, "ct: src or dst is only valid if commit is set\n");
> +		return -1;
> +	}
> +
> +	parse_action_control_dflt(&argc, &argv, &sel.action, false,
> +				  TC_ACT_PIPE);
> +	NEXT_ARG_FWD();
> +
> +	addattr16(n, MAX_MSG, TCA_CT_ACTION, ct_action);
> +	addattr_l(n, MAX_MSG, TCA_CT_PARMS, &sel, sizeof(sel));
> +	addattr_nest_end(n, tail);
> +
> +	*argc_p = argc;
> +	*argv_p = argv;
> +	return 0;
> +}
...

^ permalink raw reply

* Re: [PATCH bpf-next 1/2] bpf, libbpf: add a new API bpf_object__reuse_maps()
From: Andrii Nakryiko @ 2019-07-08 17:54 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Anton Protopopov, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, Networking, bpf, open list, Andrii Nakryiko
In-Reply-To: <734dd45a-95b0-a7fd-9e1d-0535ef4d3e12@iogearbox.net>

On Fri, Jul 5, 2019 at 2:53 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 07/05/2019 10:44 PM, Anton Protopopov wrote:
> > Add a new API bpf_object__reuse_maps() which can be used to replace all maps in
> > an object by maps pinned to a directory provided in the path argument.  Namely,
> > each map M in the object will be replaced by a map pinned to path/M.name.
> >
> > Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
> > ---
> >  tools/lib/bpf/libbpf.c   | 34 ++++++++++++++++++++++++++++++++++
> >  tools/lib/bpf/libbpf.h   |  2 ++
> >  tools/lib/bpf/libbpf.map |  1 +
> >  3 files changed, 37 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 4907997289e9..84c9e8f7bfd3 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -3144,6 +3144,40 @@ int bpf_object__unpin_maps(struct bpf_object *obj, const char *path)
> >       return 0;
> >  }
> >
> > +int bpf_object__reuse_maps(struct bpf_object *obj, const char *path)

As is, bpf_object__reuse_maps() can be easily implemented by user
applications, as it's only using public libbpf APIs, so I'm not 100%
sure we need to add method like that to libbpf.

> > +{
> > +     struct bpf_map *map;
> > +
> > +     if (!obj)
> > +             return -ENOENT;
> > +
> > +     if (!path)
> > +             return -EINVAL;
> > +
> > +     bpf_object__for_each_map(map, obj) {
> > +             int len, err;
> > +             int pinned_map_fd;
> > +             char buf[PATH_MAX];
>
> We'd need to skip the case of bpf_map__is_internal(map) since they are always
> recreated for the given object.
>
> > +             len = snprintf(buf, PATH_MAX, "%s/%s", path, bpf_map__name(map));
> > +             if (len < 0) {
> > +                     return -EINVAL;
> > +             } else if (len >= PATH_MAX) {
> > +                     return -ENAMETOOLONG;
> > +             }
> > +
> > +             pinned_map_fd = bpf_obj_get(buf);
> > +             if (pinned_map_fd < 0)
> > +                     return pinned_map_fd;
>
> Should we rather have a new map definition attribute that tells to reuse
> the map if it's pinned in bpf fs, and if not, we create it and later on
> pin it? This is what iproute2 is doing and which we're making use of heavily.

I'd like something like that as well. This would play nicely with
recently added BTF-defined maps as well.

I think it should be not just pin/don't pin flag, but rather pinning
strategy, to accommodate various typical strategies of handling maps
that are already pinned. So something like this:

1. BPF_PIN_NOTHING - default, don't pin;
2. BPF_PIN_EXCLUSIVE - pin, but if map is already pinned - fail;
3. BPF_PIN_SET - pin; if existing map exists, reset its state to be
exact state of object's map;
4. BPF_PIN_MERGE - pin, if map exists, fill in NULL entries only (this
is how Cilium is pinning PROG_ARRAY maps, if I understand correctly);
5. BPF_PIN_MERGE_OVERWRITE - pin, if map exists, overwrite non-NULL values.

This list is only for illustrative purposes, ideally people that have
a lot of experience using pinning for real-world use cases would chime
in on what strategies are useful and make sense.

> In bpf_object__reuse_maps() bailing out if bpf_obj_get() fails is perhaps
> too limiting for a generic API as new version of an object file may contain
> new maps which are not yet present in bpf fs at that point.
>
> > +             err = bpf_map__reuse_fd(map, pinned_map_fd);
> > +             if (err)
> > +                     return err;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> >  int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
> >  {
> >       struct bpf_program *prog;
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index d639f47e3110..7fe465a1be76 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -82,6 +82,8 @@ int bpf_object__variable_offset(const struct bpf_object *obj, const char *name,
> >  LIBBPF_API int bpf_object__pin_maps(struct bpf_object *obj, const char *path);
> >  LIBBPF_API int bpf_object__unpin_maps(struct bpf_object *obj,
> >                                     const char *path);
> > +LIBBPF_API int bpf_object__reuse_maps(struct bpf_object *obj,
> > +                                   const char *path);
> >  LIBBPF_API int bpf_object__pin_programs(struct bpf_object *obj,
> >                                       const char *path);
> >  LIBBPF_API int bpf_object__unpin_programs(struct bpf_object *obj,
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index 2c6d835620d2..66a30be6696c 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -172,5 +172,6 @@ LIBBPF_0.0.4 {
> >               btf_dump__new;
> >               btf__parse_elf;
> >               bpf_object__load_xattr;
> > +             bpf_object__reuse_maps;
> >               libbpf_num_possible_cpus;
> >  } LIBBPF_0.0.3;
> >
>

^ permalink raw reply

* Re: [PATCH ghak90 V6 02/10] audit: add container id
From: Richard Guy Briggs @ 2019-07-08 17:51 UTC (permalink / raw)
  To: Paul Moore
  Cc: Tycho Andersen, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev, netfilter-devel, sgrubb, omosnace,
	dhowells, simo, Eric Paris, Serge Hallyn, ebiederm, nhorman
In-Reply-To: <CAHC9VhR4fudQanvZGYWMvCf7k2CU3q7e7n1Pi7hzC3v_zpVEdw@mail.gmail.com>

On 2019-05-29 11:29, Paul Moore wrote:
> On Wed, May 29, 2019 at 10:57 AM Tycho Andersen <tycho@tycho.ws> wrote:
> >
> > On Mon, Apr 08, 2019 at 11:39:09PM -0400, Richard Guy Briggs wrote:
> > > It is not permitted to unset the audit container identifier.
> > > A child inherits its parent's audit container identifier.
> >
> > ...
> >
> > >  /**
> > > + * audit_set_contid - set current task's audit contid
> > > + * @contid: contid value
> > > + *
> > > + * Returns 0 on success, -EPERM on permission failure.
> > > + *
> > > + * Called (set) from fs/proc/base.c::proc_contid_write().
> > > + */
> > > +int audit_set_contid(struct task_struct *task, u64 contid)
> > > +{
> > > +     u64 oldcontid;
> > > +     int rc = 0;
> > > +     struct audit_buffer *ab;
> > > +     uid_t uid;
> > > +     struct tty_struct *tty;
> > > +     char comm[sizeof(current->comm)];
> > > +
> > > +     task_lock(task);
> > > +     /* Can't set if audit disabled */
> > > +     if (!task->audit) {
> > > +             task_unlock(task);
> > > +             return -ENOPROTOOPT;
> > > +     }
> > > +     oldcontid = audit_get_contid(task);
> > > +     read_lock(&tasklist_lock);
> > > +     /* Don't allow the audit containerid to be unset */
> > > +     if (!audit_contid_valid(contid))
> > > +             rc = -EINVAL;
> > > +     /* if we don't have caps, reject */
> > > +     else if (!capable(CAP_AUDIT_CONTROL))
> > > +             rc = -EPERM;
> > > +     /* if task has children or is not single-threaded, deny */
> > > +     else if (!list_empty(&task->children))
> > > +             rc = -EBUSY;
> > > +     else if (!(thread_group_leader(task) && thread_group_empty(task)))
> > > +             rc = -EALREADY;
> > > +     read_unlock(&tasklist_lock);
> > > +     if (!rc)
> > > +             task->audit->contid = contid;
> > > +     task_unlock(task);
> > > +
> > > +     if (!audit_enabled)
> > > +             return rc;
> >
> > ...but it is allowed to change it (assuming
> > capable(CAP_AUDIT_CONTROL), of course)? Seems like this might be more
> > immediately useful since we still live in the world of majority
> > privileged containers if we didn't allow changing it, in addition to
> > un-setting it.
> 
> The idea is that only container orchestrators should be able to
> set/modify the audit container ID, and since setting the audit
> container ID can have a significant effect on the records captured
> (and their routing to multiple daemons when we get there) modifying
> the audit container ID is akin to modifying the audit configuration
> which is why it is gated by CAP_AUDIT_CONTROL.  The current thinking
> is that you would only change the audit container ID from one
> set/inherited value to another if you were nesting containers, in
> which case the nested container orchestrator would need to be granted
> CAP_AUDIT_CONTROL (which everyone to date seems to agree is a workable
> compromise).  We did consider allowing for a chain of nested audit
> container IDs, but the implications of doing so are significant
> (implementation mess, runtime cost, etc.) so we are leaving that out
> of this effort.

We had previously discussed the idea of restricting
orchestrators/engines from only being able to set the audit container
identifier on their own descendants, but it was discarded.  I've added a
check to ensure this is now enforced.

I've also added a check to ensure that a process can't set its own audit
container identifier and that if the identifier is already set, then the
orchestrator/engine must be in a descendant user namespace from the
orchestrator that set the previously inherited audit container
identifier.

> From a practical perspective, un-setting the audit container ID is
> pretty much the same as changing it from one set value to another so
> most of the above applies to that case as well.
> 
> -- 
> paul moore
> www.paul-moore.com

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply

* Re: [PATCH net-next,v3 07/11] net: sched: use flow block API
From: Jiri Pirko @ 2019-07-08 17:42 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
	michael.chan, madalin.bucur, yisen.zhuang, salil.mehta,
	jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
	peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
	alexandre.torgue, joabreu, linux-net-drivers, ogerlitz,
	Manish.Chopra, marcelo.leitner, mkubecek, venkatkumar.duvvuru,
	maxime.chevallier, cphealy, netfilter-devel
In-Reply-To: <20190708160614.2226-8-pablo@netfilter.org>

Mon, Jul 08, 2019 at 06:06:09PM CEST, pablo@netfilter.org wrote:
>This patch adds tcf_block_setup() which uses the flow block API.
>
>This infrastructure takes the flow block callbacks coming from the
>driver and register/unregister to/from the cls_api core.
>
>Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

[...]


>+static int tcf_block_bind(struct tcf_block *block,
>+			  struct flow_block_offload *bo)
>+{
>+	struct flow_block_cb *block_cb, *next;
>+	int err, i = 0;
>+
>+	list_for_each_entry(block_cb, &bo->cb_list, driver_list) {
>+		err = tcf_block_playback_offloads(block, block_cb->cb,
>+						  block_cb->cb_priv, true,
>+						  tcf_block_offload_in_use(block),
>+						  bo->extack);
>+		if (err)
>+			goto err_unroll;
>+
>+		list_add(&block_cb->list, &block->cb_list);
>+		i++;
>+	}
>+	list_splice(&bo->cb_list, bo->driver_block_list);

This cl/driver_block list magic is really very hard to follow. Could you
please make it more clear?



>+
>+	return 0;
>+
>+err_unroll:
>+	list_for_each_entry_safe(block_cb, next, &bo->cb_list, driver_list) {
>+		if (i-- > 0) {
>+			list_del(&block_cb->list);
>+			tcf_block_playback_offloads(block, block_cb->cb,
>+						    block_cb->cb_priv, false,
>+						    tcf_block_offload_in_use(block),
>+						    NULL);
>+		}
>+		flow_block_cb_free(block_cb);
>+	}
>+
>+	return err;
>+}
>+
>+static void tcf_block_unbind(struct tcf_block *block,
>+			     struct flow_block_offload *bo)
>+{
>+	struct flow_block_cb *block_cb, *next;
>+
>+	list_for_each_entry_safe(block_cb, next, &bo->cb_list, driver_list) {
>+		list_del(&block_cb->driver_list);
>+		tcf_block_playback_offloads(block, block_cb->cb,
>+					    block_cb->cb_priv, false,
>+					    tcf_block_offload_in_use(block),
>+					    NULL);
>+		list_del(&block_cb->list);
>+		flow_block_cb_free(block_cb);
>+	}
>+}

[...]

^ permalink raw reply

* Re: [PATCH v3] perf cs-etm: Improve completeness for kernel address space
From: Mathieu Poirier @ 2019-07-08 17:33 UTC (permalink / raw)
  To: Leo Yan
  Cc: Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Suzuki K Poulose, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	Linux Kernel Mailing List, linux-arm-kernel, netdev, bpf,
	Peter Zijlstra, Coresight ML
In-Reply-To: <20190620034446.25561-1-leo.yan@linaro.org>

On Wed, 19 Jun 2019 at 21:45, Leo Yan <leo.yan@linaro.org> wrote:
>
> Arm and arm64 architecture reserve some memory regions prior to the
> symbol '_stext' and these memory regions later will be used by device
> module and BPF jit.  The current code misses to consider these memory
> regions thus any address in the regions will be taken as user space
> mode, but perf cannot find the corresponding dso with the wrong CPU
> mode so we misses to generate samples for device module and BPF
> related trace data.
>
> This patch parse the link scripts to get the memory size prior to start
> address and reduce this size from 'etmq->etm->kernel_start', then can
> get a fixed up kernel start address which contain memory regions for
> device module and BPF.  Finally, cs_etm__cpu_mode() can return right
> mode for these memory regions and perf can successfully generate
> samples.
>
> The reason for parsing the link scripts is Arm architecture changes text
> offset dependent on different platforms, which define multiple text
> offsets in $kernel/arch/arm/Makefile.  This offset is decided when build
> kernel and the final value is extended in the link script, so we can
> extract the used value from the link script.  We use the same way to
> parse arm64 link script as well.  If fail to find the link script, the
> pre start memory size is assumed as zero, in this case it has no any
> change caused with this patch.
>
> Below is detailed info for testing this patch:
>
> - Build LLVM/Clang 8.0 or later version;
>
> - Configure perf with ~/.perfconfig:
>
>   root@debian:~# cat ~/.perfconfig
>   # this file is auto-generated.
>   [llvm]
>           clang-path = /mnt/build/llvm-build/build/install/bin/clang
>           kbuild-dir = /mnt/linux-kernel/linux-cs-dev/
>           clang-opt = "-g"
>           dump-obj = true
>
>   [trace]
>           show_zeros = yes
>           show_duration = no
>           no_inherit = yes
>           show_timestamp = no
>           show_arg_names = no
>           args_alignment = 40
>           show_prefix = yes
>
> - Run 'perf trace' command with eBPF event:
>
>   root@debian:~# perf trace -e string \
>       -e $kernel/tools/perf/examples/bpf/augmented_raw_syscalls.c
>
> - Read eBPF program memory mapping in kernel:
>
>   root@debian:~# echo 1 > /proc/sys/net/core/bpf_jit_kallsyms
>   root@debian:~# cat /proc/kallsyms | grep -E "bpf_prog_.+_sys_[enter|exit]"
>   ffff000000086a84 t bpf_prog_f173133dc38ccf87_sys_enter  [bpf]
>   ffff000000088618 t bpf_prog_c1bd85c092d6e4aa_sys_exit   [bpf]
>
> - Launch any program which accesses file system frequently so can hit
>   the system calls trace flow with eBPF event;
>
> - Capture CoreSight trace data with filtering eBPF program:
>
>   root@debian:~# perf record -e cs_etm/@20070000.etr/ \
>           --filter 'filter 0xffff000000086a84/0x800' -a sleep 5s
>
> - Annotate for symbol 'bpf_prog_f173133dc38ccf87_sys_enter':
>
>   root@debian:~# perf report
>   Then select 'branches' samples and press 'a' to annotate symbol
>   'bpf_prog_f173133dc38ccf87_sys_enter', press 'P' to print to the
>   bpf_prog_f173133dc38ccf87_sys_enter.annotation file:
>
>   root@debian:~# cat bpf_prog_f173133dc38ccf87_sys_enter.annotation
>
>   bpf_prog_f173133dc38ccf87_sys_enter() bpf_prog_f173133dc38ccf87_sys_enter
>   Event: branches
>
>   Percent      int sys_enter(struct syscall_enter_args *args)
>                  stp  x29, x30, [sp, #-16]!
>
>                 int key = 0;
>                  mov  x29, sp
>
>                        augmented_args = bpf_map_lookup_elem(&augmented_filename_map, &key);
>                  stp  x19, x20, [sp, #-16]!
>
>                        augmented_args = bpf_map_lookup_elem(&augmented_filename_map, &key);
>                  stp  x21, x22, [sp, #-16]!
>
>                  stp  x25, x26, [sp, #-16]!
>
>                 return bpf_get_current_pid_tgid();
>                  mov  x25, sp
>
>                 return bpf_get_current_pid_tgid();
>                  mov  x26, #0x0                         // #0
>
>                  sub  sp, sp, #0x10
>
>                 return bpf_map_lookup_elem(pids, &pid) != NULL;
>                  add  x19, x0, #0x0
>
>                  mov  x0, #0x0                          // #0
>
>                  mov  x10, #0xfffffffffffffff8          // #-8
>
>                 if (pid_filter__has(&pids_filtered, getpid()))
>                  str  w0, [x25, x10]
>
>                 probe_read(&augmented_args->args, sizeof(augmented_args->args), args);
>                  add  x1, x25, #0x0
>
>                 probe_read(&augmented_args->args, sizeof(augmented_args->args), args);
>                  mov  x10, #0xfffffffffffffff8          // #-8
>
>                 syscall = bpf_map_lookup_elem(&syscalls, &augmented_args->args.syscall_nr);
>                  add  x1, x1, x10
>
>                 syscall = bpf_map_lookup_elem(&syscalls, &augmented_args->args.syscall_nr);
>                  mov  x0, #0xffff8009ffffffff           // #-140694538682369
>
>                  movk x0, #0x6698, lsl #16
>
>                  movk x0, #0x3e00
>
>                  mov  x10, #0xffffffffffff1040          // #-61376
>
>                 if (syscall == NULL || !syscall->enabled)
>                  movk x10, #0x1023, lsl #16
>
>                 if (syscall == NULL || !syscall->enabled)
>                  movk x10, #0x0, lsl #32
>
>                 loop_iter_first()
>     3.69       → blr  bpf_prog_f173133dc38ccf87_sys_enter
>                 loop_iter_first()
>                  add  x7, x0, #0x0
>
>                 loop_iter_first()
>                  add  x20, x7, #0x0
>
>                 int size = probe_read_str(&augmented_filename->value, filename_len, filename_arg);
>                  mov  x0, #0x1                          // #1

I'm not sure all this information about annotation should be in the
changelog.  This patch is about being able to decode traces that
executed outside the current kernel addresse range and as such simply
using "perf report" or "perf script" successfully is enough to test
this set.  Any information that goes beyond that muddies the water.

>
>   [...]
>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
> Cc: coresight@lists.linaro.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  tools/perf/Makefile.config | 22 ++++++++++++++++++++++
>  tools/perf/util/cs-etm.c   | 19 ++++++++++++++++++-
>  2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 51dd00f65709..a58cd5a43a98 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -418,6 +418,28 @@ ifdef CORESIGHT
>      endif
>      LDFLAGS += $(LIBOPENCSD_LDFLAGS)
>      EXTLIBS += $(OPENCSDLIBS)
> +    PRE_START_SIZE := 0
> +    ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds),)
> +      ifeq ($(SRCARCH),arm64)
> +        # Extract info from lds:
> +        #  . = ((((((((0xffffffffffffffff)) - (((1)) << (48)) + 1) + (0)) + (0x08000000))) + (0x08000000))) + 0x00080000;
> +        # PRE_START_SIZE := (0x08000000 + 0x08000000 + 0x00080000) = 0x10080000
> +        PRE_START_SIZE := $(shell egrep ' \. \= \({8}0x[0-9a-fA-F]+\){2}' \
> +          $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds | \
> +          sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
> +          awk -F' ' '{printf "0x%x", $$6+$$7+$$8}' 2>/dev/null)
> +      endif
> +      ifeq ($(SRCARCH),arm)
> +        # Extract info from lds:
> +        #   . = ((0xC0000000)) + 0x00208000;
> +        # PRE_START_SIZE := 0x00208000
> +        PRE_START_SIZE := $(shell egrep ' \. \= \({2}0x[0-9a-fA-F]+\){2}' \
> +          $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds | \
> +          sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
> +          awk -F' ' '{printf "0x%x", $$2}' 2>/dev/null)
> +      endif
> +    endif
> +    CFLAGS += -DARM_PRE_START_SIZE=$(PRE_START_SIZE)

It might be useful to do this for arm and arm64 regardless of
CoreSight but I'll let Arnaldo decide on this.

>      $(call detected,CONFIG_LIBOPENCSD)
>      ifdef CSTRACE_RAW
>        CFLAGS += -DCS_DEBUG_RAW
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 0c7776b51045..5fa0be3a3904 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -613,10 +613,27 @@ static void cs_etm__free(struct perf_session *session)
>  static u8 cs_etm__cpu_mode(struct cs_etm_queue *etmq, u64 address)
>  {
>         struct machine *machine;
> +       u64 fixup_kernel_start = 0;
>
>         machine = etmq->etm->machine;
>
> -       if (address >= etmq->etm->kernel_start) {
> +       /*
> +        * Since arm and arm64 specify some memory regions prior to
> +        * 'kernel_start', kernel addresses can be less than 'kernel_start'.
> +        *
> +        * For arm architecture, the 16MB virtual memory space prior to
> +        * 'kernel_start' is allocated to device modules, a PMD table if
> +        * CONFIG_HIGHMEM is enabled and a PGD table.
> +        *
> +        * For arm64 architecture, the root PGD table, device module memory
> +        * region and BPF jit region are prior to 'kernel_start'.
> +        *
> +        * To reflect the complete kernel address space, compensate these
> +        * pre-defined regions for kernel start address.
> +        */
> +       fixup_kernel_start = etmq->etm->kernel_start - ARM_PRE_START_SIZE;
> +
> +       if (address >= fixup_kernel_start) {
>                 if (machine__is_host(machine))
>                         return PERF_RECORD_MISC_KERNEL;
>                 else

Tested-by: Mathieu Poirier <mathieu.poirier@linaro.org>

> --
> 2.17.1
>

^ permalink raw reply

* Re: [PATCH net-next,v3 06/11] net: flow_offload: add flow_block_cb_{priv,incref,decref}()
From: Jiri Pirko @ 2019-07-08 17:28 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
	michael.chan, madalin.bucur, yisen.zhuang, salil.mehta,
	jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
	peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
	alexandre.torgue, joabreu, linux-net-drivers, ogerlitz,
	Manish.Chopra, marcelo.leitner, mkubecek, venkatkumar.duvvuru,
	maxime.chevallier, cphealy, netfilter-devel
In-Reply-To: <20190708160614.2226-7-pablo@netfilter.org>

Mon, Jul 08, 2019 at 06:06:08PM CEST, pablo@netfilter.org wrote:
>This patch completes the flow block API to introduce:
>
>* flow_block_cb_priv() to accept callback private data.

"accept"? That's sounds odd to me.


>* flow_block_cb_incref() to bump reference counter on this flow block.
>* flow_block_cb_decref() to decrement the reference counter.
>
>These function are taken from the existing tcf_block_cb_priv(),

"functions"


>tcf_block_cb_incref() and tcf_block_cb_decref().
>

[...]

^ permalink raw reply

* Re: [PATCH net-next] net: dsa: vsc73xx: Fix Kconfig warning and build errors
From: Andrew Lunn @ 2019-07-08 17:28 UTC (permalink / raw)
  To: YueHaibing
  Cc: vivien.didelot, f.fainelli, davem, paweldembicki, linux-kernel,
	netdev
In-Reply-To: <20190708144224.33376-1-yuehaibing@huawei.com>

On Mon, Jul 08, 2019 at 10:42:24PM +0800, YueHaibing wrote:
> Fix Kconfig dependency warning and subsequent build errors
> caused by OF is not set:
> 
> WARNING: unmet direct dependencies detected for NET_DSA_VITESSE_VSC73XX
>   Depends on [n]: NETDEVICES [=y] && HAVE_NET_DSA [=y] && OF [=n] && NET_DSA [=m]
>   Selected by [m]:
>   - NET_DSA_VITESSE_VSC73XX_PLATFORM [=m] && NETDEVICES [=y] && HAVE_NET_DSA [=y] && HAS_IOMEM [=y]
> 
> Move OF and NET_DSA dependencies to NET_DSA_VITESSE_VSC73XX/
> NET_DSA_VITESSE_VSC73XX_PLATFORM to fix this.

Hi YueHaibing

I might be better to make NET_DSA_VITESSE_VSC73XX_SPI and NET_DSA_VITESSE_VSC73XX_PLATFORM
depend on NET_DSA_VITESSE_VSC73XX rather than select it.

       Andrew

^ permalink raw reply

* Re: [PATCH net-next v6 04/15] ethtool: introduce ethtool netlink interface
From: Michal Kubecek @ 2019-07-08 17:27 UTC (permalink / raw)
  To: netdev
  Cc: Jiri Pirko, David Miller, Jakub Kicinski, Andrew Lunn,
	Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg,
	linux-kernel
In-Reply-To: <20190703084151.GR2250@nanopsycho>

On Wed, Jul 03, 2019 at 10:41:51AM +0200, Jiri Pirko wrote:
> Tue, Jul 02, 2019 at 04:52:41PM CEST, mkubecek@suse.cz wrote:
> >On Tue, Jul 02, 2019 at 02:25:21PM +0200, Jiri Pirko wrote:
> >> Tue, Jul 02, 2019 at 01:49:59PM CEST, mkubecek@suse.cz wrote:
> >> >+
> >> >+    ETHTOOL_A_HEADER_DEV_INDEX	(u32)		device ifindex
> >> >+    ETHTOOL_A_HEADER_DEV_NAME	(string)	device name
> >> >+    ETHTOOL_A_HEADER_INFOMASK	(u32)		info mask
> >> >+    ETHTOOL_A_HEADER_GFLAGS	(u32)		flags common for all requests
> >> >+    ETHTOOL_A_HEADER_RFLAGS	(u32)		request specific flags
> >> >+
> >> >+ETHTOOL_A_HEADER_DEV_INDEX and ETHTOOL_A_HEADER_DEV_NAME identify the device
> >> >+message relates to. One of them is sufficient in requests, if both are used,
> >> >+they must identify the same device. Some requests, e.g. global string sets, do
> >> >+not require device identification. Most GET requests also allow dump requests
> >> >+without device identification to query the same information for all devices
> >> >+providing it (each device in a separate message).
> >> >+
> >> >+Optional info mask allows to ask only for a part of data provided by GET
> >> 
> >> How this "infomask" works? What are the bits related to? Is that request
> >> specific?
> >
> >The interpretation is request specific, the information returned for
> >a GET request is divided into multiple parts and client can choose to
> >request one of them (usually one). In the code so far, infomask bits
> >correspond to top level (nest) attributes but I would rather not make it
> >a strict rule.
> 
> Wait, so it is a matter of verbosity? If you have multiple parts and the
> user is able to chose one of them, why don't you rather have multiple
> get commands, one per bit. This infomask construct seems redundant to me.

I thought it was a matter of verbosity because it is a very basic
element of the design, it was even advertised in the cover letter among
the basic ideas, it has been there since the very beginning and in five
previous versions through year and a half, noone did question it. That's
why I thought you objected against unclear description, not against the
concept as such.

There are two reasons for this design. First is to reduce the number of
requests needed to get the information. This is not so much a problem of
ethtool itself; the only existing commands that would result in multiple
request messages would be "ethtool <dev>" and "ethtool -s <dev>". Maybe
also "ethtool -x/-X <dev>" but even if the indirection table and hash
key have different bits assigned now, they don't have to be split even
if we split other commands. It may be bigger problem for daemons wanting
to keep track of system configuration which would have to issue many
requests whenever a new device appears.

Second reason is that with 8-bit genetlink command/message id, the space
is not as infinite as it might seem. I counted quickly, right now the
full series uses 14 ids for kernel messages, with split you propose it
would most likely grow to 44. For full implementation of all ethtool
functionality, we could get to ~60 ids. It's still only 1/4 of the
available space but it's not clear what the future development will look
like. We would certainly need to be careful not to start allocating new
commands for single parameters and try to be foreseeing about what can
be grouped together. But we will need to do that in any case.

On kernel side, splitting existing messages would make some things a bit
easier. It would also reduce the number of scenarios where only part of
requested information is available or only part of a SET request fails.

Michal

^ permalink raw reply

* Re: [PATCH net-next,v3 05/11] net: flow_offload: add list handling functions
From: Jiri Pirko @ 2019-07-08 17:26 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
	michael.chan, madalin.bucur, yisen.zhuang, salil.mehta,
	jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
	peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
	alexandre.torgue, joabreu, linux-net-drivers, ogerlitz,
	Manish.Chopra, marcelo.leitner, mkubecek, venkatkumar.duvvuru,
	maxime.chevallier, cphealy, netfilter-devel
In-Reply-To: <20190708160614.2226-6-pablo@netfilter.org>

Mon, Jul 08, 2019 at 06:06:07PM CEST, pablo@netfilter.org wrote:
>This patch adds the list handling functions for the flow block API:
>
>* flow_block_cb_lookup() allows drivers to look up for existing flow blocks.
>* flow_block_cb_add() adds a flow block to the list to be registered by the
>  core.

Per driver? You say "per driver" in the "remove" part.


>* flow_block_cb_remove() to remove a flow block from the list of existing
>  flow blocks per driver and to request the core to unregister this.
>
>The flow block API also annotates the netns this flow block belongs to.
>
>Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>---
>v3: extracted from former patch "net: flow_offload: add flow_block_cb API".
>
> include/net/flow_offload.h | 20 ++++++++++++++++++++
> net/core/flow_offload.c    | 18 ++++++++++++++++++
> net/sched/cls_api.c        |  3 +++
> 3 files changed, 41 insertions(+)
>
>diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
>index bcc4e2fef6ba..06acde2960fa 100644
>--- a/include/net/flow_offload.h
>+++ b/include/net/flow_offload.h
>@@ -251,12 +251,16 @@ struct flow_block_offload {
> 	enum flow_block_command command;
> 	enum flow_block_binder_type binder_type;
> 	struct tcf_block *block;
>+	struct net *net;
>+	struct list_head cb_list;
> 	struct list_head *driver_block_list;
> 	struct netlink_ext_ack *extack;
> };
> 
> struct flow_block_cb {
>+	struct list_head	driver_list;
> 	struct list_head	list;
>+	struct net		*net;
> 	tc_setup_cb_t		*cb;
> 	void			*cb_ident;
> 	void			*cb_priv;
>@@ -269,6 +273,22 @@ struct flow_block_cb *flow_block_cb_alloc(struct net *net, tc_setup_cb_t *cb,
> 					  void (*release)(void *cb_priv));
> void flow_block_cb_free(struct flow_block_cb *block_cb);
> 
>+struct flow_block_cb *flow_block_cb_lookup(struct net *net,
>+					   struct list_head *driver_flow_block_list,
>+					   tc_setup_cb_t *cb, void *cb_ident);
>+
>+static inline void flow_block_cb_add(struct flow_block_cb *block_cb,
>+				     struct flow_block_offload *offload)
>+{
>+	list_add_tail(&block_cb->driver_list, &offload->cb_list);
>+}
>+
>+static inline void flow_block_cb_remove(struct flow_block_cb *block_cb,
>+					struct flow_block_offload *offload)
>+{
>+	list_move(&block_cb->driver_list, &offload->cb_list);
>+}
>+
> int flow_block_cb_setup_simple(struct flow_block_offload *f,
> 			       struct list_head *driver_list, tc_setup_cb_t *cb,
> 			       void *cb_ident, void *cb_priv, bool ingress_only);
>diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
>index d08148cb6953..85fd5f4a1e0f 100644
>--- a/net/core/flow_offload.c
>+++ b/net/core/flow_offload.c
>@@ -176,6 +176,7 @@ struct flow_block_cb *flow_block_cb_alloc(struct net *net, tc_setup_cb_t *cb,
> 	if (!block_cb)
> 		return ERR_PTR(-ENOMEM);
> 
>+	block_cb->net = net;
> 	block_cb->cb = cb;
> 	block_cb->cb_ident = cb_ident;
> 	block_cb->cb_priv = cb_priv;
>@@ -194,6 +195,23 @@ void flow_block_cb_free(struct flow_block_cb *block_cb)
> }
> EXPORT_SYMBOL(flow_block_cb_free);
> 
>+struct flow_block_cb *flow_block_cb_lookup(struct net *net,
>+					   struct list_head *driver_block_list,

In the header, you call this "driver_flow_block_list".

Where is this list coming from? In general, I don't think it is good to
have struct list_head as an arg of exported symbol. Should be contained
in some struct. Looks like this might be the "struct
flow_block_offload"?

Does this have anything to do with "struct list_head
*driver_block_list"? This is very confusing...



>+					   tc_setup_cb_t *cb, void *cb_ident)
>+{
>+	struct flow_block_cb *block_cb;
>+
>+	list_for_each_entry(block_cb, driver_block_list, driver_list) {
>+		if (block_cb->net == net &&
>+		    block_cb->cb == cb &&
>+		    block_cb->cb_ident == cb_ident)
>+			return block_cb;
>+	}
>+
>+	return NULL;
>+}
>+EXPORT_SYMBOL(flow_block_cb_lookup);
>+
> int flow_block_cb_setup_simple(struct flow_block_offload *f,
> 			       struct list_head *driver_block_list,
> 			       tc_setup_cb_t *cb, void *cb_ident, void *cb_priv,
>diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
>index fa0c451aca59..72761b43ae41 100644
>--- a/net/sched/cls_api.c
>+++ b/net/sched/cls_api.c
>@@ -679,6 +679,7 @@ static void tc_indr_block_ing_cmd(struct tc_indr_block_dev *indr_dev,
> 	struct tc_block_offload bo = {
> 		.command	= command,
> 		.binder_type	= FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS,
>+		.net		= dev_net(indr_dev->dev),
> 		.block		= indr_dev->block,
> 	};
> 
>@@ -767,6 +768,7 @@ static void tc_indr_block_call(struct tcf_block *block, struct net_device *dev,
> 	struct tc_block_offload bo = {
> 		.command	= command,
> 		.binder_type	= ei->binder_type,
>+		.net		= dev_net(dev),
> 		.block		= block,
> 		.extack		= extack,
> 	};
>@@ -795,6 +797,7 @@ static int tcf_block_offload_cmd(struct tcf_block *block,
> {
> 	struct tc_block_offload bo = {};
> 
>+	bo.net = dev_net(dev);
> 	bo.command = command;
> 	bo.binder_type = ei->binder_type;
> 	bo.block = block;
>-- 
>2.11.0
>

^ permalink raw reply

* Re: [PATCH v2 net-next 1/3] tc-testing: Add JSON verification to tdc
From: Alexander Aring @ 2019-07-08 17:24 UTC (permalink / raw)
  To: Lucas Bates
  Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim,
	Cong Wang, Jiri Pirko, Marcelo Ricardo Leitner, Vlad Buslov,
	Davide Caratti, kernel
In-Reply-To: <CAMDBHY+Mg9W0wJRQWeUBHCk=G0Qp4nij8B4Oz77XA6AK2Dt7Gw@mail.gmail.com>

Hi,

On Mon, Jul 08, 2019 at 12:48:12PM -0400, Lucas Bates wrote:
> On Thu, Jul 4, 2019 at 4:21 PM Alexander Aring <aring@mojatatu.com> wrote:
> 
> > why you just use eval() as pattern matching operation and let the user
> > define how to declare a matching mechanism instead you introduce another
> > static matching scheme based on a json description?
> >
> > Whereas in eval() you could directly use the python bool expression
> > parser to make whatever you want.
> >
> > I don't know, I see at some points you will hit limitations what you can
> > express with this matchFOO and we need to introduce another matchBAR,
> > whereas in providing the code it should be no problem expression
> > anything. If you want smaller shortcuts writing matching patterns you
> > can implement them and using in your eval() operation.
> 
> Regarding hitting limitations: quite possibly, yes.
> 
> Using eval() to provide code for matching is going to put more of a
> dependency on the test writer knowing Python.  I know it's not a
> terribly difficult language to pick up, but it's still setting a
> higher barrier to entry.  This is the primary reason I scrapped the
> work I had presented at Netdev 1.2 in Tokyo, where all the tests were
> coded using Python's unittest framework - I want to be sure it's as
> easy as possible for people to use tdc and write tests for it.
> 
> Unless I'm off-base here?

yes you need to know some python, complex code can be hidden by some
helper functionality I guess.

I have no problem to let this patch in, it will not harm anything...

Maybe I work on a matchEval and show some examples... in a human
readable way you can even concatenate bool expressions in combinations
with helpers.

I just was curious, so I might add the matchEval or something to show
this approach.

add the and it shows like:

"x == 5 or x == '5'"

Whereas you could introduce helpers to do:

"str_or_num(x, 5)"

even

"str_or_num_any_base(x, 5)"

to also catch if somebody change the base.
In this case "or" could be also concatenate with python bool
expression... depends on how lowlvl your helpers be.

Pretty sure the x as inputstring to match can also be hidden by user or
transformed with split, regex, etc before. At the end it will work like
TC with actions just provide the code to run... or is it more like
"act_bpf"?, where act is the hook and bpf the eval(). :-)

- Alex

^ permalink raw reply

* Re: [PATCH v2 net-next] net: stmmac: enable clause 45 mdio support
From: Andrew Lunn @ 2019-07-08 17:24 UTC (permalink / raw)
  To: Voon Weifeng
  Cc: David S. Miller, Maxime Coquelin, netdev, linux-kernel,
	Jose Abreu, Giuseppe Cavallaro, Florian Fainelli,
	Alexandre Torgue, biao huang, Ong Boon Leong, Kweh Hock Leong
In-Reply-To: <1562348007-12263-1-git-send-email-weifeng.voon@intel.com>

On Sat, Jul 06, 2019 at 01:33:27AM +0800, Voon Weifeng wrote:
> From: Kweh Hock Leong <hock.leong.kweh@intel.com>
> 
> DWMAC4 is capable to support clause 45 mdio communication.
> This patch enable the feature on stmmac_mdio_write() and
> stmmac_mdio_read() by following phy_write_mmd() and
> phy_read_mmd() mdiobus read write implementation format.
> 
> Reviewed-by: Li, Yifan <yifan2.li@intel.com>
> Signed-off-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
> Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
> Signed-off-by: Voon Weifeng <weifeng.voon@intel.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCH 1/1] tools/dtrace: initial implementation of DTrace
From: Arnaldo Carvalho de Melo @ 2019-07-08 17:15 UTC (permalink / raw)
  To: Kris Van Hees
  Cc: netdev, bpf, dtrace-devel, linux-kernel, rostedt, mhiramat, ast,
	daniel, Peter Zijlstra, Chris Mason
In-Reply-To: <201907040314.x643EUoA017906@aserv0122.oracle.com>

Em Wed, Jul 03, 2019 at 08:14:30PM -0700, Kris Van Hees escreveu:
> This initial implementation of a tiny subset of DTrace functionality
> provides the following options:
> 
> 	dtrace [-lvV] [-b bufsz] -s script
> 	    -b  set trace buffer size
> 	    -l  list probes (only works with '-s script' for now)
> 	    -s  enable or list probes for the specified BPF program
> 	    -V  report DTrace API version
> 
> The patch comprises quite a bit of code due to DTrace requiring a few
> crucial components, even in its most basic form.
> 
> The code is structured around the command line interface implemented in
> dtrace.c.  It provides option parsing and drives the three modes of
> operation that are currently implemented:
> 
> 1. Report DTrace API version information.
> 	Report the version information and terminate.
> 
> 2. List probes in BPF programs.
> 	Initialize the list of probes that DTrace recognizes, load BPF
> 	programs, parse all BPF ELF section names, resolve them into
> 	known probes, and emit the probe names.  Then terminate.
> 
> 3. Load BPF programs and collect tracing data.
> 	Initialize the list of probes that DTrace recognizes, load BPF
> 	programs and attach them to their corresponding probes, set up
> 	perf event output buffers, and start processing tracing data.
> 
> This implementation makes extensive use of BPF (handled by dt_bpf.c) and
> the perf event output ring buffer (handled by dt_buffer.c).  DTrace-style
> probe handling (dt_probe.c) offers an interface to probes that hides the
> implementation details of the individual probe types by provider (dt_fbt.c
> and dt_syscall.c).  Probe lookup by name uses a hashtable implementation
> (dt_hash.c).  The dt_utils.c code populates a list of online CPU ids, so
> we know what CPUs we can obtain tracing data from.
> 
> Building the tool is trivial because its only dependency (libbpf) is in
> the kernel tree under tools/lib/bpf.  A simple 'make' in the tools/dtrace
> directory suffices.
> 
> The 'dtrace' executable needs to run as root because BPF programs cannot
> be loaded by non-root users.
> 
> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> Reviewed-by: David Mc Lean <david.mclean@oracle.com>
> Reviewed-by: Eugene Loh <eugene.loh@oracle.com>
> ---
>  MAINTAINERS                |   6 +
>  tools/dtrace/Makefile      |  88 ++++++++++
>  tools/dtrace/bpf_sample.c  | 145 ++++++++++++++++
>  tools/dtrace/dt_bpf.c      | 188 +++++++++++++++++++++
>  tools/dtrace/dt_buffer.c   | 331 +++++++++++++++++++++++++++++++++++++
>  tools/dtrace/dt_fbt.c      | 201 ++++++++++++++++++++++
>  tools/dtrace/dt_hash.c     | 211 +++++++++++++++++++++++
>  tools/dtrace/dt_probe.c    | 230 ++++++++++++++++++++++++++
>  tools/dtrace/dt_syscall.c  | 179 ++++++++++++++++++++
>  tools/dtrace/dt_utils.c    | 132 +++++++++++++++
>  tools/dtrace/dtrace.c      | 249 ++++++++++++++++++++++++++++
>  tools/dtrace/dtrace.h      |  13 ++
>  tools/dtrace/dtrace_impl.h | 101 +++++++++++
>  13 files changed, 2074 insertions(+)
>  create mode 100644 tools/dtrace/Makefile
>  create mode 100644 tools/dtrace/bpf_sample.c
>  create mode 100644 tools/dtrace/dt_bpf.c
>  create mode 100644 tools/dtrace/dt_buffer.c
>  create mode 100644 tools/dtrace/dt_fbt.c
>  create mode 100644 tools/dtrace/dt_hash.c
>  create mode 100644 tools/dtrace/dt_probe.c
>  create mode 100644 tools/dtrace/dt_syscall.c
>  create mode 100644 tools/dtrace/dt_utils.c
>  create mode 100644 tools/dtrace/dtrace.c
>  create mode 100644 tools/dtrace/dtrace.h
>  create mode 100644 tools/dtrace/dtrace_impl.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 606d1f80bc49..668468834865 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5474,6 +5474,12 @@ W:	https://linuxtv.org
>  S:	Odd Fixes
>  F:	drivers/media/pci/dt3155/
>  
> +DTRACE
> +M:	Kris Van Hees <kris.van.hees@oracle.com>
> +L:	dtrace-devel@oss.oracle.com
> +S:	Maintained
> +F:	tools/dtrace/
> +
>  DVB_USB_AF9015 MEDIA DRIVER
>  M:	Antti Palosaari <crope@iki.fi>
>  L:	linux-media@vger.kernel.org
> diff --git a/tools/dtrace/Makefile b/tools/dtrace/Makefile
> new file mode 100644
> index 000000000000..99fd0f9dd1d6
> --- /dev/null
> +++ b/tools/dtrace/Makefile
> @@ -0,0 +1,88 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# This Makefile is based on samples/bpf.
> +#
> +# Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
> +
> +DT_VERSION		:= 2.0.0
> +DT_GIT_VERSION		:= $(shell git rev-parse HEAD 2>/dev/null || \
> +				   echo Unknown)
> +
> +DTRACE_PATH		?= $(abspath $(srctree)/$(src))
> +TOOLS_PATH		:= $(DTRACE_PATH)/..
> +SAMPLES_PATH		:= $(DTRACE_PATH)/../../samples
> +
> +hostprogs-y		:= dtrace
> +
> +LIBBPF			:= $(TOOLS_PATH)/lib/bpf/libbpf.a
> +OBJS			:= dt_bpf.o dt_buffer.o dt_utils.o dt_probe.o \
> +			   dt_hash.o \
> +			   dt_fbt.o dt_syscall.o
> +
> +dtrace-objs		:= $(OBJS) dtrace.o
> +
> +always			:= $(hostprogs-y)
> +always			+= bpf_sample.o
> +
> +KBUILD_HOSTCFLAGS	+= -DDT_VERSION=\"$(DT_VERSION)\"
> +KBUILD_HOSTCFLAGS	+= -DDT_GIT_VERSION=\"$(DT_GIT_VERSION)\"
> +KBUILD_HOSTCFLAGS	+= -I$(srctree)/tools/lib
> +KBUILD_HOSTCFLAGS	+= -I$(srctree)/tools/perf

Interesting, what are you using from tools/perf/? So that we can move to
tools/{include,lib,arch}.

> +KBUILD_HOSTCFLAGS	+= -I$(srctree)/tools/include/uapi
> +KBUILD_HOSTCFLAGS	+= -I$(srctree)/tools/include/
> +KBUILD_HOSTCFLAGS	+= -I$(srctree)/usr/include
> +
> +KBUILD_HOSTLDLIBS	:= $(LIBBPF) -lelf
> +
> +LLC			?= llc
> +CLANG			?= clang
> +LLVM_OBJCOPY		?= llvm-objcopy
> +
> +ifdef CROSS_COMPILE
> +HOSTCC			= $(CROSS_COMPILE)gcc
> +CLANG_ARCH_ARGS		= -target $(ARCH)
> +endif
> +
> +all:
> +	$(MAKE) -C ../../ $(CURDIR)/ DTRACE_PATH=$(CURDIR)
> +
> +clean:
> +	$(MAKE) -C ../../ M=$(CURDIR) clean
> +	@rm -f *~
> +
> +$(LIBBPF): FORCE
> +	$(MAKE) -C $(dir $@) RM='rm -rf' LDFLAGS= srctree=$(DTRACE_PATH)/../../ O=
> +
> +FORCE:
> +
> +.PHONY: verify_cmds verify_target_bpf $(CLANG) $(LLC)
> +
> +verify_cmds: $(CLANG) $(LLC)
> +	@for TOOL in $^ ; do \
> +		if ! (which -- "$${TOOL}" > /dev/null 2>&1); then \
> +			echo "*** ERROR: Cannot find LLVM tool $${TOOL}" ;\
> +			exit 1; \
> +		else true; fi; \
> +	done
> +
> +verify_target_bpf: verify_cmds
> +	@if ! (${LLC} -march=bpf -mattr=help > /dev/null 2>&1); then \
> +		echo "*** ERROR: LLVM (${LLC}) does not support 'bpf' target" ;\
> +		echo "   NOTICE: LLVM version >= 3.7.1 required" ;\
> +		exit 2; \
> +	else true; fi
> +
> +$(DTRACE_PATH)/*.c: verify_target_bpf $(LIBBPF)
> +$(src)/*.c: verify_target_bpf $(LIBBPF)
> +
> +$(obj)/%.o: $(src)/%.c
> +	@echo "  CLANG-bpf " $@
> +	$(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) -I$(obj) \
> +		-I$(srctree)/tools/testing/selftests/bpf/ \
> +		-D__KERNEL__ -D__BPF_TRACING__ -Wno-unused-value -Wno-pointer-sign \
> +		-D__TARGET_ARCH_$(ARCH) -Wno-compare-distinct-pointer-types \
> +		-Wno-gnu-variable-sized-type-not-at-end \
> +		-Wno-address-of-packed-member -Wno-tautological-compare \
> +		-Wno-unknown-warning-option $(CLANG_ARCH_ARGS) \
> +		-I$(srctree)/samples/bpf/ -include asm_goto_workaround.h \
> +		-O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf $(LLC_FLAGS) -filetype=obj -o $@


We have the above in tools/perf/util/llvm-utils.c, perhaps we need to
move it to some place in lib/ to share?

> diff --git a/tools/dtrace/bpf_sample.c b/tools/dtrace/bpf_sample.c
> new file mode 100644
> index 000000000000..49f350390b5f
> --- /dev/null
> +++ b/tools/dtrace/bpf_sample.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * This sample DTrace BPF tracing program demonstrates how actions can be
> + * associated with different probe types.
> + *
> + * The kprobe/ksys_write probe is a Function Boundary Tracing (FBT) entry probe
> + * on the ksys_write(fd, buf, count) function in the kernel.  Arguments to the
> + * function can be retrieved from the CPU registers (struct pt_regs).
> + *
> + * The tracepoint/syscalls/sys_enter_write probe is a System Call entry probe
> + * for the write(d, buf, count) system call.  Arguments to the system call can
> + * be retrieved from the tracepoint data passed to the BPF program as context
> + * struct syscall_data) when the probe fires.
> + *
> + * The BPF program associated with each probe prepares a DTrace BPF context
> + * (struct dt_bpf_context) that stores the probe ID and up to 10 arguments.
> + * Only 3 arguments are used in this sample.  Then the prorgams call a shared
> + * BPF function (bpf_action) that implements the actual action to be taken when
> + * a probe fires.  It prepares a data record to be stored in the tracing buffer
> + * and submits it to the buffer.  The data in the data record is obtained from
> + * the DTrace BPF context.
> + *
> + * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
> + */
> +#include <uapi/linux/bpf.h>
> +#include <linux/ptrace.h>
> +#include <linux/version.h>
> +#include <uapi/linux/unistd.h>
> +#include "bpf_helpers.h"
> +
> +#include "dtrace.h"
> +
> +struct syscall_data {
> +	struct pt_regs *regs;
> +	long syscall_nr;
> +	long arg[6];
> +};
> +
> +struct bpf_map_def SEC("maps") buffers = {
> +	.type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
> +	.key_size = sizeof(u32),
> +	.value_size = sizeof(u32),
> +	.max_entries = NR_CPUS,
> +};
> +
> +#if defined(__amd64)
> +# define GET_REGS_ARG0(regs)	((regs)->di)
> +# define GET_REGS_ARG1(regs)	((regs)->si)
> +# define GET_REGS_ARG2(regs)	((regs)->dx)
> +# define GET_REGS_ARG3(regs)	((regs)->cx)
> +# define GET_REGS_ARG4(regs)	((regs)->r8)
> +# define GET_REGS_ARG5(regs)	((regs)->r9)
> +#else
> +# warning Argument retrieval from pt_regs is not supported yet on this arch.
> +# define GET_REGS_ARG0(regs)	0
> +# define GET_REGS_ARG1(regs)	0
> +# define GET_REGS_ARG2(regs)	0
> +# define GET_REGS_ARG3(regs)	0
> +# define GET_REGS_ARG4(regs)	0
> +# define GET_REGS_ARG5(regs)	0
> +#endif

We have this in tools/testing/selftests/bpf/bpf_helpers.h, probably need
to move to some other place in tools/include/ where this can be shared.

- Arnaldo

^ permalink raw reply

* [PATCH net-next] sctp: remove rcu_read_lock from sctp_bind_addr_state
From: Xin Long @ 2019-07-08 16:59 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem

sctp_bind_addr_state() is called either in packet rcv path or
by sctp_copy_local_addr_list(), which are under rcu_read_lock.
So there's no need to call it again in sctp_bind_addr_state().

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/bind_addr.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c
index f54333c..53bc615 100644
--- a/net/sctp/bind_addr.c
+++ b/net/sctp/bind_addr.c
@@ -393,24 +393,19 @@ int sctp_bind_addr_state(const struct sctp_bind_addr *bp,
 {
 	struct sctp_sockaddr_entry *laddr;
 	struct sctp_af *af;
-	int state = -1;
 
 	af = sctp_get_af_specific(addr->sa.sa_family);
 	if (unlikely(!af))
-		return state;
+		return -1;
 
-	rcu_read_lock();
 	list_for_each_entry_rcu(laddr, &bp->address_list, list) {
 		if (!laddr->valid)
 			continue;
-		if (af->cmp_addr(&laddr->a, addr)) {
-			state = laddr->state;
-			break;
-		}
+		if (af->cmp_addr(&laddr->a, addr))
+			return laddr->state;
 	}
-	rcu_read_unlock();
 
-	return state;
+	return -1;
 }
 
 /* Find the first address in the bind address list that is not present in
-- 
2.1.0


^ permalink raw reply related

* Re: [PATCH rdma-next 0/2] DEVX VHCA tunnel support
From: Jason Gunthorpe @ 2019-07-08 16:58 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Max Gurtovoy,
	Yishai Hadas, Saeed Mahameed, linux-netdev
In-Reply-To: <20190701181402.25286-1-leon@kernel.org>

On Mon, Jul 01, 2019 at 09:14:00PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
> 
> Hi,
> 
> Those two patches introduce VHCA tunnel mechanism to DEVX interface
> needed for Bluefield SOC. See extensive commit messages for more
> information.
> 
> Thanks
> 
> Max Gurtovoy (2):
>   net/mlx5: Introduce VHCA tunnel device capability
>   IB/mlx5: Implement VHCA tunnel mechanism in DEVX

Thanks, applied to for-next

Jason

^ permalink raw reply

* [PATCH net-next 4/4] sctp: rename sp strm_interleave to ep intl_enable
From: Xin Long @ 2019-07-08 16:57 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
In-Reply-To: <cover.1562604972.git.lucien.xin@gmail.com>

Like other endpoint features, strm_interleave should be moved to
sctp_endpoint and renamed to intl_enable.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/structs.h | 2 +-
 net/sctp/sm_make_chunk.c   | 4 ++--
 net/sctp/socket.c          | 8 ++++----
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index c41b57b..ba5c4f6 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -219,7 +219,6 @@ struct sctp_sock {
 		disable_fragments:1,
 		v4mapped:1,
 		frag_interleave:1,
-		strm_interleave:1,
 		recvrcvinfo:1,
 		recvnxtinfo:1,
 		data_ready_signalled:1;
@@ -1324,6 +1323,7 @@ struct sctp_endpoint {
 	struct list_head endpoint_shared_keys;
 	__u16 active_key_id;
 	__u8  auth_enable:1,
+	      intl_enable:1,
 	      prsctp_enable:1,
 	      reconf_enable:1;
 
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 31ab2c6..ed39396 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -269,7 +269,7 @@ struct sctp_chunk *sctp_make_init(const struct sctp_association *asoc,
 	if (sp->adaptation_ind)
 		chunksize += sizeof(aiparam);
 
-	if (sp->strm_interleave) {
+	if (asoc->ep->intl_enable) {
 		extensions[num_ext] = SCTP_CID_I_DATA;
 		num_ext += 1;
 	}
@@ -2027,7 +2027,7 @@ static void sctp_process_ext_param(struct sctp_association *asoc,
 				asoc->peer.asconf_capable = 1;
 			break;
 		case SCTP_CID_I_DATA:
-			if (sctp_sk(asoc->base.sk)->strm_interleave)
+			if (asoc->ep->intl_enable)
 				asoc->peer.intl_capable = 1;
 			break;
 		default:
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index b679b61..0fc5b90 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1913,7 +1913,7 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
 		if (err)
 			goto err;
 
-		if (sp->strm_interleave) {
+		if (asoc->ep->intl_enable) {
 			timeo = sock_sndtimeo(sk, 0);
 			err = sctp_wait_for_connect(asoc, &timeo);
 			if (err) {
@@ -3581,7 +3581,7 @@ static int sctp_setsockopt_fragment_interleave(struct sock *sk,
 	sctp_sk(sk)->frag_interleave = !!val;
 
 	if (!sctp_sk(sk)->frag_interleave)
-		sctp_sk(sk)->strm_interleave = 0;
+		sctp_sk(sk)->ep->intl_enable = 0;
 
 	return 0;
 }
@@ -4484,7 +4484,7 @@ static int sctp_setsockopt_interleaving_supported(struct sock *sk,
 		goto out;
 	}
 
-	sp->strm_interleave = !!params.assoc_value;
+	sp->ep->intl_enable = !!params.assoc_value;
 
 	retval = 0;
 
@@ -7711,7 +7711,7 @@ static int sctp_getsockopt_interleaving_supported(struct sock *sk, int len,
 	}
 
 	params.assoc_value = asoc ? asoc->peer.intl_capable
-				  : sctp_sk(sk)->strm_interleave;
+				  : sctp_sk(sk)->ep->intl_enable;
 
 	if (put_user(len, optlen))
 		goto out;
-- 
2.1.0


^ permalink raw reply related

* [PATCH net-next 3/4] sctp: rename asoc intl_enable to asoc peer.intl_capable
From: Xin Long @ 2019-07-08 16:57 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
In-Reply-To: <cover.1562604972.git.lucien.xin@gmail.com>

To keep consistent with other asoc features, we move intl_enable
to peer.intl_capable in asoc.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/structs.h   | 33 +++++++++++++++++----------------
 net/sctp/sm_make_chunk.c     |  4 ++--
 net/sctp/socket.c            |  2 +-
 net/sctp/stream_interleave.c |  4 ++--
 net/sctp/stream_sched.c      |  2 +-
 5 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 7f35b8e..c41b57b 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1679,28 +1679,30 @@ struct sctp_association {
 		__be16 addip_disabled_mask;
 
 		/* These are capabilities which our peer advertised.  */
-		__u8	ecn_capable:1,      /* Can peer do ECN? */
+		__u16	ecn_capable:1,      /* Can peer do ECN? */
 			ipv4_address:1,     /* Peer understands IPv4 addresses? */
 			ipv6_address:1,     /* Peer understands IPv6 addresses? */
 			hostname_address:1, /* Peer understands DNS addresses? */
 			asconf_capable:1,   /* Does peer support ADDIP? */
 			prsctp_capable:1,   /* Can peer do PR-SCTP? */
 			reconf_capable:1,   /* Can peer do RE-CONFIG? */
-			auth_capable:1;     /* Is peer doing SCTP-AUTH? */
-
-		/* sack_needed : This flag indicates if the next received
-		 *             : packet is to be responded to with a
-		 *             : SACK. This is initialized to 0.  When a packet
-		 *             : is received sack_cnt is incremented. If this value
-		 *             : reaches 2 or more, a SACK is sent and the
-		 *             : value is reset to 0. Note: This is used only
-		 *             : when no DATA chunks are received out of
-		 *             : order.  When DATA chunks are out of order,
-		 *             : SACK's are not delayed (see Section 6).
-		 */
-		__u8    sack_needed:1,     /* Do we need to sack the peer? */
+			intl_capable:1,     /* Can peer do INTERLEAVE */
+			auth_capable:1,     /* Is peer doing SCTP-AUTH? */
+			/* sack_needed:
+			 *   This flag indicates if the next received
+			 *   packet is to be responded to with a
+			 *   SACK. This is initialized to 0.  When a packet
+			 *   is received sack_cnt is incremented. If this value
+			 *   reaches 2 or more, a SACK is sent and the
+			 *   value is reset to 0. Note: This is used only
+			 *   when no DATA chunks are received out of
+			 *   order.  When DATA chunks are out of order,
+			 *   SACK's are not delayed (see Section 6).
+			 */
+			sack_needed:1,     /* Do we need to sack the peer? */
 			sack_generation:1,
 			zero_window_announced:1;
+
 		__u32	sack_cnt;
 
 		__u32   adaptation_ind;	 /* Adaptation Code point. */
@@ -2049,8 +2051,7 @@ struct sctp_association {
 
 	__u8 need_ecne:1,	/* Need to send an ECNE Chunk? */
 	     temp:1,		/* Is it a temporary association? */
-	     force_delay:1,
-	     intl_enable:1;
+	     force_delay:1;
 
 	__u8 strreset_enable;
 	__u8 strreset_outstanding; /* request param count on the fly */
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 227bbac..31ab2c6 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -438,7 +438,7 @@ struct sctp_chunk *sctp_make_init_ack(const struct sctp_association *asoc,
 	if (sp->adaptation_ind)
 		chunksize += sizeof(aiparam);
 
-	if (asoc->intl_enable) {
+	if (asoc->peer.intl_capable) {
 		extensions[num_ext] = SCTP_CID_I_DATA;
 		num_ext += 1;
 	}
@@ -2028,7 +2028,7 @@ static void sctp_process_ext_param(struct sctp_association *asoc,
 			break;
 		case SCTP_CID_I_DATA:
 			if (sctp_sk(asoc->base.sk)->strm_interleave)
-				asoc->intl_enable = 1;
+				asoc->peer.intl_capable = 1;
 			break;
 		default:
 			break;
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index da2a3c2..b679b61 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -7710,7 +7710,7 @@ static int sctp_getsockopt_interleaving_supported(struct sock *sk, int len,
 		goto out;
 	}
 
-	params.assoc_value = asoc ? asoc->intl_enable
+	params.assoc_value = asoc ? asoc->peer.intl_capable
 				  : sctp_sk(sk)->strm_interleave;
 
 	if (put_user(len, optlen))
diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
index afbf122..40c40be 100644
--- a/net/sctp/stream_interleave.c
+++ b/net/sctp/stream_interleave.c
@@ -1358,6 +1358,6 @@ void sctp_stream_interleave_init(struct sctp_stream *stream)
 	struct sctp_association *asoc;
 
 	asoc = container_of(stream, struct sctp_association, stream);
-	stream->si = asoc->intl_enable ? &sctp_stream_interleave_1
-				       : &sctp_stream_interleave_0;
+	stream->si = asoc->peer.intl_capable ? &sctp_stream_interleave_1
+					     : &sctp_stream_interleave_0;
 }
diff --git a/net/sctp/stream_sched.c b/net/sctp/stream_sched.c
index b8fa7ab..99e5f69 100644
--- a/net/sctp/stream_sched.c
+++ b/net/sctp/stream_sched.c
@@ -228,7 +228,7 @@ int sctp_sched_get_value(struct sctp_association *asoc, __u16 sid,
 void sctp_sched_dequeue_done(struct sctp_outq *q, struct sctp_chunk *ch)
 {
 	if (!list_is_last(&ch->frag_list, &ch->msg->chunks) &&
-	    !q->asoc->intl_enable) {
+	    !q->asoc->peer.intl_capable) {
 		struct sctp_stream_out *sout;
 		__u16 sid;
 
-- 
2.1.0


^ permalink raw reply related

* [PATCH net-next 2/4] sctp: remove prsctp_enable from asoc
From: Xin Long @ 2019-07-08 16:57 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
In-Reply-To: <cover.1562604972.git.lucien.xin@gmail.com>

Like reconf_enable, prsctp_enable should also be removed from asoc,
as asoc->peer.prsctp_capable has taken its job.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/structs.h | 3 +--
 net/sctp/associola.c       | 1 -
 net/sctp/sm_make_chunk.c   | 8 ++++----
 net/sctp/socket.c          | 2 +-
 4 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index d9e0e1a..7f35b8e 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -2050,8 +2050,7 @@ struct sctp_association {
 	__u8 need_ecne:1,	/* Need to send an ECNE Chunk? */
 	     temp:1,		/* Is it a temporary association? */
 	     force_delay:1,
-	     intl_enable:1,
-	     prsctp_enable:1;
+	     intl_enable:1;
 
 	__u8 strreset_enable;
 	__u8 strreset_outstanding; /* request param count on the fly */
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 321c199..5010cce 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -261,7 +261,6 @@ static struct sctp_association *sctp_association_init(
 		goto stream_free;
 
 	asoc->active_key_id = ep->active_key_id;
-	asoc->prsctp_enable = ep->prsctp_enable;
 	asoc->strreset_enable = ep->strreset_enable;
 
 	/* Save the hmacs and chunks list into this association */
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index d784dc1..227bbac 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -247,7 +247,7 @@ struct sctp_chunk *sctp_make_init(const struct sctp_association *asoc,
 	chunksize += SCTP_PAD4(SCTP_SAT_LEN(num_types));
 	chunksize += sizeof(ecap_param);
 
-	if (asoc->prsctp_enable)
+	if (asoc->ep->prsctp_enable)
 		chunksize += sizeof(prsctp_param);
 
 	/* ADDIP: Section 4.2.7:
@@ -348,7 +348,7 @@ struct sctp_chunk *sctp_make_init(const struct sctp_association *asoc,
 		sctp_addto_param(retval, num_ext, extensions);
 	}
 
-	if (asoc->prsctp_enable)
+	if (asoc->ep->prsctp_enable)
 		sctp_addto_chunk(retval, sizeof(prsctp_param), &prsctp_param);
 
 	if (sp->adaptation_ind) {
@@ -2011,7 +2011,7 @@ static void sctp_process_ext_param(struct sctp_association *asoc,
 				asoc->peer.reconf_capable = 1;
 			break;
 		case SCTP_CID_FWD_TSN:
-			if (asoc->prsctp_enable && !asoc->peer.prsctp_capable)
+			if (asoc->ep->prsctp_enable)
 				asoc->peer.prsctp_capable = 1;
 			break;
 		case SCTP_CID_AUTH:
@@ -2636,7 +2636,7 @@ static int sctp_process_param(struct sctp_association *asoc,
 		break;
 
 	case SCTP_PARAM_FWD_TSN_SUPPORT:
-		if (asoc->prsctp_enable) {
+		if (asoc->ep->prsctp_enable) {
 			asoc->peer.prsctp_capable = 1;
 			break;
 		}
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 0424876..da2a3c2 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -7343,7 +7343,7 @@ static int sctp_getsockopt_pr_supported(struct sock *sk, int len,
 		goto out;
 	}
 
-	params.assoc_value = asoc ? asoc->prsctp_enable
+	params.assoc_value = asoc ? asoc->peer.prsctp_capable
 				  : sctp_sk(sk)->ep->prsctp_enable;
 
 	if (put_user(len, optlen))
-- 
2.1.0


^ permalink raw reply related

* [PATCH net-next 1/4] sctp: remove reconf_enable from asoc
From: Xin Long @ 2019-07-08 16:57 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
In-Reply-To: <cover.1562604972.git.lucien.xin@gmail.com>

asoc's reconf support is actually decided by the 4-shakehand negotiation,
not something that users can set by sockopt. asoc->peer.reconf_capable is
working for this. So remove it from asoc.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/structs.h | 3 +--
 net/sctp/associola.c       | 1 -
 net/sctp/sm_make_chunk.c   | 5 ++---
 net/sctp/socket.c          | 7 ++-----
 4 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 0767701..d9e0e1a 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -2051,8 +2051,7 @@ struct sctp_association {
 	     temp:1,		/* Is it a temporary association? */
 	     force_delay:1,
 	     intl_enable:1,
-	     prsctp_enable:1,
-	     reconf_enable:1;
+	     prsctp_enable:1;
 
 	__u8 strreset_enable;
 	__u8 strreset_outstanding; /* request param count on the fly */
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 1999237..321c199 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -262,7 +262,6 @@ static struct sctp_association *sctp_association_init(
 
 	asoc->active_key_id = ep->active_key_id;
 	asoc->prsctp_enable = ep->prsctp_enable;
-	asoc->reconf_enable = ep->reconf_enable;
 	asoc->strreset_enable = ep->strreset_enable;
 
 	/* Save the hmacs and chunks list into this association */
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 9b0e5b0..d784dc1 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -261,7 +261,7 @@ struct sctp_chunk *sctp_make_init(const struct sctp_association *asoc,
 		num_ext += 2;
 	}
 
-	if (asoc->reconf_enable) {
+	if (asoc->ep->reconf_enable) {
 		extensions[num_ext] = SCTP_CID_RECONF;
 		num_ext += 1;
 	}
@@ -2007,8 +2007,7 @@ static void sctp_process_ext_param(struct sctp_association *asoc,
 	for (i = 0; i < num_ext; i++) {
 		switch (param.ext->chunks[i]) {
 		case SCTP_CID_RECONF:
-			if (asoc->reconf_enable &&
-			    !asoc->peer.reconf_capable)
+			if (asoc->ep->reconf_enable)
 				asoc->peer.reconf_capable = 1;
 			break;
 		case SCTP_CID_FWD_TSN:
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 39ea0a3..0424876 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4226,10 +4226,7 @@ static int sctp_setsockopt_reconfig_supported(struct sock *sk,
 	    sctp_style(sk, UDP))
 		goto out;
 
-	if (asoc)
-		asoc->reconf_enable = !!params.assoc_value;
-	else
-		sctp_sk(sk)->ep->reconf_enable = !!params.assoc_value;
+	sctp_sk(sk)->ep->reconf_enable = !!params.assoc_value;
 
 	retval = 0;
 
@@ -7554,7 +7551,7 @@ static int sctp_getsockopt_reconfig_supported(struct sock *sk, int len,
 		goto out;
 	}
 
-	params.assoc_value = asoc ? asoc->reconf_enable
+	params.assoc_value = asoc ? asoc->peer.reconf_capable
 				  : sctp_sk(sk)->ep->reconf_enable;
 
 	if (put_user(len, optlen))
-- 
2.1.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox