Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller driver

[Benjamin Poirier]

On 2016/07/26 11:16, Andreas Werner wrote:
[...]
> +
> + /* Lock for CTL_BTR register access.
> +  * This register combines bittiming bits
> +  * and the operation mode bits.
> +  * It is also used for bit r/m/w access
> +  * to all registers.
> +  */
> + spinlock_t lock;

Why not use 80 cols for comments?

[...]
> +
> +static int men_z192_xmit(struct sk_buff *skb, struct net_device *ndev)
> +{
> + struct can_frame *cf = (struct can_frame *)skb->data;
> + struct men_z192 *priv = netdev_priv(ndev);
> + struct men_z192_regs __iomem *regs = priv->regs;
> + struct net_device_stats *stats = >stats;
> + struct men_z192_cf_buf __iomem *cf_buf;
> + u32 data[2] = {0, 0};
> + int status;
> + u32 id;
> +
> + if (can_dropped_invalid_skb(ndev, skb))
> + return NETDEV_TX_OK;
> +
> + status = readl(>rx_tx_sts);
> +
> + if (MEN_Z192_TX_BUF_CNT(status) >= 255) {
> + netif_stop_queue(ndev);
> + netdev_err(ndev, "not enough space in TX buffer\n");
> +
> + return NETDEV_TX_BUSY;
> + }
> +
> + cf_buf = priv->dev_base + MEN_Z192_TX_BUF_START;
> +
> + if (cf->can_id & CAN_EFF_FLAG) {
> + /* Extended frame */
> + id = ((cf->can_id & CAN_EFF_MASK) <<
> + MEN_Z192_CFBUF_ID2_SHIFT) & MEN_Z192_CFBUF_ID2;
> +
> + id |= (((cf->can_id & CAN_EFF_MASK) >>
> + (CAN_EFF_ID_BITS - CAN_SFF_ID_BITS)) <<
> +  MEN_Z192_CFBUF_ID1_SHIFT) & MEN_Z192_CFBUF_ID1;
> +
> + id |= MEN_Z192_CFBUF_IDE;
> + id |= MEN_Z192_CFBUF_SRR;
> +
> + if (cf->can_id & CAN_RTR_FLAG)
> + id |= MEN_Z192_CFBUF_E_RTR;
> + } else {
> + /* Standard frame */
> + id = ((cf->can_id & CAN_SFF_MASK) <<
> +MEN_Z192_CFBUF_ID1_SHIFT) & MEN_Z192_CFBUF_ID1;
> +
> + if (cf->can_id & CAN_RTR_FLAG)
> + id |= MEN_Z192_CFBUF_S_RTR;
> + }
> +
> + if (cf->can_dlc > 0)
> + data[0] = be32_to_cpup((__be32 *)(cf->data));
> + if (cf->can_dlc > 3)
> + data[1] = be32_to_cpup((__be32 *)(cf->data + 4));
> +
> + writel(id, _buf->can_id);
> + writel(cf->can_dlc, _buf->length);
> +
> + if (!(cf->can_id & CAN_RTR_FLAG)) {
> + writel(data[0], _buf->data[0]);
> + writel(data[1], _buf->data[1]);
> +
> + stats->tx_bytes += cf->can_dlc;
> + }
> +
> + /* be sure everything is written to the
> +  * device before acknowledge the data.
> +  */
> + mmiowb();
> +
> + /* trigger the transmission */
> + men_z192_ack_tx_pkg(priv, 1);
> +
> + stats->tx_packets++;
> +
> + kfree_skb(skb);

What prevents the skb data to be freed/reused before the device has
accessed it?

[...]
> +
> +static int men_z192_probe(struct mcb_device *mdev,
> +   const struct mcb_device_id *id)
> +{
> + struct device *dev = >dev;
> + struct men_z192 *priv;
> + struct net_device *ndev;
> + void __iomem *dev_base;
> + struct resource *mem;
> + u32 timebase;
> + int ret = 0;
> + int irq;
> +
> + mem = mcb_request_mem(mdev, dev_name(dev));
> + if (IS_ERR(mem)) {
> + dev_err(dev, "failed to request device memory");
> + return PTR_ERR(mem);
> + }
> +
> + dev_base = ioremap(mem->start, resource_size(mem));
> + if (!dev_base) {
> + dev_err(dev, "failed to ioremap device memory");
> + ret = -ENXIO;
> + goto out_release;
> + }
> +
> + irq = mcb_get_irq(mdev);
> + if (irq <= 0) {
> + ret = -ENODEV;
> + goto out_unmap;
> + }
> +
> + ndev = alloc_candev(sizeof(struct men_z192), 1);
> + if (!ndev) {
> + dev_err(dev, "failed to allocate the can device");
> + ret = -ENOMEM;
> + goto out_unmap;
> + }
> +
> + ndev->netdev_ops = _z192_netdev_ops;
> + ndev->irq = irq;
> +
> + priv = netdev_priv(ndev);
> + priv->ndev = ndev;
> + priv->dev = dev;
> +
> + priv->mem = mem;
> + priv->dev_base = dev_base;
> + priv->regs = priv->dev_base + MEN_Z192_REGS_OFFS;
> +
> + timebase = readl(>regs->timebase);
> + if (!timebase) {
> + dev_err(dev, "invalid timebase configured (timebase=%d)\n",
> + timebase);
> + ret = -EINVAL;
> + goto out_unmap;

free_candev is missing in this error path

> + }
> +
> + priv->can.clock.freq = timebase;
> + priv->can.bittiming_const = _z192_bittiming_const;
> + priv->can.do_set_mode = men_z192_set_mode;
> + priv->can.do_get_berr_counter = men_z192_get_berr_counter;
> + priv->can.ctrlmode_supported = CAN_CTRLMODE_LISTENONLY |
> +CAN_CTRLMODE_3_SAMPLES |
> +

Re: [net-next 0/2] BPF, kprobes: Add current_in_cgroup helper

[Alexei Starovoitov]

On Sun, Aug 07, 2016 at 08:08:19PM -0700, Sargun Dhillon wrote:
> Thanks for your feedback Alexei,
> I really appreciate it.
> 
> On Sun, Aug 07, 2016 at 05:52:36PM -0700, Alexei Starovoitov wrote:
> > On Sat, Aug 06, 2016 at 09:56:06PM -0700, Sargun Dhillon wrote:
> > > On Sat, Aug 06, 2016 at 09:32:05PM -0700, Alexei Starovoitov wrote:
> > > > On Sat, Aug 06, 2016 at 09:06:53PM -0700, Sargun Dhillon wrote:
> > > > > This patchset includes a helper and an example to determine whether 
> > > > > the kprobe 
> > > > > is currently executing in the context of a specific cgroup based on a 
> > > > > cgroup
> > > > > bpf map / array. 
> > > > 
> > > > description is too short to understand how this new helper is going to 
> > > > be used.
> > > > depending on kprobe current is not always valid.
> > > Anything not in in_interrupt() should have a current, right?
> > > 
> > > > what are you trying to achieve?
> > > This is primarily to help troubleshoot containers (Docker, and now 
> > > systemd). A 
> > > lot of the time we want to determine what's going on in a given container 
> > > (opening files, connecting to systems, etc...). There's not really a 
> > > great way 
> > > to restrict to containers except by manually walking datastructures to 
> > > check for 
> > > the right cgroup. This seems like a better alternative.
> > 
> > so it's about restricting or determining?
> > In other words if it's analytics/tracing that's one thing, but
> > enforcement/restriction is quite different.
> > For analytics one can walk task_css_set(current)->dfl_cgrp and remember
> > that pointer in a map or something for stats collections and similar.
> > If it's restricting apps in containers then kprobe approach
> > is not usable. I don't think you'd want to built an enforcement system
> > on an unstable api then can vary kernel-to-kernel.
> > 
> The first real-world use case are to implement something like Sysdig. Often 
> the 
> team running the team running the containers don't always know what's inside 
> of 
> them, so they want to be able to view network, I/O, and other activity by 
> container. Right now, the lowest common denominator between all of the 
> containerization techniques is cgroups. We've seen examples of where a admin 
> is 
> unsure of the workload, and would love to use opensnoop, but there are too 
> many 
> workloads on the machine.

Indeed it would be a useful feature to teach opensnoop to filter by a cgroup
and all descentants of it. If you can prepare a patch for it that would be
a strong use case for this bpf_current_in_cgroup helper and solid justification
to accept it in the kernel.
Something like cgroupv2 string path as an argument ?

> Unfortunately, I don't think that it's possible just to check 
> task_css_set(current)->dfl_cgrp in a bpf program. The container, especially 
> containers with sidecars (what Kubernetes calls Pods, I believe?) tend to 
> have 
> multiple nested cgroups inside of them. If you had a way to convert cgroup 
> array 
> entries to pointers, I imagine you could write an unrolled loop to check for 
> ownership within a limited range.
> 
> I'm still looking for comments from the LSM folks on Checmate[1]. It appears 
> that there has been very little churn in the LSM hooks API that's 
> API-breaking. 
> For many of syscall hooks, they're closely tied to the syscall API, so they 
> can't really change too much. I think that a toolkit like iovisor, or another 
> userland translation layer, these hooks could be very powerful. I would love 
> to 
> hear feedback from the LSM folks.
> 
> My plan with those patches is to reimplement Yama, and Hardchroot in BPF 
> programs to show off the potential capabilities of Checmate. I'd also like to 
> create some example programs blocking CVEs that have popped up. I think of 
> the 
> idea like nftables for kernel syscalls, storage, and the network stack.

looking forward to more details on checmate, so far I'm convinced we need it.

> The other example I want to show is implementing Docker-bridge style network 
> isolation with Checmate. Most folks use it to map ports, and to restrict 
> binding 
> to specific ports, and not the dedicated network namespace, or loopback 
> interface. It turns out for some applications this comes at a pretty 
> significant 
> hit[2][3], as well as awkward upper bounds based on conntrack.

the default nat setup of docker is obviously slow, but that doesn't mean
kernel needs anything more than it already has.
If you're at linuxcon this year, Thomas's talk [4] shouldn't be missed.

> > > > This looks like an alternative to lsm patches submitted earlier?
> > > No. But I would like to use this helper in the LSM patches I'm working 
> > > on. For 
> > > now, with those patches, and this helper, I can create a map sized 1, and 
> > > add 
> > > the cgroup I care about to it. Given I can add as many bpf programs to an 
> > > LSM
> > > hook I want, I can use this mechanism to "attach BPF programs to cgroups" 
> > 

Re: [PATCH v4 1/1] rps: Inspect PPTP encapsulated by GRE to get flow hash

[Philp Prindeville]

Feng,

An anonymous structure is defined here: 
https://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html


i.e.:

struct gre_full_hdr {
struct gre_base_hdr;
...

so yes, I'm talking about making fixed_header be anonymous instead.

-Philip


On 08/07/2016 08:50 PM, Feng Gao wrote:

Hi Philp,

Forgive my poor English, I am not clear about the comment #1.
"Can you make gre_base_hdr be anonymous?".

+struct gre_full_hdr {
+   struct gre_base_hdr fixed_header;

Do you mean make the member "fixed_header" as anonymous or not?

Best Regards
Feng


On Mon, Aug 8, 2016 at 5:03 AM, Philp Prindeville
 wrote:

Inline...



On 08/04/2016 01:06 AM, f...@48lvckh6395k16k5.yundunddos.com wrote:

From: Gao Feng 

The PPTP is encapsulated by GRE header with that GRE_VERSION bits
must contain one. But current GRE RPS needs the GRE_VERSION must be
zero. So RPS does not work for PPTP traffic.

In my test environment, there are four MIPS cores, and all traffic
are passed through by PPTP. As a result, only one core is 100% busy
while other three cores are very idle. After this patch, the usage
of four cores are balanced well.

Signed-off-by: Gao Feng 
---
   v4: 1) Define struct gre_full_hdr, and use sizeof its member directly;
   2) Move version and routing check ahead;
   3) Only PPTP in GRE check the ack flag;
   v3: 1) Move struct pptp_gre_header defination into new file pptp.h
   2) Use sizeof GRE and PPTP type instead of literal value;
   3) Remove strict flag check for PPTP to robust;
   4) Consolidate the codes again;
   v2: Update according to Tom and Philp's advice.
   1) Consolidate the codes with GRE version 0 path;
   2) Use PPP_PROTOCOL to get ppp protol;
   3) Set the FLOW_DIS_ENCAPSULATION flag;
   v1: Intial Patch

   drivers/net/ppp/pptp.c |  36 +
   include/net/gre.h  |  10 +++-
   include/net/pptp.h |  40 +++
   include/uapi/linux/if_tunnel.h |   7 ++-
   net/core/flow_dissector.c  | 113
-
   5 files changed, 135 insertions(+), 71 deletions(-)
   create mode 100644 include/net/pptp.h

diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c
index ae0905e..3e68dbc 100644
--- a/drivers/net/ppp/pptp.c
+++ b/drivers/net/ppp/pptp.c
@@ -37,6 +37,7 @@
   #include 
   #include 
   #include 
+#include 
 #include 
   @@ -53,41 +54,6 @@ static struct proto pptp_sk_proto __read_mostly;
   static const struct ppp_channel_ops pptp_chan_ops;
   static const struct proto_ops pptp_ops;
   -#define PPP_LCP_ECHOREQ 0x09
-#define PPP_LCP_ECHOREP 0x0A
-#define SC_RCV_BITS(SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP)
-
-#define MISSING_WINDOW 20
-#define WRAPPED(curseq, lastseq)\
-   curseq) & 0xff00) == 0) &&\
-   (((lastseq) & 0xff00) == 0xff00))
-
-#define PPTP_GRE_PROTO  0x880B
-#define PPTP_GRE_VER0x1
-
-#define PPTP_GRE_FLAG_C0x80
-#define PPTP_GRE_FLAG_R0x40
-#define PPTP_GRE_FLAG_K0x20
-#define PPTP_GRE_FLAG_S0x10
-#define PPTP_GRE_FLAG_A0x80
-
-#define PPTP_GRE_IS_C(f) ((f)_GRE_FLAG_C)
-#define PPTP_GRE_IS_R(f) ((f)_GRE_FLAG_R)
-#define PPTP_GRE_IS_K(f) ((f)_GRE_FLAG_K)
-#define PPTP_GRE_IS_S(f) ((f)_GRE_FLAG_S)
-#define PPTP_GRE_IS_A(f) ((f)_GRE_FLAG_A)
-
-#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header))
-struct pptp_gre_header {
-   u8  flags;
-   u8  ver;
-   __be16 protocol;
-   __be16 payload_len;
-   __be16 call_id;
-   __be32 seq;
-   __be32 ack;
-} __packed;
-
   static struct pppox_sock *lookup_chan(u16 call_id, __be32 s_addr)
   {
 struct pppox_sock *sock;
diff --git a/include/net/gre.h b/include/net/gre.h
index 7a54a31..c469dcc 100644
--- a/include/net/gre.h
+++ b/include/net/gre.h
@@ -7,9 +7,17 @@
   struct gre_base_hdr {
 __be16 flags;
 __be16 protocol;
-};
+} __packed;
   #define GRE_HEADER_SECTION 4
   +struct gre_full_hdr {
+   struct gre_base_hdr fixed_header;


Can you make gre_base_hdr be anonymous?




+   __be16 csum;
+   __be16 reserved1;
+   __be32 key;
+   __be32 seq;
+} __packed;
+
   #define GREPROTO_CISCO0
   #define GREPROTO_PPTP 1
   #define GREPROTO_MAX  2
diff --git a/include/net/pptp.h b/include/net/pptp.h
new file mode 100644
index 000..301d3e2
--- /dev/null
+++ b/include/net/pptp.h
@@ -0,0 +1,40 @@
+#ifndef _NET_PPTP_H
+#define _NET_PPTP_H
+
+#define PPP_LCP_ECHOREQ 0x09
+#define PPP_LCP_ECHOREP 0x0A
+#define SC_RCV_BITS (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP)
+
+#define MISSING_WINDOW 20
+#define WRAPPED(curseq, lastseq)\
+   curseq) & 0xff00) == 0) &&\
+   (((lastseq) & 0xff00) == 0xff00))
+
+#define PPTP_GRE_PROTO  0x880B
+#define PPTP_GRE_VER0x1
+
+#define PPTP_GRE_FLAG_C 0x80
+#define PPTP_GRE_FLAG_R 0x40
+#define PPTP_GRE_FLAG_K 0x20

Re: [net-next 0/2] BPF, kprobes: Add current_in_cgroup helper

[Sargun Dhillon]

Thanks for your feedback Alexei,
I really appreciate it.

On Sun, Aug 07, 2016 at 05:52:36PM -0700, Alexei Starovoitov wrote:
> On Sat, Aug 06, 2016 at 09:56:06PM -0700, Sargun Dhillon wrote:
> > On Sat, Aug 06, 2016 at 09:32:05PM -0700, Alexei Starovoitov wrote:
> > > On Sat, Aug 06, 2016 at 09:06:53PM -0700, Sargun Dhillon wrote:
> > > > This patchset includes a helper and an example to determine whether the 
> > > > kprobe 
> > > > is currently executing in the context of a specific cgroup based on a 
> > > > cgroup
> > > > bpf map / array. 
> > > 
> > > description is too short to understand how this new helper is going to be 
> > > used.
> > > depending on kprobe current is not always valid.
> > Anything not in in_interrupt() should have a current, right?
> > 
> > > what are you trying to achieve?
> > This is primarily to help troubleshoot containers (Docker, and now 
> > systemd). A 
> > lot of the time we want to determine what's going on in a given container 
> > (opening files, connecting to systems, etc...). There's not really a great 
> > way 
> > to restrict to containers except by manually walking datastructures to 
> > check for 
> > the right cgroup. This seems like a better alternative.
> 
> so it's about restricting or determining?
> In other words if it's analytics/tracing that's one thing, but
> enforcement/restriction is quite different.
> For analytics one can walk task_css_set(current)->dfl_cgrp and remember
> that pointer in a map or something for stats collections and similar.
> If it's restricting apps in containers then kprobe approach
> is not usable. I don't think you'd want to built an enforcement system
> on an unstable api then can vary kernel-to-kernel.
> 
The first real-world use case are to implement something like Sysdig. Often the 
team running the team running the containers don't always know what's inside of 
them, so they want to be able to view network, I/O, and other activity by 
container. Right now, the lowest common denominator between all of the 
containerization techniques is cgroups. We've seen examples of where a admin is 
unsure of the workload, and would love to use opensnoop, but there are too many 
workloads on the machine.

Unfortunately, I don't think that it's possible just to check 
task_css_set(current)->dfl_cgrp in a bpf program. The container, especially 
containers with sidecars (what Kubernetes calls Pods, I believe?) tend to have 
multiple nested cgroups inside of them. If you had a way to convert cgroup 
array 
entries to pointers, I imagine you could write an unrolled loop to check for 
ownership within a limited range.

I'm still looking for comments from the LSM folks on Checmate[1]. It appears 
that there has been very little churn in the LSM hooks API that's API-breaking. 
For many of syscall hooks, they're closely tied to the syscall API, so they 
can't really change too much. I think that a toolkit like iovisor, or another 
userland translation layer, these hooks could be very powerful. I would love to 
hear feedback from the LSM folks.

My plan with those patches is to reimplement Yama, and Hardchroot in BPF 
programs to show off the potential capabilities of Checmate. I'd also like to 
create some example programs blocking CVEs that have popped up. I think of the 
idea like nftables for kernel syscalls, storage, and the network stack.

The other example I want to show is implementing Docker-bridge style network 
isolation with Checmate. Most folks use it to map ports, and to restrict 
binding 
to specific ports, and not the dedicated network namespace, or loopback 
interface. It turns out for some applications this comes at a pretty 
significant 
hit[2][3], as well as awkward upper bounds based on conntrack.

> > > This looks like an alternative to lsm patches submitted earlier?
> > No. But I would like to use this helper in the LSM patches I'm working on. 
> > For 
> > now, with those patches, and this helper, I can create a map sized 1, and 
> > add 
> > the cgroup I care about to it. Given I can add as many bpf programs to an 
> > LSM
> > hook I want, I can use this mechanism to "attach BPF programs to cgroups" 
> > -- 
> > I put that in quotes because you're not really attaching it to a cgroup,
> > but just burning some instructions on checking it. 
> 
> how many cgroups will you need to check? The current bpf_skb_in_cgroup()
> suffers similar scaling issues.
> I think the proper restriction/enforcement could be done via attaching bpf
> program to a cgroup. These patches are being worked on Daniel Mack cc-ed.
> Then bpf program will be able to enforce networking behavior of applications
> in cgroups.
> For global container analytics I think we need something that converts
> current to cgroup_id or cgroup_handle. I don't think descendant check
> can scale for such use case.
> 
Usually there's a top level cgroup for a container, and then cgroup for each 
subprocess, and maybe a third level if that fans out to multiple workers (See: 

Re: [PATCH v4 1/1] rps: Inspect PPTP encapsulated by GRE to get flow hash

[Feng Gao]

Hi Philp,

Forgive my poor English, I am not clear about the comment #1.
"Can you make gre_base_hdr be anonymous?".

+struct gre_full_hdr {
+   struct gre_base_hdr fixed_header;

Do you mean make the member "fixed_header" as anonymous or not?

Best Regards
Feng


On Mon, Aug 8, 2016 at 5:03 AM, Philp Prindeville
 wrote:
> Inline...
>
>
>
> On 08/04/2016 01:06 AM, f...@48lvckh6395k16k5.yundunddos.com wrote:
>>
>> From: Gao Feng 
>>
>> The PPTP is encapsulated by GRE header with that GRE_VERSION bits
>> must contain one. But current GRE RPS needs the GRE_VERSION must be
>> zero. So RPS does not work for PPTP traffic.
>>
>> In my test environment, there are four MIPS cores, and all traffic
>> are passed through by PPTP. As a result, only one core is 100% busy
>> while other three cores are very idle. After this patch, the usage
>> of four cores are balanced well.
>>
>> Signed-off-by: Gao Feng 
>> ---
>>   v4: 1) Define struct gre_full_hdr, and use sizeof its member directly;
>>   2) Move version and routing check ahead;
>>   3) Only PPTP in GRE check the ack flag;
>>   v3: 1) Move struct pptp_gre_header defination into new file pptp.h
>>   2) Use sizeof GRE and PPTP type instead of literal value;
>>   3) Remove strict flag check for PPTP to robust;
>>   4) Consolidate the codes again;
>>   v2: Update according to Tom and Philp's advice.
>>   1) Consolidate the codes with GRE version 0 path;
>>   2) Use PPP_PROTOCOL to get ppp protol;
>>   3) Set the FLOW_DIS_ENCAPSULATION flag;
>>   v1: Intial Patch
>>
>>   drivers/net/ppp/pptp.c |  36 +
>>   include/net/gre.h  |  10 +++-
>>   include/net/pptp.h |  40 +++
>>   include/uapi/linux/if_tunnel.h |   7 ++-
>>   net/core/flow_dissector.c  | 113
>> -
>>   5 files changed, 135 insertions(+), 71 deletions(-)
>>   create mode 100644 include/net/pptp.h
>>
>> diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c
>> index ae0905e..3e68dbc 100644
>> --- a/drivers/net/ppp/pptp.c
>> +++ b/drivers/net/ppp/pptp.c
>> @@ -37,6 +37,7 @@
>>   #include 
>>   #include 
>>   #include 
>> +#include 
>> #include 
>>   @@ -53,41 +54,6 @@ static struct proto pptp_sk_proto __read_mostly;
>>   static const struct ppp_channel_ops pptp_chan_ops;
>>   static const struct proto_ops pptp_ops;
>>   -#define PPP_LCP_ECHOREQ 0x09
>> -#define PPP_LCP_ECHOREP 0x0A
>> -#define SC_RCV_BITS(SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP)
>> -
>> -#define MISSING_WINDOW 20
>> -#define WRAPPED(curseq, lastseq)\
>> -   curseq) & 0xff00) == 0) &&\
>> -   (((lastseq) & 0xff00) == 0xff00))
>> -
>> -#define PPTP_GRE_PROTO  0x880B
>> -#define PPTP_GRE_VER0x1
>> -
>> -#define PPTP_GRE_FLAG_C0x80
>> -#define PPTP_GRE_FLAG_R0x40
>> -#define PPTP_GRE_FLAG_K0x20
>> -#define PPTP_GRE_FLAG_S0x10
>> -#define PPTP_GRE_FLAG_A0x80
>> -
>> -#define PPTP_GRE_IS_C(f) ((f)_GRE_FLAG_C)
>> -#define PPTP_GRE_IS_R(f) ((f)_GRE_FLAG_R)
>> -#define PPTP_GRE_IS_K(f) ((f)_GRE_FLAG_K)
>> -#define PPTP_GRE_IS_S(f) ((f)_GRE_FLAG_S)
>> -#define PPTP_GRE_IS_A(f) ((f)_GRE_FLAG_A)
>> -
>> -#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header))
>> -struct pptp_gre_header {
>> -   u8  flags;
>> -   u8  ver;
>> -   __be16 protocol;
>> -   __be16 payload_len;
>> -   __be16 call_id;
>> -   __be32 seq;
>> -   __be32 ack;
>> -} __packed;
>> -
>>   static struct pppox_sock *lookup_chan(u16 call_id, __be32 s_addr)
>>   {
>> struct pppox_sock *sock;
>> diff --git a/include/net/gre.h b/include/net/gre.h
>> index 7a54a31..c469dcc 100644
>> --- a/include/net/gre.h
>> +++ b/include/net/gre.h
>> @@ -7,9 +7,17 @@
>>   struct gre_base_hdr {
>> __be16 flags;
>> __be16 protocol;
>> -};
>> +} __packed;
>>   #define GRE_HEADER_SECTION 4
>>   +struct gre_full_hdr {
>> +   struct gre_base_hdr fixed_header;
>
>
> Can you make gre_base_hdr be anonymous?
>
>
>
>> +   __be16 csum;
>> +   __be16 reserved1;
>> +   __be32 key;
>> +   __be32 seq;
>> +} __packed;
>> +
>>   #define GREPROTO_CISCO0
>>   #define GREPROTO_PPTP 1
>>   #define GREPROTO_MAX  2
>> diff --git a/include/net/pptp.h b/include/net/pptp.h
>> new file mode 100644
>> index 000..301d3e2
>> --- /dev/null
>> +++ b/include/net/pptp.h
>> @@ -0,0 +1,40 @@
>> +#ifndef _NET_PPTP_H
>> +#define _NET_PPTP_H
>> +
>> +#define PPP_LCP_ECHOREQ 0x09
>> +#define PPP_LCP_ECHOREP 0x0A
>> +#define SC_RCV_BITS (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP)
>> +
>> +#define MISSING_WINDOW 20
>> +#define WRAPPED(curseq, lastseq)\
>> +   curseq) & 0xff00) == 0) &&\
>> +   (((lastseq) & 0xff00) == 0xff00))
>> +
>> +#define PPTP_GRE_PROTO  0x880B
>> +#define PPTP_GRE_VER0x1
>> +
>> +#define 

Re: order-0 vs order-N driver allocation. Was: [PATCH v10 07/12] net/mlx4_en: add page recycle to prepare rx ring for tx support

[Alexei Starovoitov]

On Fri, Aug 05, 2016 at 09:15:33AM +0200, Eric Dumazet wrote:
> On Thu, 2016-08-04 at 18:19 +0200, Jesper Dangaard Brouer wrote:
> 
> > I actually agree, that we should switch to order-0 allocations.
> > 
> > *BUT* this will cause performance regressions on platforms with
> > expensive DMA operations (as they no longer amortize the cost of
> > mapping a larger page).
> 
> 
> We much prefer reliable behavior, even it it is ~1 % slower than the
> super-optimized thing that opens highways for attackers.

+1
It's more important to have deterministic performance at fresh boot
and after long uptime when high order-N are gone.

> Anyway, in most cases pages are re-used, so we only call
> dma_sync_single_range_for_cpu(), and there is no way to avoid this.
> 
> Using order-0 pages [1] is actually faster, since when we use high-order
> pages (multiple frames per 'page') we can not reuse the pages.
> 
> [1] I had a local patch to allocate these pages using a very simple
> allocator allocating max order (order-10) pages and splitting them into
> order-0 ages, in order to lower TLB footprint. But I could not measure a
> gain doing so on x86, at least on my lab machines.

Which driver was that?
I suspect that should indeed be the case for any driver that
uses build_skb and <256 copybreak.

Saeed,
could you please share the performance numbers for mlx5 order-0 vs order-N ?
You mentioned that there was some performance improvement. We need to know
how much we'll lose when we turn off order-N.
Thanks!

Re: [net-next 0/2] BPF, kprobes: Add current_in_cgroup helper

[Alexei Starovoitov]

On Sat, Aug 06, 2016 at 09:56:06PM -0700, Sargun Dhillon wrote:
> On Sat, Aug 06, 2016 at 09:32:05PM -0700, Alexei Starovoitov wrote:
> > On Sat, Aug 06, 2016 at 09:06:53PM -0700, Sargun Dhillon wrote:
> > > This patchset includes a helper and an example to determine whether the 
> > > kprobe 
> > > is currently executing in the context of a specific cgroup based on a 
> > > cgroup
> > > bpf map / array. 
> > 
> > description is too short to understand how this new helper is going to be 
> > used.
> > depending on kprobe current is not always valid.
> Anything not in in_interrupt() should have a current, right?
> 
> > what are you trying to achieve?
> This is primarily to help troubleshoot containers (Docker, and now systemd). 
> A 
> lot of the time we want to determine what's going on in a given container 
> (opening files, connecting to systems, etc...). There's not really a great 
> way 
> to restrict to containers except by manually walking datastructures to check 
> for 
> the right cgroup. This seems like a better alternative.

so it's about restricting or determining?
In other words if it's analytics/tracing that's one thing, but
enforcement/restriction is quite different.
For analytics one can walk task_css_set(current)->dfl_cgrp and remember
that pointer in a map or something for stats collections and similar.
If it's restricting apps in containers then kprobe approach
is not usable. I don't think you'd want to built an enforcement system
on an unstable api then can vary kernel-to-kernel.

> > This looks like an alternative to lsm patches submitted earlier?
> No. But I would like to use this helper in the LSM patches I'm working on. 
> For 
> now, with those patches, and this helper, I can create a map sized 1, and add 
> the cgroup I care about to it. Given I can add as many bpf programs to an LSM
> hook I want, I can use this mechanism to "attach BPF programs to cgroups" -- 
> I put that in quotes because you're not really attaching it to a cgroup,
> but just burning some instructions on checking it. 

how many cgroups will you need to check? The current bpf_skb_in_cgroup()
suffers similar scaling issues.
I think the proper restriction/enforcement could be done via attaching bpf
program to a cgroup. These patches are being worked on Daniel Mack cc-ed.
Then bpf program will be able to enforce networking behavior of applications
in cgroups.
For global container analytics I think we need something that converts
current to cgroup_id or cgroup_handle. I don't think descendant check
can scale for such use case.

Re: [PATCH v4 1/1] rps: Inspect PPTP encapsulated by GRE to get flow hash

[Philp Prindeville]

Inline...


On 08/04/2016 01:06 AM, f...@48lvckh6395k16k5.yundunddos.com wrote:

From: Gao Feng 

The PPTP is encapsulated by GRE header with that GRE_VERSION bits
must contain one. But current GRE RPS needs the GRE_VERSION must be
zero. So RPS does not work for PPTP traffic.

In my test environment, there are four MIPS cores, and all traffic
are passed through by PPTP. As a result, only one core is 100% busy
while other three cores are very idle. After this patch, the usage
of four cores are balanced well.

Signed-off-by: Gao Feng 
---
  v4: 1) Define struct gre_full_hdr, and use sizeof its member directly;
  2) Move version and routing check ahead;
  3) Only PPTP in GRE check the ack flag;
  v3: 1) Move struct pptp_gre_header defination into new file pptp.h
  2) Use sizeof GRE and PPTP type instead of literal value;
  3) Remove strict flag check for PPTP to robust;
  4) Consolidate the codes again;
  v2: Update according to Tom and Philp's advice.
  1) Consolidate the codes with GRE version 0 path;
  2) Use PPP_PROTOCOL to get ppp protol;
  3) Set the FLOW_DIS_ENCAPSULATION flag;
  v1: Intial Patch

  drivers/net/ppp/pptp.c |  36 +
  include/net/gre.h  |  10 +++-
  include/net/pptp.h |  40 +++
  include/uapi/linux/if_tunnel.h |   7 ++-
  net/core/flow_dissector.c  | 113 -
  5 files changed, 135 insertions(+), 71 deletions(-)
  create mode 100644 include/net/pptp.h

diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c
index ae0905e..3e68dbc 100644
--- a/drivers/net/ppp/pptp.c
+++ b/drivers/net/ppp/pptp.c
@@ -37,6 +37,7 @@
  #include 
  #include 
  #include 
+#include 
  
  #include 
  
@@ -53,41 +54,6 @@ static struct proto pptp_sk_proto __read_mostly;

  static const struct ppp_channel_ops pptp_chan_ops;
  static const struct proto_ops pptp_ops;
  
-#define PPP_LCP_ECHOREQ 0x09

-#define PPP_LCP_ECHOREP 0x0A
-#define SC_RCV_BITS(SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP)
-
-#define MISSING_WINDOW 20
-#define WRAPPED(curseq, lastseq)\
-   curseq) & 0xff00) == 0) &&\
-   (((lastseq) & 0xff00) == 0xff00))
-
-#define PPTP_GRE_PROTO  0x880B
-#define PPTP_GRE_VER0x1
-
-#define PPTP_GRE_FLAG_C0x80
-#define PPTP_GRE_FLAG_R0x40
-#define PPTP_GRE_FLAG_K0x20
-#define PPTP_GRE_FLAG_S0x10
-#define PPTP_GRE_FLAG_A0x80
-
-#define PPTP_GRE_IS_C(f) ((f)_GRE_FLAG_C)
-#define PPTP_GRE_IS_R(f) ((f)_GRE_FLAG_R)
-#define PPTP_GRE_IS_K(f) ((f)_GRE_FLAG_K)
-#define PPTP_GRE_IS_S(f) ((f)_GRE_FLAG_S)
-#define PPTP_GRE_IS_A(f) ((f)_GRE_FLAG_A)
-
-#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header))
-struct pptp_gre_header {
-   u8  flags;
-   u8  ver;
-   __be16 protocol;
-   __be16 payload_len;
-   __be16 call_id;
-   __be32 seq;
-   __be32 ack;
-} __packed;
-
  static struct pppox_sock *lookup_chan(u16 call_id, __be32 s_addr)
  {
struct pppox_sock *sock;
diff --git a/include/net/gre.h b/include/net/gre.h
index 7a54a31..c469dcc 100644
--- a/include/net/gre.h
+++ b/include/net/gre.h
@@ -7,9 +7,17 @@
  struct gre_base_hdr {
__be16 flags;
__be16 protocol;
-};
+} __packed;
  #define GRE_HEADER_SECTION 4
  
+struct gre_full_hdr {

+   struct gre_base_hdr fixed_header;


Can you make gre_base_hdr be anonymous?



+   __be16 csum;
+   __be16 reserved1;
+   __be32 key;
+   __be32 seq;
+} __packed;
+
  #define GREPROTO_CISCO0
  #define GREPROTO_PPTP 1
  #define GREPROTO_MAX  2
diff --git a/include/net/pptp.h b/include/net/pptp.h
new file mode 100644
index 000..301d3e2
--- /dev/null
+++ b/include/net/pptp.h
@@ -0,0 +1,40 @@
+#ifndef _NET_PPTP_H
+#define _NET_PPTP_H
+
+#define PPP_LCP_ECHOREQ 0x09
+#define PPP_LCP_ECHOREP 0x0A
+#define SC_RCV_BITS (SC_RCV_B7_1|SC_RCV_B7_0|SC_RCV_ODDP|SC_RCV_EVNP)
+
+#define MISSING_WINDOW 20
+#define WRAPPED(curseq, lastseq)\
+   curseq) & 0xff00) == 0) &&\
+   (((lastseq) & 0xff00) == 0xff00))
+
+#define PPTP_GRE_PROTO  0x880B
+#define PPTP_GRE_VER0x1
+
+#define PPTP_GRE_FLAG_C 0x80
+#define PPTP_GRE_FLAG_R 0x40
+#define PPTP_GRE_FLAG_K 0x20
+#define PPTP_GRE_FLAG_S 0x10
+#define PPTP_GRE_FLAG_A 0x80
+
+#define PPTP_GRE_IS_C(f) ((f)_GRE_FLAG_C)
+#define PPTP_GRE_IS_R(f) ((f)_GRE_FLAG_R)
+#define PPTP_GRE_IS_K(f) ((f)_GRE_FLAG_K)
+#define PPTP_GRE_IS_S(f) ((f)_GRE_FLAG_S)
+#define PPTP_GRE_IS_A(f) ((f)_GRE_FLAG_A)
+
+#define PPTP_HEADER_OVERHEAD (2+sizeof(struct pptp_gre_header))
+struct pptp_gre_header {
+   u8  flags;
+   u8  ver;
+   __be16 protocol;
+   __be16 payload_len;
+   __be16 call_id;
+   __be32 seq;
+   __be32 ack;
+} __packed;
+
+
+#endif
diff --git a/include/uapi/linux/if_tunnel.h b/include/uapi/linux/if_tunnel.h
index 1046f55..7d889db 100644
--- 

[PATCH iproute2] bridge: vlan json: skip ports with empty vlans

[Roopa Prabhu]

From: Roopa Prabhu 

The non-json output prints 'None' for such vlans.
And this can garble json output.

Fixes: d82a49ce85f0 ("bridge: add json support for bridge vlan show")
Signed-off-by: Roopa Prabhu 
---
saw this when deploying a mix of vlan filtering and non-vlan
filtering bridges.

 bridge/vlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bridge/vlan.c b/bridge/vlan.c
index 22f32a5..d3505b5 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -212,7 +212,7 @@ static int print_vlan(const struct sockaddr_nl *who,
 
/* if AF_SPEC isn't there, vlan table is not preset for this port */
if (!tb[IFLA_AF_SPEC]) {
-   if (!filter_vlan)
+   if (!filter_vlan && !jw_global)
fprintf(fp, "%s\tNone\n",
ll_index_to_name(ifm->ifi_index));
return 0;
-- 
2.1.4

Re: [Bridge] [PATCH net] bridge: Fix problems around fdb entries pointing to the bridge device

[Toshiaki Makita]

On 16/08/06 (土) 7:26, Stephen Hemminger wrote:

On Thu, 04 Aug 2016 11:27:25 -0700
Roopa Prabhu  wrote:


On 8/4/16, 1:15 AM, Toshiaki Makita wrote:

On 2016/08/04 16:24, Roopa Prabhu wrote:

On 8/3/16, 7:11 PM, Toshiaki Makita wrote:

Adding fdb entries pointing to the bridge device uses fdb_insert(),
which lacks various checks and does not respect added_by_user flag.

As a result, some inconsistent behavior can happen:
* Adding temporary entries succeeds but results in permanent entries.

IIRC this is not specific to fdb entries on the bridge device. all temp
fdb entries via iproute2 result in permanent entries. thats why 'dynamic'
was added.

Sorry for confusing you, I meant "temp" (i.e., "static") of bridge fdb
command.
"temp", "dynamic" and "use" should not result in "permanent".


* Same goes for "dynamic" and "use".

This patch seems to not allow dynamic and use entries
on the bridge device. I don't see a strong use-case to
allow them, but any reason you want to add the restriction now ?

Because dynamic fdb entries pointing the bridge device cannot be
created. So it is prohibited. I cannot find other appropriate behavior
about this.
Or are you suggesting local entries with aging supported or something
like that?

no, i am ok with prohibiting it, just was wondering if this is necessary.




* Changing mac address of the bridge device causes deletion of
  user-added entries.

unless I am missing something, this does not seem to be related to the
external fdb entry on the bridge device.

Yes this is related to manually-added fdb entries on the bridge device.
When manual addition of fdb pointing the bridge device was introduced,
we should have set added_by_user on adding the entry and modify
br_fdb_change_mac_address() to respect the flag, but both were not done.


* Replacing existing entries looks successful from userspace but actually
  not, regardless of NLM_F_EXCL flag.

curious about this one. I will try it, but if you have an example that
will help.

Before:
# bridge fdb add 12:34:56:78:90:ab dev enp3s0 master
# bridge fdb add 12:34:56:78:90:ab dev br0; echo $?
0
# bridge fdb show
...
12:34:56:78:90:ab dev enp3s0 master br0 permanent

# bridge fdb replace 12:34:56:78:90:ab dev br0; echo $?
0
# bridge fdb show
...
12:34:56:78:90:ab dev enp3s0 master br0 permanent

After:
# bridge fdb add 12:34:56:78:90:ab dev enp3s0 master
# bridge fdb add 12:34:56:78:90:ab dev br0; echo $?
RTNETLINK answers: File exists
255

# bridge fdb replace 12:34:56:78:90:ab dev br0; echo $?
0
# bridge fdb show
...
12:34:56:78:90:ab dev br0 master br0 permanent


ok, thanks for the example.

Acked-by: Roopa Prabhu 


It should be possible to manually add fdb entries with any combination
of valid flags. That is it should be possible to add temporary as well as 
permanent
entries. There are people that use this facility to do long and short ageing 
temporary
entries.


I see. But allowing dynamic entries means that bridge effectively 
supports local entries with aging supported, and bridge has not have 
such a feature so far: Bridge currently accepts addition of dynamic 
entries, but any addition of fdb entries pointing to the bridge device 
results in permanent entries. Bridge is just pretending support of 
dynamic local entries for now.
What I'm fixing here is just a user API problem, so I'm not thinking we 
should allow dynamic entries here. I guess we can remove the restriction 
in -next in the future.


Thanks,
Toshiaki Makita

[PATCH] tc/m_gact: Fix action_a2n() return code check

[Phil Sutter]

The function returns zero on success.

Reported-by: Mark Bloch 
Fixes: 69f5aff63c770b ("tc: use action_a2n() everywhere")
Signed-off-by: Phil Sutter 
---
 tc/m_gact.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tc/m_gact.c b/tc/m_gact.c
index c0a938c716b6a..2bfd9a7c317e4 100644
--- a/tc/m_gact.c
+++ b/tc/m_gact.c
@@ -73,7 +73,7 @@ get_act(char ***argv_p)
 {
int n;
 
-   if (!action_a2n(**argv_p, , false)) {
+   if (action_a2n(**argv_p, , false)) {
fprintf(stderr, "bad action type %s\n", **argv_p);
return -10;
}
-- 
2.9.0

[PATCH iproute2] ip route: restore_handler should check tb[RTA_PREFSRC] for local networks

[Xin Long]

Prior to this patch, If one route entry's RTA_PREFSRC and RTA_GATEWAY
both were NULL, it was supposed to be restored ONLY as a local address.

But as it didn't check tb[RTA_PREFSRC] when restoring local networks,
rtattr_cmp would return a success if it was NULL, this route entry would
be restored again as a local network.

This patch is to add tb[RTA_PREFSRC] check when restoring local networks.

Fixes: 74af8dd9620e ("ip route: restore route entries in correct order")
Signed-off-by: Xin Long 
---
 ip/iproute.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ip/iproute.c b/ip/iproute.c
index c52294d..3da23af 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -1813,7 +1813,7 @@ static int restore_handler(const struct sockaddr_nl *nl,
if (!prio && !tb[RTA_GATEWAY] && (!tb[RTA_PREFSRC] ||
!rtattr_cmp(tb[RTA_PREFSRC], tb[RTA_DST])))
goto restore;
-   else if (prio == 1 && !tb[RTA_GATEWAY] &&
+   else if (prio == 1 && !tb[RTA_GATEWAY] && tb[RTA_PREFSRC] &&
 rtattr_cmp(tb[RTA_PREFSRC], tb[RTA_DST]))
goto restore;
else if (prio == 2 && tb[RTA_GATEWAY])
-- 
2.1.0

Пробив любых данных т. +79776066203 Email: nah...@163.com

[nah...@163.com]

Пробив любых данных т. +79776066203 Email: nah...@163.com

flow_dissector: Get vlan priority in addition to vlan id

[Hadar Hen Zion]

Hi Tom and Jiri,

I would like to add vlan priority to  __skb_flow_dissect.

In the current vlan tag implementation there isn't any room left to
vlan priority next to the vlan id.

struct flow_dissector_key_tags {
u32 vlan_id:12,
flow_label:20;
};

According to the discussion between you two [1], I'll be happy to get
your advice about what is the best way of adding vlan priority?

My suggestion is to add new vlan tag struct, it will make the code
cleaner and since we have to add 3 bits to vlan priority any way it
won't add unnecessary physical address holes.

struct flow_dissector_key_tags {
u32 flow_label:20;
};

struct flow_dissector_key_vlan {
u16 vlan_id:12,
vlan_priority:3;
};

Thanks,
Hadar

[1] - http://marc.info/?l=linux-netdev=143232557025994=2

Re: [PATCH] net: macb: Correct CAPS mask

[David Miller]

From: Harini Katakam 
Date: Fri, 5 Aug 2016 10:31:58 +0530

> USRIO and JUMBO CAPS have the same mask.
> Fix the same.
> 
> Fixes: ce721a702197 ("net: ethernet: cadence-macb: Add disabled usrio caps")
> Cc: sta...@vger.kernel.org # v4.5+
> Signed-off-by: Harini Katakam 
> Acked-by: Nicolas Ferre 

Applied.

Re: pull-request: mac80211 2016-08-05

[David Miller]

From: Johannes Berg 
Date: Fri,  5 Aug 2016 14:32:08 +0200

> Here's a first set of fixes for the current cycle. See the tag message
> for more information.
> 
> I'll probably have a follow-up fix for the real problem in mac80211
> that caused the crash later, but for now we have this patch and it
> makes sense and fixes the crash, even if the behaviour isn't quite
> right (afaict.)
> 
> Let me know if there's any problem.

Pulled, thanks.

[PATCH net] sctp: use event->chunk when it's valid

[Xin Long]

Commit 52253db924d1 ("sctp: also point GSO head_skb to the sk when
it's available") used event->chunk->head_skb to get the head_skb in
sctp_ulpevent_set_owner().

But at that moment, the event->chunk was NULL, as it cloned the skb
in sctp_ulpevent_make_rcvmsg(). Therefore, that patch didn't really
work.

This patch is to move the event->chunk initialization before calling
sctp_ulpevent_receive_data() so that it uses event->chunk when it's
valid.

Fixes: 52253db924d1 ("sctp: also point GSO head_skb to the sk when it's 
available")
Signed-off-by: Xin Long 
---
 net/sctp/ulpevent.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sctp/ulpevent.c b/net/sctp/ulpevent.c
index 1bc4f71..d85b803 100644
--- a/net/sctp/ulpevent.c
+++ b/net/sctp/ulpevent.c
@@ -702,14 +702,14 @@ struct sctp_ulpevent *sctp_ulpevent_make_rcvmsg(struct 
sctp_association *asoc,
 */
sctp_ulpevent_init(event, 0, skb->len + sizeof(struct sk_buff));
 
-   sctp_ulpevent_receive_data(event, asoc);
-
/* And hold the chunk as we need it for getting the IP headers
 * later in recvmsg
 */
sctp_chunk_hold(chunk);
event->chunk = chunk;
 
+   sctp_ulpevent_receive_data(event, asoc);
+
event->stream = ntohs(chunk->subh.data_hdr->stream);
event->ssn = ntohs(chunk->subh.data_hdr->ssn);
event->ppid = chunk->subh.data_hdr->ppid;
-- 
2.1.0