* Re: [PATCH] mlx4_en: fix allocation of device tx_cq
From: Amir Vadai @ 2013-02-23 7:05 UTC (permalink / raw)
To: Kleber Sacilotto de Souza; +Cc: davem, netdev
In-Reply-To: <1361581082-5583-1-git-send-email-klebers@linux.vnet.ibm.com>
On 23/02/2013 02:58, Kleber Sacilotto de Souza wrote:
> The memory to hold the network device tx_cq is not being allocated with
> the correct size in mlx4_en_init_netdev(). It should use MAX_TX_RINGS
> instead of MAX_RX_RINGS. This can cause problems if the number of tx
> rings being used is greater than MAX_RX_RINGS.
>
> Signed-off-by: Kleber Sacilotto de Souza <klebers@linux.vnet.ibm.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 5088dc5..5944dd6 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -2067,7 +2067,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
> err = -ENOMEM;
> goto out;
> }
> - priv->tx_cq = kzalloc(sizeof(struct mlx4_en_cq) * MAX_RX_RINGS,
> + priv->tx_cq = kzalloc(sizeof(struct mlx4_en_cq) * MAX_TX_RINGS,
> GFP_KERNEL);
> if (!priv->tx_cq) {
> err = -ENOMEM;
>
(sorry for double posting - tried to send from my tablet in plain text
without success).
Acked-by: Amir Vadai <amirv@mellanox.com>
^ permalink raw reply
* Re: 3.7.8/amd64 full interrupt hangs due to iwlwifi under big nfs copies out
From: Marc MERLIN @ 2013-02-23 6:14 UTC (permalink / raw)
To: Johannes Berg
Cc: Eric Dumazet, David Miller, Larry.Finger, bhutchings,
linux-wireless, netdev
In-Reply-To: <1361351703.8629.5.camel@jlt4.sipsolutions.net>
On Wed, Feb 20, 2013 at 10:15:03AM +0100, Johannes Berg wrote:
> On Wed, 2013-02-20 at 10:12 +0100, Johannes Berg wrote:
> > On Tue, 2013-02-19 at 08:21 -0800, Eric Dumazet wrote:
> > > On Tue, 2013-02-19 at 11:03 +0100, Johannes Berg wrote:
> > > > On Mon, 2013-02-18 at 21:17 -0800, Eric Dumazet wrote:
> > > >
> > > > > > chrome: page allocation failure: order:1, mode:0x4020
> > > > > > Pid: 8730, comm: chrome Tainted: G O 3.7.8-amd64-preempt-20121226-fixwd #1
> > > > > > Call Trace:
> > > > > > <IRQ> [<ffffffff810d5f38>] warn_alloc_failed+0x117/0x12c
> > > >
> > > > > You could try to load iwlwifi with amsdu_size_8K set to 0 (disable)
> > > > >
> > > > > It should hopefully use order-0 pages
> > > >
> > > > It will, do that then, unfortunately it can't switch at runtime because
> > > > it advertised this support to the access point or clients.
> > >
> > > What are the drawbacks of setting amsdu_size_8K to 0 by default ?
> >
> > We're discussing this now, the only downside would be that we couldn't
> > receive 8k A-MSDUs. Thing is, practically nobody uses A-MSDU anyway, and
> > even when I suspect the difference between 4k and 8k won't be huge.
>
> OTOH, this affects the protocol, and when you really can't allocate any
> order-1 pages you pointed out yourself that many other things also won't
> work, so I'm not really sure it makes a big difference if we change the
> driver?
That as an unscientific test, but when I did the NFS eats all my pages
test using ethernet, my system didn't hang like it did with iwlagn.
So while the NFS code is definitely doing something wrong when it uses
its default huge buffers, the e1000e code deals with it without hanging
my system.
So thanks for trying to improve the iwlagn code to avoid those system
lockups.
Marc
--
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems ....
.... what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/
^ permalink raw reply
* [PATCH] mlx4_en: fix allocation of CPU affinity reverse-map
From: Kleber Sacilotto de Souza @ 2013-02-23 5:14 UTC (permalink / raw)
To: davem; +Cc: Amir Vadai, netdev, Kleber Sacilotto de Souza
The mlx4_en driver allocates the number of objects for the CPU affinity
reverse-map based on the number of rx rings of the device. However,
mlx4_assign_eq() calls irq_cpu_rmap_add() as many times as IRQ's are
assigned to EQ's, which can be as large as mlx4_dev->caps.comp_pool. If
caps.comp_pool is larger than rx_ring_num we will eventually hit the
BUG_ON() in cpu_rmap_add().
Fix this problem by allocating space for the maximum number of CPU
affinity reverse-map objects we might want to add.
Signed-off-by: Kleber Sacilotto de Souza <klebers@linux.vnet.ibm.com>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 5944dd6..5385474 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1829,7 +1829,7 @@ int mlx4_en_alloc_resources(struct mlx4_en_priv *priv)
}
#ifdef CONFIG_RFS_ACCEL
- priv->dev->rx_cpu_rmap = alloc_irq_cpu_rmap(priv->rx_ring_num);
+ priv->dev->rx_cpu_rmap = alloc_irq_cpu_rmap(priv->mdev->dev->caps.comp_pool);
if (!priv->dev->rx_cpu_rmap)
goto err;
#endif
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 1/1] net: fec: fix miss init spinlock
From: Fabio Estevam @ 2013-02-23 2:08 UTC (permalink / raw)
To: Frank Li; +Cc: Jim Baxter, netdev
In-Reply-To: <CAHrpEqRhCyM8z8th0ktYErUYPtLXVayEJm0BZSb6WWm=7dgN5w@mail.gmail.com>
Frank,
On Fri, Feb 22, 2013 at 9:46 PM, Frank Li <lznuaa@gmail.com> wrote:
> Does it happen when run ptp4?
> I am working on put xmit to NAPI and remove lock.
Jim reported the way to reproduce this issue in a previous post in this thread:
"Thank you, I have tried your new patch, it fixes the boot deadlock but when I
run the ptp4l command I get another dead lock:
On client running "./ptp4l -S -i eth0 -s -m"
On the i.MX6:
root@mx6q35:~# ptp4l -H -m -i eth0 -p /dev/ptp0
ptp4l[98.894]: selected /dev/ptp0 as PTP clock
ptp4l[98.896]: failed to read out the clock frequency adjustment: Operation not
supported
ptp4l[98.900]: driver changed our HWTSTAMP options
ptp4l[98.900]: tx_type 1 not 1
ptp4l[98.900]: rx_filter 1 not 12
ptp4l[98.901]: port 1: INITIALIZING to LISTENING on INITIALIZE
ptp4l[98.902]: port 0: INITIALIZING to LISTENING on INITIALIZE
ptp4l[104.901]: port 1: LISTENING to MASTER on ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES
[ 111.161885] =================================
[ 111.166249] [ INFO: inconsistent lock state ]
[ 111.170616] 3.5.7.3-01268-g6696bd7-dirty #2 Not tainted
[ 111.175848] ---------------------------------"
^ permalink raw reply
* Re: How to hairpin two Ethernet interfaces together (L2 hub/repeater mode)?
From: Hannes Frederic Sowa @ 2013-02-23 1:42 UTC (permalink / raw)
To: Petri Gynther; +Cc: netdev
In-Reply-To: <CAKuTDr_eC4Wrv=uGthGJk3Zps=fnyAxpqqmsSDau7ZLs4PGyZw@mail.gmail.com>
On Fri, Feb 22, 2013 at 04:10:54PM -0800, Petri Gynther wrote:
> Any trivial solutions to this? Can Linux bridge be configured in L2
> hub/repeater mode so that it simply floods every Rx frame to all other
> ports of the bridge?
You could have a look at openvswitch, it supports (R)SPAN ports.
^ permalink raw reply
* [PATCH iproute 2/2] ss: show socket memory stats for unix sockets if requested
From: Hannes Frederic Sowa @ 2013-02-23 1:28 UTC (permalink / raw)
To: stephen; +Cc: netdev
The output format is the same as for tcp sockets but only the following
fields are currently non-zero: sk_rcvbuf, sk_wmem_alloc and sk_sndbuf.
Cc: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
misc/ss.c | 47 +++++++++++++++++++++++++++++++----------------
1 file changed, 31 insertions(+), 16 deletions(-)
diff --git a/misc/ss.c b/misc/ss.c
index 0de700a..e742aad 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -1327,6 +1327,29 @@ static char *sprint_bw(char *buf, double bw)
return buf;
}
+static void print_skmeminfo(struct rtattr *tb[], int attrtype)
+{
+ const __u32 *skmeminfo;
+ if (!tb[attrtype])
+ return;
+ skmeminfo = RTA_DATA(tb[attrtype]);
+
+ printf(" skmem:(r%u,rb%u,t%u,tb%u,f%u,w%u,o%u",
+ skmeminfo[SK_MEMINFO_RMEM_ALLOC],
+ skmeminfo[SK_MEMINFO_RCVBUF],
+ skmeminfo[SK_MEMINFO_WMEM_ALLOC],
+ skmeminfo[SK_MEMINFO_SNDBUF],
+ skmeminfo[SK_MEMINFO_FWD_ALLOC],
+ skmeminfo[SK_MEMINFO_WMEM_QUEUED],
+ skmeminfo[SK_MEMINFO_OPTMEM]);
+
+ if (RTA_PAYLOAD(tb[attrtype]) >=
+ (SK_MEMINFO_BACKLOG + 1) * sizeof(__u32))
+ printf(",bl%u", skmeminfo[SK_MEMINFO_BACKLOG]);
+
+ printf(")");
+}
+
static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r)
{
struct rtattr * tb[INET_DIAG_MAX+1];
@@ -1337,22 +1360,7 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r)
nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*r)));
if (tb[INET_DIAG_SKMEMINFO]) {
- const __u32 *skmeminfo = RTA_DATA(tb[INET_DIAG_SKMEMINFO]);
-
- printf(" skmem:(r%u,rb%u,t%u,tb%u,f%u,w%u,o%u",
- skmeminfo[SK_MEMINFO_RMEM_ALLOC],
- skmeminfo[SK_MEMINFO_RCVBUF],
- skmeminfo[SK_MEMINFO_WMEM_ALLOC],
- skmeminfo[SK_MEMINFO_SNDBUF],
- skmeminfo[SK_MEMINFO_FWD_ALLOC],
- skmeminfo[SK_MEMINFO_WMEM_QUEUED],
- skmeminfo[SK_MEMINFO_OPTMEM]);
-
- if (RTA_PAYLOAD(tb[INET_DIAG_SKMEMINFO]) >=
- (SK_MEMINFO_BACKLOG + 1) * sizeof(__u32))
- printf(",bl%u", skmeminfo[SK_MEMINFO_BACKLOG]);
-
- printf(")");
+ print_skmeminfo(tb, INET_DIAG_SKMEMINFO);
} else if (tb[INET_DIAG_MEMINFO]) {
const struct inet_diag_meminfo *minfo
= RTA_DATA(tb[INET_DIAG_MEMINFO]);
@@ -2188,6 +2196,11 @@ static int unix_show_sock(struct nlmsghdr *nlh, struct filter *f)
printf(" users:(%s)", ubuf);
}
+ if (show_mem) {
+ printf("\n\t");
+ print_skmeminfo(tb, UNIX_DIAG_MEMINFO);
+ }
+
printf("\n");
return 0;
@@ -2214,6 +2227,8 @@ static int unix_show_netlink(struct filter *f, FILE *dump_fp)
req.r.sdiag_family = AF_UNIX;
req.r.udiag_states = f->states;
req.r.udiag_show = UDIAG_SHOW_NAME | UDIAG_SHOW_PEER | UDIAG_SHOW_RQLEN;
+ if (show_mem)
+ req.r.udiag_show |= UDIAG_SHOW_MEMINFO;
if (send(fd, &req, sizeof(req), 0) < 0) {
close(fd);
--
1.8.1.2
^ permalink raw reply related
* [PATCH iproute 1/2] ss: show send queue length on unix domain sockets
From: Hannes Frederic Sowa @ 2013-02-23 1:28 UTC (permalink / raw)
To: stephen; +Cc: netdev
On sockets in listen state Send-Q reports the maximum backlog,
otherwise it reports allocated socket write memory.
Cc: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
misc/ss.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/misc/ss.c b/misc/ss.c
index ff7c194..0de700a 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2139,7 +2139,7 @@ static int unix_show_sock(struct nlmsghdr *nlh, struct filter *f)
struct rtattr *tb[UNIX_DIAG_MAX+1];
char name[128];
int peer_ino;
- int rqlen;
+ __u32 rqlen, wqlen;
parse_rtattr(tb, UNIX_DIAG_MAX, (struct rtattr*)(r+1),
nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*r)));
@@ -2150,12 +2150,16 @@ static int unix_show_sock(struct nlmsghdr *nlh, struct filter *f)
if (state_width)
printf("%-*s ", state_width, sstate_name[r->udiag_state]);
- if (tb[UNIX_DIAG_RQLEN])
- rqlen = *(int *)RTA_DATA(tb[UNIX_DIAG_RQLEN]);
- else
+ if (tb[UNIX_DIAG_RQLEN]) {
+ struct unix_diag_rqlen *rql = RTA_DATA(tb[UNIX_DIAG_RQLEN]);
+ rqlen = rql->udiag_rqueue;
+ wqlen = rql->udiag_wqueue;
+ } else {
rqlen = 0;
+ wqlen = 0;
+ }
- printf("%-6d %-6d ", rqlen, 0);
+ printf("%-6u %-6u ", rqlen, wqlen);
if (tb[UNIX_DIAG_NAME]) {
int len = RTA_PAYLOAD(tb[UNIX_DIAG_NAME]);
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH v3] datapath: Increase maximum allocation size of action list.
From: Pravin Shelar @ 2013-02-23 1:08 UTC (permalink / raw)
To: netdev
In-Reply-To: <1361581410-2311-1-git-send-email-pshelar@nicira.com>
Please ignore this patch.
This patch is for openvswitch mailing list.
On Fri, Feb 22, 2013 at 5:03 PM, Pravin B Shelar <pshelar@nicira.com> wrote:
> The switch to flow based tunneling increased the size of each output
> action in the flow action list. In extreme cases, this can result
> in the action list exceeding the maximum buffer size.
>
> This doubles the maximum buffer size to compensate for the increase
> in action size. In the common case, most allocations will be
> less than a page and those uses kmalloc. Therefore, for the majority
> of situations, this will have no impact.
>
> Bug #15203
> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
> ---
> Fixed according to commnets from Jesse.
> v2-v3:
> - Use size for buf_size.
> - Fixed size check in ovs_flow_actions_free().
>
> v1-v2:
> Fixes some issues noticed in earlier patch.
> ---
> datapath/datapath.c | 8 ++++----
> datapath/flow.c | 19 +++++++++++++++++--
> datapath/flow.h | 5 ++++-
> 3 files changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index fc5d2de..bd2d57b 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -434,10 +434,10 @@ static struct nlattr *reserve_sfa_size(struct sw_flow_actions **sfa, int attr_le
> int next_offset = offsetof(struct sw_flow_actions, actions) +
> (*sfa)->actions_len;
>
> - if (req_size <= (ksize(*sfa) - next_offset))
> + if (req_size <= ((*sfa)->buf_size - next_offset))
> goto out;
>
> - new_acts_size = ksize(*sfa) * 2;
> + new_acts_size = (*sfa)->buf_size * 2;
>
> if (new_acts_size > MAX_ACTIONS_BUFSIZE) {
> if ((MAX_ACTIONS_BUFSIZE - next_offset) < req_size)
> @@ -451,7 +451,7 @@ static struct nlattr *reserve_sfa_size(struct sw_flow_actions **sfa, int attr_le
>
> memcpy(acts->actions, (*sfa)->actions, (*sfa)->actions_len);
> acts->actions_len = (*sfa)->actions_len;
> - kfree(*sfa);
> + ovs_flow_actions_free(*sfa);
> *sfa = acts;
>
> out:
> @@ -1292,7 +1292,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
> return 0;
>
> err_kfree:
> - kfree(acts);
> + ovs_flow_actions_free(acts);
> error:
> return error;
> }
> diff --git a/datapath/flow.c b/datapath/flow.c
> index b14229f..b6bb7a7 100644
> --- a/datapath/flow.c
> +++ b/datapath/flow.c
> @@ -207,14 +207,29 @@ struct sw_flow_actions *ovs_flow_actions_alloc(int size)
> if (size > MAX_ACTIONS_BUFSIZE)
> return ERR_PTR(-EINVAL);
>
> - sfa = kmalloc(sizeof(*sfa) + size, GFP_KERNEL);
> + size += sizeof(*sfa);
> + if (size <= MAX_ACTIONS_BUFSIZE_KMALLOC)
> + sfa = kmalloc(size, GFP_KERNEL);
> + else
> + sfa = vmalloc(size);
> +
> if (!sfa)
> return ERR_PTR(-ENOMEM);
>
> sfa->actions_len = 0;
> + sfa->buf_size = size;
> +
> return sfa;
> }
>
> +void ovs_flow_actions_free(struct sw_flow_actions *sfa)
> +{
> + if (sfa->buf_size <= MAX_ACTIONS_BUFSIZE_KMALLOC)
> + kfree(sfa);
> + else
> + vfree(sfa);
> +}
> +
> struct sw_flow *ovs_flow_alloc(void)
> {
> struct sw_flow *flow;
> @@ -437,7 +452,7 @@ static void rcu_free_acts_callback(struct rcu_head *rcu)
> {
> struct sw_flow_actions *sf_acts = container_of(rcu,
> struct sw_flow_actions, rcu);
> - kfree(sf_acts);
> + ovs_flow_actions_free(sf_acts);
> }
>
> /* Schedules 'sf_acts' to be freed after the next RCU grace period.
> diff --git a/datapath/flow.h b/datapath/flow.h
> index 6949640..3b08ea6 100644
> --- a/datapath/flow.h
> +++ b/datapath/flow.h
> @@ -37,6 +37,7 @@ struct sk_buff;
> struct sw_flow_actions {
> struct rcu_head rcu;
> u32 actions_len;
> + int buf_size;
> struct nlattr actions[];
> };
>
> @@ -151,6 +152,7 @@ void ovs_flow_deferred_free(struct sw_flow *);
> void ovs_flow_free(struct sw_flow *);
>
> struct sw_flow_actions *ovs_flow_actions_alloc(int actions_len);
> +void ovs_flow_actions_free(struct sw_flow_actions *sfa);
> void ovs_flow_deferred_free_acts(struct sw_flow_actions *);
>
> int ovs_flow_extract(struct sk_buff *, u16 in_port, struct sw_flow_key *,
> @@ -194,7 +196,8 @@ int ovs_flow_from_nlattrs(struct sw_flow_key *swkey, int *key_lenp,
> int ovs_flow_metadata_from_nlattrs(struct sw_flow *flow, int key_len,
> const struct nlattr *attr);
>
> -#define MAX_ACTIONS_BUFSIZE (16 * 1024)
> +#define MAX_ACTIONS_BUFSIZE (32 * 1024)
> +#define MAX_ACTIONS_BUFSIZE_KMALLOC PAGE_SIZE
> #define TBL_MIN_BUCKETS 1024
>
> struct flow_table {
> --
> 1.7.1
>
^ permalink raw reply
* [PATCH] mlx4_en: fix allocation of device tx_cq
From: Kleber Sacilotto de Souza @ 2013-02-23 0:58 UTC (permalink / raw)
To: davem; +Cc: Amir Vadai, netdev, Kleber Sacilotto de Souza
The memory to hold the network device tx_cq is not being allocated with
the correct size in mlx4_en_init_netdev(). It should use MAX_TX_RINGS
instead of MAX_RX_RINGS. This can cause problems if the number of tx
rings being used is greater than MAX_RX_RINGS.
Signed-off-by: Kleber Sacilotto de Souza <klebers@linux.vnet.ibm.com>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 5088dc5..5944dd6 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2067,7 +2067,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
err = -ENOMEM;
goto out;
}
- priv->tx_cq = kzalloc(sizeof(struct mlx4_en_cq) * MAX_RX_RINGS,
+ priv->tx_cq = kzalloc(sizeof(struct mlx4_en_cq) * MAX_TX_RINGS,
GFP_KERNEL);
if (!priv->tx_cq) {
err = -ENOMEM;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 2/2] vlan: adjust vlan_set_encap_proto() for its callers
From: Jesse Gross @ 2013-02-23 1:05 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, David S. Miller
In-Reply-To: <1361525547-21808-2-git-send-email-amwang@redhat.com>
On Fri, Feb 22, 2013 at 1:32 AM, Cong Wang <amwang@redhat.com> wrote:
> From: Cong Wang <amwang@redhat.com>
>
> There are two places to call vlan_set_encap_proto():
> vlan_untag() and __pop_vlan_tci().
>
> vlan_untag() assumes skb->data points after mac addr, otherwise
> the following code
>
> vhdr = (struct vlan_hdr *) skb->data;
> vlan_tci = ntohs(vhdr->h_vlan_TCI);
> __vlan_hwaccel_put_tag(skb, vlan_tci);
>
> skb_pull_rcsum(skb, VLAN_HLEN);
>
> won't be correct. But __pop_vlan_tci() assumes points _before_
> mac addr.
>
> In vlan_set_encap_proto(), it looks for some magic L2 value
> after mac addr:
>
> rawp = skb->data;
> if (*(unsigned short *) rawp == 0xFFFF)
> ...
>
> Therefore __pop_vlan_tci() is obviously wrong.
>
> A quick fix is avoiding using skb->data in vlan_set_encap_proto(),
> use 'vhdr+1' is always correct in both cases.
>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Jesse Gross <jesse@nicira.com>
> Signed-off-by: Cong Wang <amwang@redhat.com>
Acked-by: Jesse Gross <jesse@nicira.com>
^ permalink raw reply
* Re: [Patch] openvswitch: remove some useless comments
From: Jesse Gross @ 2013-02-23 1:04 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, David S. Miller
In-Reply-To: <1361533286-29210-1-git-send-email-amwang@redhat.com>
On Fri, Feb 22, 2013 at 3:41 AM, Cong Wang <amwang@redhat.com> wrote:
> From: Cong Wang <amwang@redhat.com>
>
> These comments are useless in upstream kernel.
>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Jesse Gross <jesse@nicira.com>
> Signed-off-by: Cong Wang <amwang@redhat.com>
Also applied.
^ permalink raw reply
* Re: [PATCH 1/2] openvswitch: fix the calculation of checksum for vlan header
From: Jesse Gross @ 2013-02-23 1:04 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, David S. Miller
In-Reply-To: <1361525547-21808-1-git-send-email-amwang@redhat.com>
On Fri, Feb 22, 2013 at 1:32 AM, Cong Wang <amwang@redhat.com> wrote:
> From: Cong Wang <amwang@redhat.com>
>
> In vlan_insert_tag(), we insert a 4-byte VLAN header _after_
> mac header:
>
> memmove(skb->data, skb->data + VLAN_HLEN, 2 * ETH_ALEN);
> ...
> veth->h_vlan_proto = htons(ETH_P_8021Q);
> ...
> veth->h_vlan_TCI = htons(vlan_tci);
>
> so after it, we should recompute the checksum to include these 4 bytes.
> skb->data still points to the mac header, therefore VLAN header is at
> (2 * ETH_ALEN = 12) bytes after it, not (ETH_HLEN = 14) bytes.
>
> This can also be observed via tcpdump:
>
> 0x0000: ffff ffff ffff 5254 005d 6f6e 8100 000a
> 0x0010: 0806 0001 0800 0604 0001 5254 005d 6f6e
> 0x0020: c0a8 026e 0000 0000 0000 c0a8 0282
>
> Similar for __pop_vlan_tci(), the vlan header we remove is the one
> overwritten in:
>
> memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
>
> Therefore the VLAN_HLEN = 4 bytes after 2 * ETH_ALEN is the part
> we want to sub from checksum.
>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Jesse Gross <jesse@nicira.com>
> Signed-off-by: Cong Wang <amwang@redhat.com>
Applied, thanks.
^ permalink raw reply
* [PATCH v3] datapath: Increase maximum allocation size of action list.
From: Pravin B Shelar @ 2013-02-23 1:03 UTC (permalink / raw)
To: netdev; +Cc: edumazet, jesse, stephen, Pravin B Shelar
The switch to flow based tunneling increased the size of each output
action in the flow action list. In extreme cases, this can result
in the action list exceeding the maximum buffer size.
This doubles the maximum buffer size to compensate for the increase
in action size. In the common case, most allocations will be
less than a page and those uses kmalloc. Therefore, for the majority
of situations, this will have no impact.
Bug #15203
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
Fixed according to commnets from Jesse.
v2-v3:
- Use size for buf_size.
- Fixed size check in ovs_flow_actions_free().
v1-v2:
Fixes some issues noticed in earlier patch.
---
datapath/datapath.c | 8 ++++----
datapath/flow.c | 19 +++++++++++++++++--
datapath/flow.h | 5 ++++-
3 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index fc5d2de..bd2d57b 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -434,10 +434,10 @@ static struct nlattr *reserve_sfa_size(struct sw_flow_actions **sfa, int attr_le
int next_offset = offsetof(struct sw_flow_actions, actions) +
(*sfa)->actions_len;
- if (req_size <= (ksize(*sfa) - next_offset))
+ if (req_size <= ((*sfa)->buf_size - next_offset))
goto out;
- new_acts_size = ksize(*sfa) * 2;
+ new_acts_size = (*sfa)->buf_size * 2;
if (new_acts_size > MAX_ACTIONS_BUFSIZE) {
if ((MAX_ACTIONS_BUFSIZE - next_offset) < req_size)
@@ -451,7 +451,7 @@ static struct nlattr *reserve_sfa_size(struct sw_flow_actions **sfa, int attr_le
memcpy(acts->actions, (*sfa)->actions, (*sfa)->actions_len);
acts->actions_len = (*sfa)->actions_len;
- kfree(*sfa);
+ ovs_flow_actions_free(*sfa);
*sfa = acts;
out:
@@ -1292,7 +1292,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
return 0;
err_kfree:
- kfree(acts);
+ ovs_flow_actions_free(acts);
error:
return error;
}
diff --git a/datapath/flow.c b/datapath/flow.c
index b14229f..b6bb7a7 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -207,14 +207,29 @@ struct sw_flow_actions *ovs_flow_actions_alloc(int size)
if (size > MAX_ACTIONS_BUFSIZE)
return ERR_PTR(-EINVAL);
- sfa = kmalloc(sizeof(*sfa) + size, GFP_KERNEL);
+ size += sizeof(*sfa);
+ if (size <= MAX_ACTIONS_BUFSIZE_KMALLOC)
+ sfa = kmalloc(size, GFP_KERNEL);
+ else
+ sfa = vmalloc(size);
+
if (!sfa)
return ERR_PTR(-ENOMEM);
sfa->actions_len = 0;
+ sfa->buf_size = size;
+
return sfa;
}
+void ovs_flow_actions_free(struct sw_flow_actions *sfa)
+{
+ if (sfa->buf_size <= MAX_ACTIONS_BUFSIZE_KMALLOC)
+ kfree(sfa);
+ else
+ vfree(sfa);
+}
+
struct sw_flow *ovs_flow_alloc(void)
{
struct sw_flow *flow;
@@ -437,7 +452,7 @@ static void rcu_free_acts_callback(struct rcu_head *rcu)
{
struct sw_flow_actions *sf_acts = container_of(rcu,
struct sw_flow_actions, rcu);
- kfree(sf_acts);
+ ovs_flow_actions_free(sf_acts);
}
/* Schedules 'sf_acts' to be freed after the next RCU grace period.
diff --git a/datapath/flow.h b/datapath/flow.h
index 6949640..3b08ea6 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -37,6 +37,7 @@ struct sk_buff;
struct sw_flow_actions {
struct rcu_head rcu;
u32 actions_len;
+ int buf_size;
struct nlattr actions[];
};
@@ -151,6 +152,7 @@ void ovs_flow_deferred_free(struct sw_flow *);
void ovs_flow_free(struct sw_flow *);
struct sw_flow_actions *ovs_flow_actions_alloc(int actions_len);
+void ovs_flow_actions_free(struct sw_flow_actions *sfa);
void ovs_flow_deferred_free_acts(struct sw_flow_actions *);
int ovs_flow_extract(struct sk_buff *, u16 in_port, struct sw_flow_key *,
@@ -194,7 +196,8 @@ int ovs_flow_from_nlattrs(struct sw_flow_key *swkey, int *key_lenp,
int ovs_flow_metadata_from_nlattrs(struct sw_flow *flow, int key_len,
const struct nlattr *attr);
-#define MAX_ACTIONS_BUFSIZE (16 * 1024)
+#define MAX_ACTIONS_BUFSIZE (32 * 1024)
+#define MAX_ACTIONS_BUFSIZE_KMALLOC PAGE_SIZE
#define TBL_MIN_BUCKETS 1024
struct flow_table {
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 1/1] net: fec: fix miss init spinlock
From: Frank Li @ 2013-02-23 0:46 UTC (permalink / raw)
To: Jim Baxter; +Cc: netdev
In-Reply-To: <loom.20130222T132206-265@post.gmane.org>
2013/2/22 Jim Baxter <jim_baxter@mentor.com>:
>> > On 2/6/2013 1:50 AM, Frank Li wrote:
>> >>
>> >>
>> >> It is not easy to fix. Need put tx to NAPI poll function.
>> >> I will work on that after China New year holiday
>
>
>
> Hi Frank,
>
> Have you had time to have a look at this issues again?
Does it happen when run ptp4?
I am working on put xmit to NAPI and remove lock.
>
> Jim
>
>
>
>
>
>
>
>
>
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: AF_VSOCK and the LSMs
From: Paul Moore @ 2013-02-23 0:45 UTC (permalink / raw)
To: Casey Schaufler
Cc: netdev, linux-security-module, selinux, Andy King, Gerd Hoffmann,
Eric Paris
In-Reply-To: <5127F874.1050907@schaufler-ca.com>
On Friday, February 22, 2013 03:00:04 PM Casey Schaufler wrote:
> Please add an LSM blob. Please do not use a secid. I am currently
> battling with secids in my efforts for multiple LSM support.
>
> ...
>
> I am going to be able to deal with secids for AF_INET only because
> SELinux prefers XFRM, Smack requires CIPSO, and AppArmor is going to
> be willing to have networking be optional.
"prefers"? Really Casey, did you think I would let you get away with that
statement? What a LSM "prefers" is really not relevant to the stacking
effort, what a LSM _supports_ is what matters.
SELinux _supports_ NetLabel (CIPSO, etc.), XFRM (labeled IPsec), and secmark.
Smack _supports_ NetLabel (CIPSO).
AppArmor and TOMOYO don't really do any of the forms of labeled networking
that are relevant for this discussion. If you are going to do stacking with
LSMs that conflict when it comes to what they _support_, not what they
_prefer_, with labeled networking then you are either going to have to either:
1. Selectively remove support from all but one of the LSMs. (ungh ...)
2. Convince netdev to give you a blob in the sk_buff. (the pigs are flying!)
3. Work some sub-system dependent magic.
If you want to try option #3 I think we might be able to do something with
NetLabel to support multiple LSMs as the label abstraction stuff should
theoretically make this possible; although the NetLabel cache will need some
work. Labeled IPsec is likely out due to the way it was designed unless you
want to attempt to negotiate two labels during the IKE exchange (yuck). I
think we can also rule out secmark as multi-LSM enabled due to the limitations
on a 32 bit integer.
If you want to talk about this further let me know - I think we've talked
about this at the past two security summits - but don't attempt to gloss over
details with this "prefers" crap.
> If you have two LSMs that use secids you are never going to have a
> rational way to get the information for both into one secid.
Exactly, I don't disagree which is why I've always said that networking was
going to be a major problem for the stacked LSM effort. Unfortunately it
sounds like you haven't yet made any serious effort into resolving that
problem other than saying "don't do that".
Now, circling back to the issue of secid/blob in the AF_VSOCK/VMCI context ...
based on Andy's email I think I'm still missing some critical bit of
understanding regarding how VMCI is used so let's punt on this for a moment;
however, your preference for a blob is noted (you also remember that I prefer
blobs when they make sense, reference a lot of our earlier discussions).
--
paul moore
security and virtualization @ redhat
^ permalink raw reply
* Re: AF_VSOCK and the LSMs
From: Paul Moore @ 2013-02-23 0:27 UTC (permalink / raw)
To: Andy King
Cc: netdev, linux-security-module, selinux, Gerd Hoffmann, Eric Paris
In-Reply-To: <888679886.3769933.1361573683299.JavaMail.root@vmware.com>
On Friday, February 22, 2013 02:54:43 PM Andy King wrote:
> Hi Paul,
>
> > to see if anyone had any strong feelings on this approach (either good or
> > bad). Here is what I am proposing, and currently working on ...
> >
> > * Add a LSM secid/blob to the vmci_datagram struct
>
> I think perhaps this is the wrong layer at which to embed this. Think
> of that structure as an ethernet header, with VMCI being ethernet; it's
> what the device (and the hypervisor and peer) understand. So this
> really cannot be changed.
Hmmm, so can VMware/VMCI-enabled guests send vmci_datagram packets directly
into the kernel? It isn't wrapped by things like AF_VSOCK? If that is the
case, then yes, we'll probably need to add a thin wrapper struct to carry the
security label; similar to the control packets but not quite, as we have data
to deal with unlike the control packets. However, if vmci_datagram is an
internal only structure, why not add the extra field?
Either way, we should be able to work around this, it would just be cleaner if
we could add it to the datagram directly.
> It's also not entirely clear to me how this will work in a heterogeneous
> environments. What if there's a Linux guest running on a Windows host,
> or vice-versa?
I maybe missing something here, but VMCI never leaves the physical host system
correct? It doesn't get tunneled over some external network does it?
Assuming it stays on the physical host system then we don't really care about
a Windows host in this context do we? From a guests point of view it doesn't
really matter, the kernel handles all of the labeling and access control; the
guests create their AF_VSOCKS as they normally would.
> I'll take a closer read at the rest of your mail, but I think we need to
> address the above first.
I think there is some confusion about VMCI - which is almost surely on my end
- and what I'm trying to accomplish with the labeling, perhaps by answering
the above questions you can help me gain a better understanding and we can
sort things out.
Thanks.
--
paul moore
security and virtualization @ redhat
^ permalink raw reply
* How to hairpin two Ethernet interfaces together (L2 hub/repeater mode)?
From: Petri Gynther @ 2013-02-23 0:10 UTC (permalink / raw)
To: netdev
Hi netdev:
I'd like to hairpin two Ethernet interfaces together (e.g. eth0 and
eth1) so that any frame received on eth0 is forwarded as-is to eth1
and vice versa.
Does Linux networking code support this? I've looked at the bridging
code, but it doesn't seem to do this (at least trivially).
Specifically, when eth0 and eth1 are added to bridge, unicast frames
destined to eth0 (or eth1) MAC address end up to the bridge interface
for local consumption, rather than being flooded to other ports on the
bridge.
Any trivial solutions to this? Can Linux bridge be configured in L2
hub/repeater mode so that it simply floods every Rx frame to all other
ports of the bridge?
^ permalink raw reply
* Re: Fw: [Bug 54231] r8169 driver regression caused by the commit aee77e4accbeb2c86b1d294cd84fec4a12dde3bd
From: Francois Romieu @ 2013-02-22 23:09 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, tomimo
In-Reply-To: <20130222095546.396e63a8@nehalam.linuxnetplumber.net>
Stephen Hemminger <stephen@networkplumber.org> :
[...]
> https://bugzilla.kernel.org/show_bug.cgi?id=54231
>
> --- Comment #1 from Tomi Orava <tomimo@ncircle.nullnet.fi> 2013-02-22 16:40:49 ---
> Although the r8169 has been working just fine on 3.4.31 for the past 5 days, it
> seems that I missed the second DMA Burst setting in the previous patch that
> should get fixed as well:
[...]
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -4406,8 +4407,14 @@ static void rtl_set_rx_tx_config_registers(struct
> rtl8169_private *tp)
> void __iomem *ioaddr = tp->mmio_addr;
>
> /* Set DMA burst size and Interframe Gap Time */
> - RTL_W32(TxConfig, (TX_DMA_BURST << TxDMAShift) |
> - (InterFrameGap << TxInterFrameGapShift));
> +
> + if (tp->mac_version == RTL_GIGA_MAC_VER_11) {
> + RTL_W32(TxConfig, (TX_DMA_BURST_512 << TxDMAShift) |
> + (InterFrameGap << TxInterFrameGapShift));
> + } else {
> + RTL_W32(TxConfig, (TX_DMA_BURST << TxDMAShift) |
> + (InterFrameGap << TxInterFrameGapShift));
> + }
> }
This one should not be used in the (RTL_CFG_1) 8168 / RTL_GIGA_MAC_VER_11
path.
Tomi, what does lspci say about your 8168b device ?
> static void rtl_hw_start(struct net_device *dev)
> @@ -5148,8 +5155,13 @@ static void rtl_hw_start_8168(struct net_device *dev)
>
> rtl_set_rx_mode(dev);
>
> - RTL_W32(TxConfig, (TX_DMA_BURST << TxDMAShift) |
> - (InterFrameGap << TxInterFrameGapShift));
> + if (tp->mac_version == RTL_GIGA_MAC_VER_11) {
> + RTL_W32(TxConfig, (TX_DMA_BURST_512 << TxDMAShift) |
> + (InterFrameGap << TxInterFrameGapShift));
> + } else {
> + RTL_W32(TxConfig, (TX_DMA_BURST << TxDMAShift) |
> + (InterFrameGap << TxInterFrameGapShift));
> + }
RTL_W32(TxConfig, (InterFrameGap << TxInterFrameGapShift) |
(((tp->mac_version == RTL_GIGA_MAC_VER_11) ?
TX_DMA_BURST_512 : TX_DMA_BURST) << TxDMAShift));
--
Ueimor
^ permalink raw reply
* Re: Problem with Reaktek 8168
From: Francois Romieu @ 2013-02-22 23:07 UTC (permalink / raw)
To: Ralf Friedl; +Cc: netdev
In-Reply-To: <512786D7.6090003@online.de>
Ralf Friedl <Ralf.Friedl@online.de> :
[...]
> I have one warning in the syslog, but I hope it is not serious.
> WARNING: at net/sched/sch_generic.c:254 dev_watchdog+0x239/0x250()
> NETDEV WATCHDOG: eth0 (r8169): transmit queue 0 timed out
[snip]
It's the usual transmit timeout watchdog. A transient loss of network
traffic should be noticable when it triggers. You may live with it as
long as it does not happen much but it means that something does not
work correctly.
--
Ueimor
^ permalink raw reply
* Re: AF_VSOCK and the LSMs
From: Casey Schaufler @ 2013-02-22 23:00 UTC (permalink / raw)
To: Paul Moore
Cc: netdev, linux-security-module, selinux, Andy King, Gerd Hoffmann,
Eric Paris, Casey Schaufler
In-Reply-To: <1803195.0cVPJuGAEx@sifl>
On 2/22/2013 2:33 PM, Paul Moore wrote:
> With AF_VSOCK now in the next tree I've started looking at what we would need
> to do to add the appropriate LSM hooks so that AF_VSOCK traffic can be
> controlled just as we do with other protocols. The reasons why are pretty
> simple, those users who rely on LSMs to help enforce separation between VMs
> want to be able to ensure that the separation extends down to this new method
> of communication, especially considering it provides a mechanism for
> communication between VMs.
>
> Looking at AF_VSOCK, and the underlying VMCI transport (currently the only
> transport option), it looks fairly similar to AF_INET, at least it is much
> similar to AF_INET than it is to AF_UNIX. With that in mind, taking an
> approach similar to what we currently do for AF_INET should make the necessary
> LSM kernel changes smaller and the related, LSM-specific security policy more
> approachable to those already familiar with the LSM's network access controls.
>
> Perhaps the biggest different between the current AF_VSOCK/VMCI combination
> and AF_INET is the lack of labeling support at the transport layer. Basically
> the labeling in AF_INET, via protocols that leverage IP, allow the sender to
> tag traffic with a security label that can be used to perform access control
> on receipt of the traffic. Since AF_VSOCK traffic sent over the VMCI
> transport does not carry any LSM related information about the sender we are
> not able to do the same level of access control. However, if we were to
> augment the current VMCI tranport to tag traffic with the security label of
> the sender we could start doing proper LSM based access control with AF_VSOCK
> traffic.
>
> I'm currently working on a set of patches to do just that, but before I get
> too far down this path, I thought I would toss this out to the various lists
> to see if anyone had any strong feelings on this approach (either good or
> bad). Here is what I am proposing, and currently working on ...
>
> * Add a LSM secid/blob to the vmci_datagram struct
Please add an LSM blob. Please do not use a secid. I am currently
battling with secids in my efforts for multiple LSM support.
> First some background on LSM stacking: there is a lot of work going on to look
> at stacking different LSMs (currently it is a one-at-a-time system) but
> unfortunately due to a lack of a proper security blob (e.g. we would need a
> security void pointer in the sk_buff) it is unlikely that we will be able to
> stack LSMs which use network labels anytime soon. With that in mind, while
> some on the LSM list would likely argue for a security blob being added to
> struct vmci_datagram I think the easiest solution for the time being is to
> just add a secid field (a single u32 scalar); true it is probably not ideal
> but it simplifies the management of the field considerably and is no worse
> than what we currently have for AF_INET[6]. In the future we could always add
> a proper LSM blob if needed as this is internal and private to the kernel.
Please! No secids!
I am going to be able to deal with secids for AF_INET only because
SELinux prefers XFRM, Smack requires CIPSO, and AppArmor is going to
be willing to have networking be optional.
If you have two LSMs that use secids you are never going to have a
rational way to get the information for both into one secid.
>
> With the background out of the way, adding a LSM secid/blob to struct
> vmci_datagram would allow us to convey the sending socket's LSM label with the
> rest of the VMCI datagram/packet to the receiving socket where we could
> perform a LSM access check using the sender and receiver's LSM labels. Once
> again, this is very similar to what we currently do with AF_INET[6].
>
> In order to do this we would need to make some changes to a few functions,
> mostly just to ensure we have access to the necessary socket labels when
> needed:
>
> - vmci_transport_send_control_pkt()
> - vmci_transport_reply_control_pkt_fast() / vmci_transport_reply_reset()
> - vmci_transport_send_control_pkt_bh() / *notify_pkt() handler
> - vmci_tansport_send_control_pkt()
>
> This may not be a complete list, and I'm being vague on the actual changes as
> this is currently a work in progress ...
>
> * Add LSM hooks to vmci_transport_recv_{dgram,stream}_cb()
>
> In both cases we would probably want the LSM access control hook/check to
> occur just after the call to vmci_transport_allow_dgram(). While I haven't
> gotten to this part of the patchset yet, I expect the LSM hook to look very
> similar to the existing security_sock_rcv_skb() hook; in fact, I hope to just
> reuse the existing hook but we will have to see how things develop.
>
> * Add LSM hooks to vmci_transport_recv_connecting_{client,server}()
>
> This isn't so much an access control point, that is handled above, but rather
> a notification for the LSM that the negotiation has finished and the sockets
> are connected. This notification allows the LSM to update any internal state,
> e.g. the socket's peer labels.
>
> * Update the SELinux and Smack LSMs to support the AF_VSOCK address family
>
> Essentially this means just making sure that the socket level access controls,
> and perhaps some of the packet level controls if reused, understand the
> AF_VSOCK family and do the right thing. For Smack this should be rather
> trivial, for SELinux it will be slightly more involved but still rather simple
> and straightforward (perhaps add a new "virt_socket" object class).
>
> The other LSMs, TOMOYO and AppArmor, handle their network access controls
> differently and as a result, I believe there is no work needed for these LSMs
> but I would encourage the TOMOYO and AppArmor devs to correct me if I missed
> anything.
>
^ permalink raw reply
* Re: AF_VSOCK and the LSMs
From: Andy King @ 2013-02-22 22:54 UTC (permalink / raw)
To: Paul Moore
Cc: netdev, linux-security-module, selinux, Gerd Hoffmann, Eric Paris
In-Reply-To: <1803195.0cVPJuGAEx@sifl>
Hi Paul,
> to see if anyone had any strong feelings on this approach (either good or
> bad). Here is what I am proposing, and currently working on ...
>
> * Add a LSM secid/blob to the vmci_datagram struct
I think perhaps this is the wrong layer at which to embed this. Think
of that structure as an ethernet header, with VMCI being ethernet; it's
what the device (and the hypervisor and peer) understand. So this
really cannot be changed.
It's also not entirely clear to me how this will work in a heterogeneous
environments. What if there's a Linux guest running on a Windows host,
or vice-versa?
I'll take a closer read at the rest of your mail, but I think we need to
address the above first.
Thanks!
- Andy
^ permalink raw reply
* AF_VSOCK and the LSMs
From: Paul Moore @ 2013-02-22 22:33 UTC (permalink / raw)
To: netdev, linux-security-module, selinux
Cc: Andy King, Gerd Hoffmann, Eric Paris
With AF_VSOCK now in the next tree I've started looking at what we would need
to do to add the appropriate LSM hooks so that AF_VSOCK traffic can be
controlled just as we do with other protocols. The reasons why are pretty
simple, those users who rely on LSMs to help enforce separation between VMs
want to be able to ensure that the separation extends down to this new method
of communication, especially considering it provides a mechanism for
communication between VMs.
Looking at AF_VSOCK, and the underlying VMCI transport (currently the only
transport option), it looks fairly similar to AF_INET, at least it is much
similar to AF_INET than it is to AF_UNIX. With that in mind, taking an
approach similar to what we currently do for AF_INET should make the necessary
LSM kernel changes smaller and the related, LSM-specific security policy more
approachable to those already familiar with the LSM's network access controls.
Perhaps the biggest different between the current AF_VSOCK/VMCI combination
and AF_INET is the lack of labeling support at the transport layer. Basically
the labeling in AF_INET, via protocols that leverage IP, allow the sender to
tag traffic with a security label that can be used to perform access control
on receipt of the traffic. Since AF_VSOCK traffic sent over the VMCI
transport does not carry any LSM related information about the sender we are
not able to do the same level of access control. However, if we were to
augment the current VMCI tranport to tag traffic with the security label of
the sender we could start doing proper LSM based access control with AF_VSOCK
traffic.
I'm currently working on a set of patches to do just that, but before I get
too far down this path, I thought I would toss this out to the various lists
to see if anyone had any strong feelings on this approach (either good or
bad). Here is what I am proposing, and currently working on ...
* Add a LSM secid/blob to the vmci_datagram struct
First some background on LSM stacking: there is a lot of work going on to look
at stacking different LSMs (currently it is a one-at-a-time system) but
unfortunately due to a lack of a proper security blob (e.g. we would need a
security void pointer in the sk_buff) it is unlikely that we will be able to
stack LSMs which use network labels anytime soon. With that in mind, while
some on the LSM list would likely argue for a security blob being added to
struct vmci_datagram I think the easiest solution for the time being is to
just add a secid field (a single u32 scalar); true it is probably not ideal
but it simplifies the management of the field considerably and is no worse
than what we currently have for AF_INET[6]. In the future we could always add
a proper LSM blob if needed as this is internal and private to the kernel.
With the background out of the way, adding a LSM secid/blob to struct
vmci_datagram would allow us to convey the sending socket's LSM label with the
rest of the VMCI datagram/packet to the receiving socket where we could
perform a LSM access check using the sender and receiver's LSM labels. Once
again, this is very similar to what we currently do with AF_INET[6].
In order to do this we would need to make some changes to a few functions,
mostly just to ensure we have access to the necessary socket labels when
needed:
- vmci_transport_send_control_pkt()
- vmci_transport_reply_control_pkt_fast() / vmci_transport_reply_reset()
- vmci_transport_send_control_pkt_bh() / *notify_pkt() handler
- vmci_tansport_send_control_pkt()
This may not be a complete list, and I'm being vague on the actual changes as
this is currently a work in progress ...
* Add LSM hooks to vmci_transport_recv_{dgram,stream}_cb()
In both cases we would probably want the LSM access control hook/check to
occur just after the call to vmci_transport_allow_dgram(). While I haven't
gotten to this part of the patchset yet, I expect the LSM hook to look very
similar to the existing security_sock_rcv_skb() hook; in fact, I hope to just
reuse the existing hook but we will have to see how things develop.
* Add LSM hooks to vmci_transport_recv_connecting_{client,server}()
This isn't so much an access control point, that is handled above, but rather
a notification for the LSM that the negotiation has finished and the sockets
are connected. This notification allows the LSM to update any internal state,
e.g. the socket's peer labels.
* Update the SELinux and Smack LSMs to support the AF_VSOCK address family
Essentially this means just making sure that the socket level access controls,
and perhaps some of the packet level controls if reused, understand the
AF_VSOCK family and do the right thing. For Smack this should be rather
trivial, for SELinux it will be slightly more involved but still rather simple
and straightforward (perhaps add a new "virt_socket" object class).
The other LSMs, TOMOYO and AppArmor, handle their network access controls
differently and as a result, I believe there is no work needed for these LSMs
but I would encourage the TOMOYO and AppArmor devs to correct me if I missed
anything.
--
paul moore
security and virtualization @ redhat
^ permalink raw reply
* [PATCH net-next 2/2] VXLAN: Use tunnel_ip_select_ident() for tunnel IP-Identification.
From: Pravin B Shelar @ 2013-02-22 17:30 UTC (permalink / raw)
To: netdev; +Cc: edumazet, jesse, stephen, Pravin B Shelar
tunnel_ip_select_ident() is more efficient when generating ip-header
id given inner packet is of ipv4 type.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
drivers/net/vxlan.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 9d70421..f736823 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -33,6 +33,7 @@
#include <net/arp.h>
#include <net/ndisc.h>
#include <net/ip.h>
+#include <net/ipip.h>
#include <net/icmp.h>
#include <net/udp.h>
#include <net/rtnetlink.h>
@@ -962,13 +963,13 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
iph->daddr = dst;
iph->saddr = fl4.saddr;
iph->ttl = ttl ? : ip4_dst_hoplimit(&rt->dst);
+ tunnel_ip_select_ident(skb, old_iph, &rt->dst);
vxlan_set_owner(dev, skb);
/* See iptunnel_xmit() */
if (skb->ip_summed != CHECKSUM_PARTIAL)
skb->ip_summed = CHECKSUM_NONE;
- ip_select_ident(iph, &rt->dst, NULL);
err = ip_local_out(skb);
if (likely(net_xmit_eval(err) == 0)) {
--
1.7.1
^ permalink raw reply related
* [PATCH net-next 1/2 v2] IP_GRE: Fix IP-Identification.
From: Pravin B Shelar @ 2013-02-22 17:30 UTC (permalink / raw)
To: netdev; +Cc: edumazet, jesse, stephen, Pravin B Shelar
GRE-GSO generates ip fragments with id 0,2,3,4... for every
GSO packet, which is not correct. Following patch fixes it
by setting ip-header id unique id of fragments are allowed.
As Eric Dumazet suggested it is optimized by using inner ip-header
whenever inner packet is ipv4.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
include/net/ipip.h | 17 +++++++++++++++++
net/ipv4/af_inet.c | 6 ++++--
net/ipv4/ip_gre.c | 3 ++-
3 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/include/net/ipip.h b/include/net/ipip.h
index 21947cf..fd19625 100644
--- a/include/net/ipip.h
+++ b/include/net/ipip.h
@@ -71,4 +71,21 @@ static inline void iptunnel_xmit(struct sk_buff *skb, struct net_device *dev)
}
}
+static inline void tunnel_ip_select_ident(struct sk_buff *skb,
+ const struct iphdr *old_iph,
+ struct dst_entry *dst)
+{
+ struct iphdr *iph = ip_hdr(skb);
+
+ if (iph->frag_off & htons(IP_DF))
+ iph->id = 0;
+ else {
+ /* Use inner packet iph-id if possible. */
+ if (skb->protocol == htons(ETH_P_IP) && old_iph->id)
+ iph->id = old_iph->id;
+ else
+ __ip_select_ident(iph, dst,
+ (skb_shinfo(skb)->gso_segs ?: 1) - 1);
+ }
+}
#endif
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index e225a4e..2992f37 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1327,8 +1327,10 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
if (skb->next != NULL)
iph->frag_off |= htons(IP_MF);
offset += (skb->len - skb->mac_len - iph->ihl * 4);
- } else
- iph->id = htons(id++);
+ } else {
+ if (!(iph->frag_off & htons(IP_DF)))
+ iph->id = htons(id++);
+ }
iph->tot_len = htons(skb->len - skb->mac_len);
iph->check = 0;
iph->check = ip_fast_csum(skb_network_header(skb), iph->ihl);
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 5ef4da7..b8bada0 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -970,7 +970,8 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev
iph->daddr = fl4.daddr;
iph->saddr = fl4.saddr;
iph->ttl = ttl;
- iph->id = 0;
+
+ tunnel_ip_select_ident(skb, old_iph, &rt->dst);
if (ttl == 0) {
if (skb->protocol == htons(ETH_P_IP))
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] net: Fix locking bug in netif_set_xps_queue
From: David Miller @ 2013-02-22 21:44 UTC (permalink / raw)
To: alexander.h.duyck; +Cc: netdev, fengguang.wu, dan.carpenter, jeffrey.t.kirsher
In-Reply-To: <5127E145.2070008@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
Date: Fri, 22 Feb 2013 13:21:09 -0800
> On 02/22/2013 12:14 PM, David Miller wrote:
>> From: Alexander Duyck <alexander.h.duyck@intel.com>
>> Date: Fri, 22 Feb 2013 08:38:44 -0800
>>
>>> Smatch found a locking bug in netif_set_xps_queue in which we were not
>>> releasing the lock in the case of an allocation failure.
>>>
>>> This change corrects that so that we release the xps_map_mutex before
>>> returning -ENOMEM in the case of an allocation failure.
>>>
>>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> Applied and queue up for -stable, thanks Alex.
>
> No patch is needed for -stable. This was something I had introduced
> into net-next when I moved the XPS logic into its own function.
Great, thanks for letting me know.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox