* Re: Fw: [Bug 54491] New: UFO (UDP fragmentation offload) does not work if the payload size specified is within the range ( (MTU-28) to (MTU-8) )
From: Cong Wang @ 2013-02-27 3:20 UTC (permalink / raw)
To: netdev
In-Reply-To: <20130226074606.4d16ebfb@nehalam.linuxnetplumber.net>
On Tue, 26 Feb 2013 at 15:46 GMT, Stephen Hemminger <stephen@networkplumber.org> wrote:
> For IPv4/UDP if the payload size specified is within the range ( (MTU-28)
> to (MTU-8) ) packet gets segmented in kernel even if the following netdev
> features are set:
>
> NETIF_F_SG
> NETIF_F_GEN_CSUM
> NETIF_F_UFO
> NETIF_F_TSO
> NETIF_F_GSO
> NETIF_F_GSO_ROBUST
>
> Analysis:
> udp_sendmsg() computes packet length by adding UDP header size to payload size:
> “ulen += sizeof(struct udphdr)”
> Then the function calls ip_make_skb(), however IP header size is not added to
> the packet length.
> ip_make_skb() in turn calls __ip_append_data(). The same packet length is used
> here.
> __ip_append_data() compares the packet length with MTU. The packet length still
> does not include IP header size (20 bytes).
> In result, when payload size “is within the range ( (MTU-28) to (MTU-8) )”
> standard branch is taken rather than ip_ufo_append_data(). Gso_size is not
> computed, therefore IP fragmentation is triggered from ip_finish_output().
>
I haven't looked into this deeply, it sounds like we need this fix:
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 5e12dca..3ccb704 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -845,7 +845,7 @@ static int __ip_append_data(struct sock *sk,
csummode = CHECKSUM_PARTIAL;
cork->length += length;
- if (((length > mtu) || (skb &&skb_is_gso(skb))) &&
+ if (((length + fragheaderlen > mtu) || (skb &&skb_is_gso(skb))) &&
(sk->sk_protocol == IPPROTO_UDP) &&
(rt->dst.dev->features & NETIF_F_UFO) && !rt->dst.header_len) {
err = ip_ufo_append_data(sk, queue, getfrag, from,length,
^ permalink raw reply related
* Re: [PATCH 4/4] sctp: fix association hangs due to partial delivery errors
From: Vlad Yasevich @ 2013-02-27 2:34 UTC (permalink / raw)
To: Lee A. Roberts; +Cc: netdev
In-Reply-To: <1361889376-22171-5-git-send-email-lee.roberts@hp.com>
On 02/26/2013 09:36 AM, Lee A. Roberts wrote:
> From: "Lee A. Roberts" <lee.roberts@hp.com>
>
> Resolve SCTP association hangs observed during SCTP stress
> testing. Observable symptoms include communications hangs
> with data being held in the association reassembly and/or lobby
> (ordering) queues. Close examination of reassembly queue shows
> missing packets.
>
> In sctp_ulpq_tail_data(), use return values 0,1 to indicate whether
> a complete event (with MSG_EOR set) was delivered. A return value
> of -ENOMEM continues to indicate an out-of-memory condition was
> encountered.
>
> In sctp_ulpq_retrieve_partial() and sctp_ulpq_retrieve_first(),
> correct message reassembly logic for SCTP partial delivery.
> Change logic to ensure that as much data as possible is sent
> with the initial partial delivery and that following partial
> deliveries contain all available data.
>
> In sctp_ulpq_partial_delivery(), attempt partial delivery only
> if the data on the head of the reassembly queue is at or before
> the cumulative TSN ACK point.
>
> In sctp_ulpq_renege(), use the modified return values from
> sctp_ulpq_tail_data() to choose whether to attempt partial
> delivery or to attempt to drain the reassembly queue as a
> means to reduce memory pressure. Remove call to
> sctp_tsnmap_mark(), as this is handled correctly in call to
> sctp_ulpq_tail_data().
>
> Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
> ---
> net/sctp/ulpqueue.c | 54 ++++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 43 insertions(+), 11 deletions(-)
>
> diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c
> index f221fbb..482e3ea 100644
> --- a/net/sctp/ulpqueue.c
> +++ b/net/sctp/ulpqueue.c
> @@ -106,6 +106,7 @@ int sctp_ulpq_tail_data(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
> {
> struct sk_buff_head temp;
> struct sctp_ulpevent *event;
> + int event_eor = 0;
>
> /* Create an event from the incoming chunk. */
> event = sctp_ulpevent_make_rcvmsg(chunk->asoc, chunk, gfp);
> @@ -127,10 +128,12 @@ int sctp_ulpq_tail_data(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
> /* Send event to the ULP. 'event' is the sctp_ulpevent for
> * very first SKB on the 'temp' list.
> */
> - if (event)
> + if (event) {
> + event_eor = (event->msg_flags & MSG_EOR) ? 1 : 0;
> sctp_ulpq_tail_event(ulpq, event);
> + }
You can re-use the check just before this one to catch the EOR
condition. You already do
if ((event) && (event->msg_flags & MSG_EOR)){
right after you try to get a reassembled event.
Everything else looks good.
-vlad
>
> - return 0;
> + return event_eor;
> }
>
> /* Add a new event for propagation to the ULP. */
> @@ -540,14 +543,19 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_partial(struct sctp_ulpq *ulpq)
> ctsn = cevent->tsn;
>
> switch (cevent->msg_flags & SCTP_DATA_FRAG_MASK) {
> + case SCTP_DATA_FIRST_FRAG:
> + if (!first_frag)
> + return NULL;
> + goto done;
> case SCTP_DATA_MIDDLE_FRAG:
> if (!first_frag) {
> first_frag = pos;
> next_tsn = ctsn + 1;
> last_frag = pos;
> - } else if (next_tsn == ctsn)
> + } else if (next_tsn == ctsn) {
> next_tsn++;
> - else
> + last_frag = pos;
> + } else
> goto done;
> break;
> case SCTP_DATA_LAST_FRAG:
> @@ -651,6 +659,14 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_first(struct sctp_ulpq *ulpq)
> } else
> goto done;
> break;
> +
> + case SCTP_DATA_LAST_FRAG:
> + if (!first_frag)
> + return NULL;
> + else
> + goto done;
> + break;
> +
> default:
> return NULL;
> }
> @@ -1025,16 +1041,28 @@ void sctp_ulpq_partial_delivery(struct sctp_ulpq *ulpq,
> struct sctp_ulpevent *event;
> struct sctp_association *asoc;
> struct sctp_sock *sp;
> + __u32 ctsn;
> + struct sk_buff *skb;
>
> asoc = ulpq->asoc;
> sp = sctp_sk(asoc->base.sk);
>
> /* If the association is already in Partial Delivery mode
> - * we have noting to do.
> + * we have nothing to do.
> */
> if (ulpq->pd_mode)
> return;
>
> + /* Data must be at or below the Cumulative TSN ACK Point to
> + * start partial delivery.
> + */
> + skb = skb_peek(&asoc->ulpq.reasm);
> + if (skb != NULL) {
> + ctsn = sctp_skb2event(skb)->tsn;
> + if (!TSN_lte(ctsn, sctp_tsnmap_get_ctsn(&asoc->peer.tsn_map)))
> + return;
> + }
> +
> /* If the user enabled fragment interleave socket option,
> * multiple associations can enter partial delivery.
> * Otherwise, we can only enter partial delivery if the
> @@ -1077,12 +1105,16 @@ void sctp_ulpq_renege(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
> }
> /* If able to free enough room, accept this chunk. */
> if (chunk && (freed >= needed)) {
> - __u32 tsn;
> - tsn = ntohl(chunk->subh.data_hdr->tsn);
> - sctp_tsnmap_mark(&asoc->peer.tsn_map, tsn, chunk->transport);
> - sctp_ulpq_tail_data(ulpq, chunk, gfp);
> -
> - sctp_ulpq_partial_delivery(ulpq, gfp);
> + int retval;
> + retval = sctp_ulpq_tail_data(ulpq, chunk, gfp);
> + /*
> + * Enter partial delivery if chunk has not been
> + * delivered; otherwise, drain the reassembly queue.
> + */
> + if (retval <= 0)
> + sctp_ulpq_partial_delivery(ulpq, chunk, gfp);
> + else if (retval == 1)
> + sctp_ulpq_reasm_drain(ulpq);
> }
>
> sk_mem_reclaim(asoc->base.sk);
>
^ permalink raw reply
* Re: [RFC PATCH] core: Add ioctls to control device unicast hw addresses
From: Vlad Yasevich @ 2013-02-27 2:27 UTC (permalink / raw)
To: John Fastabend; +Cc: David Miller, john.r.fastabend, netdev
In-Reply-To: <20130227020744.GA25063@nitbit.x32>
On 02/26/2013 09:07 PM, John Fastabend wrote:
> On Tue, Feb 26, 2013 at 05:15:03PM -0500, David Miller wrote:
>> From: John Fastabend <john.r.fastabend@intel.com>
>> Date: Tue, 26 Feb 2013 13:58:39 -0800
>>
>>> [...]
>>>
>>>>>>
>>>>>>
>>>>>> [RFC PATCH] rtnetlink: Add support for adding/removing additional hw
>>>>>> addresses.
>>>>>>
>>>>>> Add an ability to add and remove HW addresses to the device
>>>>>> unicast and multicast address lists. Right now, we only have
>>>>>> an ioctl() to manage the multicast addresses and there is no
>>>>>> way the manage the unicast list.
>>>>>>
>>>>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>>>>>
>>>>> This is a step in the right direction, and you're right that there is
>>>>> a difficulty in detecting whether support exists or not.
>>>>>
>>>>> I am so surprised that we've have ->set_rx_mode() support for multiple
>>>>> unicast MAC addresses in so many drivers all this time, yet no way
>>>>> outside of FDB to make use of it at all.
>>>>
>>>> And even that is not always available. In most drivers it requires
>>>> module parameters or other explicit configuration steps. Meanwhile
>>>> set_rx_mode() doesn't seem to depend on any of those and just does the
>>>> right thing.
>>>>
>>>> For what I was trying to do ioctl() was a really easy way out for both
>>>> kernel and user space implementation, so I gave is shot.
>>>>
>>>> -vlad
>>>>
>>>
>>> Don't we already support this with
>>
>> The whole point is that these multiple-unicast-address configuration
>> facilities are inaccessible without FDB, and there is no reason
>> whatsoever for that.
>
> Yes I see now sorry I was behind the thread.
>
> We could just use the fdb hooks and add a dflt routine to handle the
> case where the netdev doesn't provide a specific ndo_op for us. This
> boils down to calling set_rx_mode anyways through this call chain,
>
> ndo_fdb_add
> dev_uc_add_excl
> __dev_set_rx_mode
> ndo_set_rx_mode
>
> As long as folks don't think this is too much of an abuse of the fdb
> hooks. Here's an example I only compile tested this so take it as
> an example only. It probably needs some cleanups and the dump routine
> needs some thought not sure you want to dump all MACs always.
I thought about doing, but this becomes broken on the drivers that
support ndo_fdb_* functions. Those drivers mainly require additional
configuration that is not necessary for dev_uc_add_excl() to work.
For example, ixgbe and melanox requires VF to be on, qlogic needs a
module parameter, macvlan has to be in passthrough. However,
ndo_set_rx_mode() in most cases doesn't care about those settings and
just works.
So fdb will not always work even if the driver has proper support for
IFF_UNICAST_FLT.
-vlad
>
>
> [RFC PATCH] net: fdb: generic set support for drivers without ndo_op
>
> If the driver does not support the ndo_op use the generic
> handler for it. This should work in the majority of cases.
> Eventually the fdb_dflt_add call gets translated into a
> __dev_set_rx_mode() call which should handle hardware
> support for filtering via the IFF_UNICAST_FLT flag.
>
> Namely IFF_UNICAST_FLT indicates if the hardware can do
> unicast address filtering. If no support is available
> the device is put into promisc mode.
>
> It looks like we likely also need IFF_MULTICAST_FLT and
> also tighten up how drivers handle multicast filters. But
> thats another patch.
>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
> net/core/rtnetlink.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 74 insertions(+), 6 deletions(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index d8aa20f..d1c6f71 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -2049,6 +2049,37 @@ errout:
> rtnl_set_sk_err(net, RTNLGRP_NEIGH, err);
> }
>
> +/**
> + * ndo_dflt_fdb_add - default netdevice operation to add an FDB entry
> + */
> +static int ndo_dflt_fdb_add(struct ndmsg *ndm,
> + struct nlattr *tb[],
> + struct net_device *dev,
> + const unsigned char *addr,
> + u16 flags)
> +{
> + int err = -EINVAL;
> +
> + /* If aging addresses are supported device will need to
> + * implement its own handler for this.
> + */
> + if (ndm->ndm_state && !(ndm->ndm_state & NUD_PERMANENT)) {
> + pr_info("%s: FDB only supports static addresses\n", dev->name);
> + return err;
> + }
> +
> + if (is_unicast_ether_addr(addr) || is_link_local_ether_addr(addr))
> + err = dev_uc_add_excl(dev, addr);
> + else if (is_multicast_ether_addr(addr))
> + err = dev_mc_add_excl(dev, addr);
> +
> + /* Only return duplicate errors if NLM_F_EXCL is set */
> + if (err == -EEXIST && !(flags & NLM_F_EXCL))
> + err = 0;
> +
> + return err;
> +}
> +
> static int rtnl_fdb_add(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
> {
> struct net *net = sock_net(skb->sk);
> @@ -2101,10 +2132,14 @@ static int rtnl_fdb_add(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
> }
>
> /* Embedded bridge, macvlan, and any other device support */
> - if ((ndm->ndm_flags & NTF_SELF) && dev->netdev_ops->ndo_fdb_add) {
> - err = dev->netdev_ops->ndo_fdb_add(ndm, tb,
> - dev, addr,
> - nlh->nlmsg_flags);
> + if ((ndm->ndm_flags & NTF_SELF)) {
> + if (dev->netdev_ops->ndo_fdb_add)
> + err = dev->netdev_ops->ndo_fdb_add(ndm, tb,
> + dev, addr,
> + nlh->nlmsg_flags);
> + else
> + err = ndo_dflt_fdb_add(ndm, tb, dev, addr,
> + nlh->nlmsg_flags);
>
> if (!err) {
> rtnl_fdb_notify(dev, addr, RTM_NEWNEIGH);
> @@ -2115,6 +2150,34 @@ out:
> return err;
> }
>
> +/**
> + * ndo_dflt_fdb_del - default netdevice operation to delete an FDB entry
> + */
> +static int ndo_dflt_fdb_del(struct ndmsg *ndm,
> + struct nlattr *tb[],
> + struct net_device *dev,
> + const unsigned char *addr)
> +{
> + int err = -EOPNOTSUPP;
> +
> + /* If aging addresses are supported device will need to
> + * implement its own handler for this.
> + */
> + if (ndm->ndm_state & NUD_PERMANENT) {
> + pr_info("%s: FDB only supports static addresses\n", dev->name);
> + return -EINVAL;
> + }
> +
> + if (is_unicast_ether_addr(addr))
> + err = dev_uc_del(dev, addr);
> + else if (is_multicast_ether_addr(addr))
> + err = dev_mc_del(dev, addr);
> + else
> + err = -EINVAL;
> +
> + return err;
> +}
> +
> static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
> {
> struct net *net = sock_net(skb->sk);
> @@ -2172,8 +2235,11 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
> }
>
> /* Embedded bridge, macvlan, and any other device support */
> - if ((ndm->ndm_flags & NTF_SELF) && dev->netdev_ops->ndo_fdb_del) {
> - err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr);
> + if (ndm->ndm_flags & NTF_SELF) {
> + if (dev->netdev_ops->ndo_fdb_del)
> + err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr);
> + else
> + err = ndo_dflt_fdb_del(ndm, tb, dev, addr);
>
> if (!err) {
> rtnl_fdb_notify(dev, addr, RTM_DELNEIGH);
> @@ -2258,6 +2324,8 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
>
> if (dev->netdev_ops->ndo_fdb_dump)
> idx = dev->netdev_ops->ndo_fdb_dump(skb, cb, dev, idx);
> + else
> + ndo_dflt_fdb_dump(skb, cb, dev, idx);
> }
> rcu_read_unlock();
>
>
^ permalink raw reply
* Re: [RFC PATCH] core: Add ioctls to control device unicast hw addresses
From: John Fastabend @ 2013-02-27 2:07 UTC (permalink / raw)
To: David Miller; +Cc: john.r.fastabend, vyasevic, netdev
In-Reply-To: <20130226.171503.1736601811583226926.davem@davemloft.net>
On Tue, Feb 26, 2013 at 05:15:03PM -0500, David Miller wrote:
> From: John Fastabend <john.r.fastabend@intel.com>
> Date: Tue, 26 Feb 2013 13:58:39 -0800
>
> > [...]
> >
> >>>>
> >>>>
> >>>> [RFC PATCH] rtnetlink: Add support for adding/removing additional hw
> >>>> addresses.
> >>>>
> >>>> Add an ability to add and remove HW addresses to the device
> >>>> unicast and multicast address lists. Right now, we only have
> >>>> an ioctl() to manage the multicast addresses and there is no
> >>>> way the manage the unicast list.
> >>>>
> >>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> >>>
> >>> This is a step in the right direction, and you're right that there is
> >>> a difficulty in detecting whether support exists or not.
> >>>
> >>> I am so surprised that we've have ->set_rx_mode() support for multiple
> >>> unicast MAC addresses in so many drivers all this time, yet no way
> >>> outside of FDB to make use of it at all.
> >>
> >> And even that is not always available. In most drivers it requires
> >> module parameters or other explicit configuration steps. Meanwhile
> >> set_rx_mode() doesn't seem to depend on any of those and just does the
> >> right thing.
> >>
> >> For what I was trying to do ioctl() was a really easy way out for both
> >> kernel and user space implementation, so I gave is shot.
> >>
> >> -vlad
> >>
> >
> > Don't we already support this with
>
> The whole point is that these multiple-unicast-address configuration
> facilities are inaccessible without FDB, and there is no reason
> whatsoever for that.
Yes I see now sorry I was behind the thread.
We could just use the fdb hooks and add a dflt routine to handle the
case where the netdev doesn't provide a specific ndo_op for us. This
boils down to calling set_rx_mode anyways through this call chain,
ndo_fdb_add
dev_uc_add_excl
__dev_set_rx_mode
ndo_set_rx_mode
As long as folks don't think this is too much of an abuse of the fdb
hooks. Here's an example I only compile tested this so take it as
an example only. It probably needs some cleanups and the dump routine
needs some thought not sure you want to dump all MACs always.
[RFC PATCH] net: fdb: generic set support for drivers without ndo_op
If the driver does not support the ndo_op use the generic
handler for it. This should work in the majority of cases.
Eventually the fdb_dflt_add call gets translated into a
__dev_set_rx_mode() call which should handle hardware
support for filtering via the IFF_UNICAST_FLT flag.
Namely IFF_UNICAST_FLT indicates if the hardware can do
unicast address filtering. If no support is available
the device is put into promisc mode.
It looks like we likely also need IFF_MULTICAST_FLT and
also tighten up how drivers handle multicast filters. But
thats another patch.
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
net/core/rtnetlink.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 74 insertions(+), 6 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index d8aa20f..d1c6f71 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2049,6 +2049,37 @@ errout:
rtnl_set_sk_err(net, RTNLGRP_NEIGH, err);
}
+/**
+ * ndo_dflt_fdb_add - default netdevice operation to add an FDB entry
+ */
+static int ndo_dflt_fdb_add(struct ndmsg *ndm,
+ struct nlattr *tb[],
+ struct net_device *dev,
+ const unsigned char *addr,
+ u16 flags)
+{
+ int err = -EINVAL;
+
+ /* If aging addresses are supported device will need to
+ * implement its own handler for this.
+ */
+ if (ndm->ndm_state && !(ndm->ndm_state & NUD_PERMANENT)) {
+ pr_info("%s: FDB only supports static addresses\n", dev->name);
+ return err;
+ }
+
+ if (is_unicast_ether_addr(addr) || is_link_local_ether_addr(addr))
+ err = dev_uc_add_excl(dev, addr);
+ else if (is_multicast_ether_addr(addr))
+ err = dev_mc_add_excl(dev, addr);
+
+ /* Only return duplicate errors if NLM_F_EXCL is set */
+ if (err == -EEXIST && !(flags & NLM_F_EXCL))
+ err = 0;
+
+ return err;
+}
+
static int rtnl_fdb_add(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
{
struct net *net = sock_net(skb->sk);
@@ -2101,10 +2132,14 @@ static int rtnl_fdb_add(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
}
/* Embedded bridge, macvlan, and any other device support */
- if ((ndm->ndm_flags & NTF_SELF) && dev->netdev_ops->ndo_fdb_add) {
- err = dev->netdev_ops->ndo_fdb_add(ndm, tb,
- dev, addr,
- nlh->nlmsg_flags);
+ if ((ndm->ndm_flags & NTF_SELF)) {
+ if (dev->netdev_ops->ndo_fdb_add)
+ err = dev->netdev_ops->ndo_fdb_add(ndm, tb,
+ dev, addr,
+ nlh->nlmsg_flags);
+ else
+ err = ndo_dflt_fdb_add(ndm, tb, dev, addr,
+ nlh->nlmsg_flags);
if (!err) {
rtnl_fdb_notify(dev, addr, RTM_NEWNEIGH);
@@ -2115,6 +2150,34 @@ out:
return err;
}
+/**
+ * ndo_dflt_fdb_del - default netdevice operation to delete an FDB entry
+ */
+static int ndo_dflt_fdb_del(struct ndmsg *ndm,
+ struct nlattr *tb[],
+ struct net_device *dev,
+ const unsigned char *addr)
+{
+ int err = -EOPNOTSUPP;
+
+ /* If aging addresses are supported device will need to
+ * implement its own handler for this.
+ */
+ if (ndm->ndm_state & NUD_PERMANENT) {
+ pr_info("%s: FDB only supports static addresses\n", dev->name);
+ return -EINVAL;
+ }
+
+ if (is_unicast_ether_addr(addr))
+ err = dev_uc_del(dev, addr);
+ else if (is_multicast_ether_addr(addr))
+ err = dev_mc_del(dev, addr);
+ else
+ err = -EINVAL;
+
+ return err;
+}
+
static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
{
struct net *net = sock_net(skb->sk);
@@ -2172,8 +2235,11 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
}
/* Embedded bridge, macvlan, and any other device support */
- if ((ndm->ndm_flags & NTF_SELF) && dev->netdev_ops->ndo_fdb_del) {
- err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr);
+ if (ndm->ndm_flags & NTF_SELF) {
+ if (dev->netdev_ops->ndo_fdb_del)
+ err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr);
+ else
+ err = ndo_dflt_fdb_del(ndm, tb, dev, addr);
if (!err) {
rtnl_fdb_notify(dev, addr, RTM_DELNEIGH);
@@ -2258,6 +2324,8 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
if (dev->netdev_ops->ndo_fdb_dump)
idx = dev->netdev_ops->ndo_fdb_dump(skb, cb, dev, idx);
+ else
+ ndo_dflt_fdb_dump(skb, cb, dev, idx);
}
rcu_read_unlock();
^ permalink raw reply related
* Re: [PATCH iproute 2/2] ss: show socket memory stats for unix sockets if requested
From: Stephen Hemminger @ 2013-02-27 1:38 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: netdev
In-Reply-To: <20130223012818.GA6655@order.stressinduktion.org>
On Sat, 23 Feb 2013 02:28:18 +0100
Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> 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>
Both applied
^ permalink raw reply
* Re: [iproute2 ] Fix compilation error of m_ipt.c with -Werror enabled
From: Stephen Hemminger @ 2013-02-27 1:36 UTC (permalink / raw)
To: Vijay Subramanian; +Cc: netdev, Stephen Hemminger
In-Reply-To: <1361905927-31534-1-git-send-email-subramanian.vijay@gmail.com>
On Tue, 26 Feb 2013 11:12:07 -0800
Vijay Subramanian <subramanian.vijay@gmail.com> wrote:
> Commit (5a650703d47e10aa386406c855eff5a593b2120b Makefile: make warnings into
> errors ) causes the following build error.
>
> gcc -Wall -Wstrict-prototypes -Werror -Wmissing-prototypes
> -Wmissing-declarations -Wold-style-definition -O2 -I../include
> -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\"
> -D_GNU_SOURCE -DCONFIG_GACT -DCONFIG_GACT_PROB -DIPT_LIB_DIR=\"/lib/xtables\"
> -DYY_NO_INPUT -c -o m_ipt.o m_ipt.c
> cc1: warnings being treated as errors
> m_ipt.c:72: error: no previous prototype for 'xtables_register_target'
> m_ipt.c:361: error: no previous prototype for 'build_st'
> make[1]: *** [m_ipt.o] Error 1
>
> This is fixed by adding the prototype in the header include/iptables.h
>
> I am not sure if this is due to something wrong on my build system but I am
> using current glibc 2.17.
>
>
> Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
Applied, thanks
^ permalink raw reply
* Re: [PATCH v4] net: sh_eth: Add support of device tree probe
From: Nobuhiro Iwamatsu @ 2013-02-27 1:05 UTC (permalink / raw)
To: Simon Horman
Cc: Nobuhiro Iwamatsu, netdev, horms+renesas, magnus.damm,
devicetree-discuss, kda, kuninori.morimoto.gx
In-Reply-To: <20130216022638.GC11628@verge.net.au>
Hi, Simon.
Thanks for the supporting. I understood.
Best regards,
Nobuhiro
(2013/02/16 11:26), Simon Horman wrote:
> On Fri, Feb 15, 2013 at 10:53:25AM +0900, Nobuhiro Iwamatsu wrote:
>> From: Nobuhiro Iwamatsu<nobuhiro.iwamatsu.yj@renesas.com>
>>
>> This adds support of device tree probe for Renesas sh-ether driver.
>
> Hi Iwamatsu-san,
>
> I do not expect this code to go through the renesas tree. However, in
> order to provide a basis for work on renesas SoCs I have added this patch
> to the topic/sh_eth topic branch in the reneas tree on kernel.org and
> merged it into topic/all+next.
>
> In other words, I am not picking this patch up to merge it or add it to
> linux-next, rather I am storing it for reference.
>
> Also, I do note that there have already been some comments made on this
> patch and I plan to date topic/sh_eth topic branch as new versions are
> posted.
>
>> Signed-off-by: Nobuhiro Iwamatsu<nobuhiro.iwamatsu.yj@renesas.com>
>>
>> V4: - Remove empty sh_eth_parse_dt().
>> V3: - Removed sentnece of "needs-init" from document.
>> V2: - Removed ether_setup().
>> - Fixed typo from "sh-etn" to "sh-eth".
>> - Removed "needs-init" and sh-eth,endian from definition of DT.
>> - Changed "sh-eth,edmac-endian" instead of "sh-eth,edmac-big-endain"
>> in definition of DT.
>> ---
>> Documentation/devicetree/bindings/net/sh_ether.txt | 39 +++++
>> drivers/net/ethernet/renesas/sh_eth.c | 150 ++++++++++++++++----
>> 2 files changed, 165 insertions(+), 24 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/net/sh_ether.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/sh_ether.txt b/Documentation/devicetree/bindings/net/sh_ether.txt
>> new file mode 100644
>> index 0000000..f20d66a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/sh_ether.txt
>> @@ -0,0 +1,39 @@
>> +* Renesas Electronics SuperH EMAC
>> +
>> +This file provides information, what the device node
>> +for the sh_eth interface contains.
>> +
>> +Required properties:
>> +- compatible: "renesas,sh-eth";
>> +- interrupt-parent: The phandle for the interrupt controller that
>> + services interrupts for this device.
>> +- reg: Offset and length of the register set for the
>> + device.
>> +- interrupts: Interrupt mapping for the sh_eth interrupt
>> + sources (vector id).
>> +- phy-mode: String, operation mode of the PHY interface.
>> +- sh-eth,register-type: String, register type of sh_eth.
>> + Please select "gigabit", "fast-sh4" or
>> + "fast-sh3-sh2".
>> +- sh-eth,phy-id: PHY id.
>> +
>> +Optional properties:
>> +- local-mac-address: 6 bytes, mac address
>> +- sh-eth,no-ether-link: Set link control by software. When device does
>> + not support ether-link, set.
>> +- sh-eth,ether-link-active-low: Set link check method.
>> + When detection of link is treated as active-low,
>> + set.
>> +- sh-eth,edmac-big-endian: Set big endian for sh_eth dmac.
>> + It work as little endian when this is not set.
>> +
>> +Example (armadillo800eva):
>> + sh-eth@e9a00000 {
>> + compatible = "renesas,sh-eth";
>> + interrupt-parent =<&intca>;
>> + reg =<0xe9a00000 0x800>,<0xe9a01800 0x800>;
>> + interrupts =<0x500>;
>> + phy-mode = "mii";
>> + sh-eth,register-type = "gigabit";
>> + sh-eth,phy-id =<0>;
>> + };
>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
>> index 3d70586..b41249a 100644
>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>> @@ -1,7 +1,7 @@
>> /*
>> * SuperH Ethernet device driver
>> *
>> - * Copyright (C) 2006-2012 Nobuhiro Iwamatsu
>> + * Copyright (C) 2006-2013 Nobuhiro Iwamatsu
>> * Copyright (C) 2008-2012 Renesas Solutions Corp.
>> *
>> * This program is free software; you can redistribute it and/or modify it
>> @@ -31,6 +31,12 @@
>> #include<linux/platform_device.h>
>> #include<linux/mdio-bitbang.h>
>> #include<linux/netdevice.h>
>> +#include<linux/of.h>
>> +#include<linux/of_device.h>
>> +#include<linux/of_platform.h>
>> +#include<linux/of_address.h>
>> +#include<linux/of_irq.h>
>> +#include<linux/of_net.h>
>> #include<linux/phy.h>
>> #include<linux/cache.h>
>> #include<linux/io.h>
>> @@ -2347,26 +2353,103 @@ static const struct net_device_ops sh_eth_netdev_ops = {
>> .ndo_change_mtu = eth_change_mtu,
>> };
>>
>> +#ifdef CONFIG_OF
>> +
>> +static int
>> +sh_eth_of_get_register_type(struct device_node *np)
>> +{
>> + const char *of_str;
>> + int ret = of_property_read_string(np, "sh-eth,register-type",&of_str);
>> + if (ret)
>> + return ret;
>> +
>> + if (of_str&& !strcmp(of_str, "gigabit"))
>> + return SH_ETH_REG_GIGABIT;
>> +
>> + else if (of_str&& !strcmp(of_str, "fast-sh4"))
>> + return SH_ETH_REG_FAST_SH4;
>> + else if (of_str&& !strcmp(of_str, "fast-sh3-sh2"))
>> + return SH_ETH_REG_FAST_SH3_SH2;
>> + else
>> + return -EINVAL;
>> +}
>> +
>> +static struct sh_eth_plat_data *
>> +sh_eth_parse_dt(struct device *dev, struct net_device *ndev)
>> +{
>> + int ret;
>> + struct device_node *np = dev->of_node;
>> + struct sh_eth_plat_data *pdata;
>> +
>> + pdata = devm_kzalloc(dev, sizeof(struct sh_eth_plat_data),
>> + GFP_KERNEL);
>> + if (!pdata) {
>> + dev_err(dev, "%s: failed to allocate config data\n", __func__);
>> + return NULL;
>> + }
>> +
>> + pdata->phy_interface = of_get_phy_mode(np);
>> +
>> + of_property_read_u32(np, "sh-eth,phy-id",&pdata->phy);
>> +
>> + if (of_find_property(np, "sh-eth,edmac-big-endian", NULL))
>> + pdata->edmac_endian = EDMAC_BIG_ENDIAN;
>> + else
>> + pdata->edmac_endian = EDMAC_LITTLE_ENDIAN;
>> +
>> + if (of_find_property(np, "sh-eth,no-ether-link", NULL))
>> + pdata->no_ether_link = 1;
>> + else
>> + pdata->no_ether_link = 0;
>> +
>> + if (of_find_property(np, "sh-eth,ether-link-active-low", NULL))
>> + pdata->ether_link_active_low = 1;
>> + else
>> + pdata->ether_link_active_low = 0;
>> +
>> + ret = sh_eth_of_get_register_type(np);
>> + if (ret< 0)
>> + goto error;
>> + pdata->register_type = ret;
>> +
>> +#ifdef CONFIG_OF_NET
>> + if (!is_valid_ether_addr(ndev->dev_addr)) {
>> + const char *macaddr = of_get_mac_address(np);
>> + if (macaddr)
>> + memcpy(pdata->mac_addr, macaddr, ETH_ALEN);
>> + }
>> +#endif
>> +
>> + return pdata;
>> +
>> +error:
>> + devm_kfree(dev, pdata);
>> + return NULL;
>> +}
>> +#endif
>> +
>> static int sh_eth_drv_probe(struct platform_device *pdev)
>> {
>> - int ret, devno = 0;
>> + int ret = 0, devno = 0;
>> struct resource *res;
>> struct net_device *ndev = NULL;
>> struct sh_eth_private *mdp = NULL;
>> struct sh_eth_plat_data *pd;
>> + struct device_node *np = pdev->dev.of_node;
>> +
>> + ndev = alloc_etherdev(sizeof(struct sh_eth_private));
>> + if (!ndev) {
>> + ret = -ENOMEM;
>> + goto out;
>> + }
>>
>> + mdp = netdev_priv(ndev);
>> /* get base addr */
>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> if (unlikely(res == NULL)) {
>> dev_err(&pdev->dev, "invalid resource\n");
>> ret = -EINVAL;
>> - goto out;
>> - }
>> -
>> - ndev = alloc_etherdev(sizeof(struct sh_eth_private));
>> - if (!ndev) {
>> - ret = -ENOMEM;
>> - goto out;
>> + goto out_release;
>> }
>>
>> /* The sh Ether-specific entries in the device structure. */
>> @@ -2383,27 +2466,41 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
>> }
>> ndev->irq = ret;
>>
>> - SET_NETDEV_DEV(ndev,&pdev->dev);
>> -
>> - /* Fill in the fields of the device structure with ethernet values. */
>> - ether_setup(ndev);
>> -
>> - mdp = netdev_priv(ndev);
>> - mdp->num_tx_ring = TX_RING_SIZE;
>> - mdp->num_rx_ring = RX_RING_SIZE;
>> mdp->addr = ioremap(res->start, resource_size(res));
>> if (mdp->addr == NULL) {
>> ret = -ENOMEM;
>> dev_err(&pdev->dev, "ioremap failed.\n");
>> goto out_release;
>> }
>> +#ifdef CONFIG_OF
>> + if (np&& of_device_is_available(np)) {
>> + pd = sh_eth_parse_dt(&pdev->dev, ndev);
>> + if (pdev->dev.platform_data) {
>> + struct sh_eth_plat_data *tmp =
>> + pdev->dev.platform_data;
>> + pd->set_mdio_gate = tmp->set_mdio_gate;
>> + pd->needs_init = tmp->needs_init;
>> + }
>> + } else
>> +#endif
>> + pd = (struct sh_eth_plat_data *)(pdev->dev.platform_data);
>> +
>> + if (!pd) {
>> + dev_err(&pdev->dev, "no setup data defined\n");
>> + ret = -EINVAL;
>> + goto out_release;
>> + }
>> +
>> + SET_NETDEV_DEV(ndev,&pdev->dev);
>> +
>> + mdp->num_tx_ring = TX_RING_SIZE;
>> + mdp->num_rx_ring = RX_RING_SIZE;
>>
>> spin_lock_init(&mdp->lock);
>> mdp->pdev = pdev;
>> pm_runtime_enable(&pdev->dev);
>> pm_runtime_resume(&pdev->dev);
>>
>> - pd = (struct sh_eth_plat_data *)(pdev->dev.platform_data);
>> /* get PHY ID */
>> mdp->phy_id = pd->phy;
>> mdp->phy_interface = pd->phy_interface;
>> @@ -2412,6 +2509,8 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
>> mdp->no_ether_link = pd->no_ether_link;
>> mdp->ether_link_active_low = pd->ether_link_active_low;
>> mdp->reg_offset = sh_eth_get_register_offset(pd->register_type);
>> + /* read and set MAC address */
>> + read_mac_address(ndev, pd->mac_addr);
>>
>> /* set cpu data */
>> #if defined(SH_ETH_HAS_BOTH_MODULES)
>> @@ -2429,20 +2528,16 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
>> /* debug message level */
>> mdp->msg_enable = SH_ETH_DEF_MSG_ENABLE;
>>
>> - /* read and set MAC address */
>> - read_mac_address(ndev, pd->mac_addr);
>> -
>> /* ioremap the TSU registers */
>> if (mdp->cd->tsu) {
>> struct resource *rtsu;
>> rtsu = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> if (!rtsu) {
>> dev_err(&pdev->dev, "Not found TSU resource\n");
>> - ret = -ENODEV;
>> goto out_release;
>> }
>> mdp->tsu_addr = ioremap(rtsu->start,
>> - resource_size(rtsu));
>> + resource_size(rtsu));
>> mdp->port = devno % 2;
>> ndev->features = NETIF_F_HW_VLAN_FILTER;
>> }
>> @@ -2522,17 +2617,24 @@ static int sh_eth_runtime_nop(struct device *dev)
>> return 0;
>> }
>>
>> -static struct dev_pm_ops sh_eth_dev_pm_ops = {
>> +static const struct dev_pm_ops sh_eth_dev_pm_ops = {
>> .runtime_suspend = sh_eth_runtime_nop,
>> .runtime_resume = sh_eth_runtime_nop,
>> };
>>
>> +static struct of_device_id sh_eth_match[] = {
>> + { .compatible = "renesas,sh-eth",},
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, sh_eth_match);
>> +
>> static struct platform_driver sh_eth_driver = {
>> .probe = sh_eth_drv_probe,
>> .remove = sh_eth_drv_remove,
>> .driver = {
>> .name = CARDNAME,
>> .pm =&sh_eth_dev_pm_ops,
>> + .of_match_table = sh_eth_match,
>> },
>> };
>>
>> --
>> 1.7.10.4
>>
>> --
>> 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: [PATCHi v2] net: sh_eth: Add support of device tree probe
From: Nobuhiro Iwamatsu @ 2013-02-27 0:39 UTC (permalink / raw)
To: Simon Horman
Cc: Kuninori Morimoto, netdev, magnus.damm, devicetree-discuss, kda
In-Reply-To: <20130222184918.GC31345@verge.net.au>
Hi, Simon.
This does not apply yet to upstream.
I will send a patch without of_device_is_available function.
Best regards,
Nobuhiro
(2013/02/23 3:49), Simon Horman wrote:
> On Thu, Feb 14, 2013 at 11:56:57AM +0900, Nobuhiro Iwamatsu wrote:
>> (2013/02/14 10:24), Kuninori Morimoto wrote:
>>>
>>> Hi Iwamatsu-san
>>>
>>> Thank you for this patch.
>>>
>>> Small comment from me
>>>
>>>> +#ifdef CONFIG_OF
>>> (snip)
>>>> +sh_eth_parse_dt(struct device *dev, struct net_device *ndev)
>>>> +{
>>>> + int ret;
>>>> + struct device_node *np = dev->of_node;
>>>> + struct sh_eth_plat_data *pdata;
>>> ...
>>>> +#else
>>>> +static struct sh_eth_plat_data *
>>>> +sh_eth_parse_dt(struct device *dev, struct net_device *ndev)
>>>> +{
>>>> + return NULL;
>>>> +}
>>>> +#endif
>>>
>>> (snip)
>>>
>>>> static int sh_eth_drv_probe(struct platform_device *pdev)
>>>> {
>>> ...
>>>> +#ifdef CONFIG_OF
>>>> + if (np&& of_device_is_available(np)) {
>>>> + pd = sh_eth_parse_dt(&pdev->dev, ndev);
>>>> + if (pdev->dev.platform_data) {
>>>> + struct sh_eth_plat_data *tmp =
>>>> + pdev->dev.platform_data;
>>>> + pd->set_mdio_gate = tmp->set_mdio_gate;
>>>> + pd->needs_init = tmp->needs_init;
>>>> + }
>>>> + } else
>>>> +#endif
>>>
>>> sh_eth_parse_dt() was defined for both CONFIG_OF and !CONFIG_OF.
>>> But it is called only from CONFIG_OF ?
>>>
>>
>> Because of_device_is_available needs CONFIG_OF.
>> I already send a patch which add empty function of of_device_is_available.
>> If this was apply, this ifdef becomes without need.
>
> Hi Iwamatsu-san,
>
> could you let me know of the status of that patch?
> Has it been queued-up or merged? If so, where and when?
>
>
^ permalink raw reply
* Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks
From: Lai Jiangshan @ 2013-02-27 0:33 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: Michel Lespinasse, linux-doc, peterz, fweisbec, linux-kernel,
namhyung, mingo, linux-arch, linux, xiaoguangrong, wangyun,
paulmck, nikunj, linux-pm, rusty, rostedt, rjw, vincent.guittot,
tglx, linux-arm-kernel, netdev, oleg, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <512D0D67.9010609@linux.vnet.ibm.com>
On Wed, Feb 27, 2013 at 3:30 AM, Srivatsa S. Bhat
<srivatsa.bhat@linux.vnet.ibm.com> wrote:
> On 02/26/2013 09:55 PM, Lai Jiangshan wrote:
>> On Tue, Feb 26, 2013 at 10:22 PM, Srivatsa S. Bhat
>> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>>>
>>> Hi Lai,
>>>
>>> I'm really not convinced that piggy-backing on lglocks would help
>>> us in any way. But still, let me try to address some of the points
>>> you raised...
>>>
>>> On 02/26/2013 06:29 PM, Lai Jiangshan wrote:
>>>> On Tue, Feb 26, 2013 at 5:02 PM, Srivatsa S. Bhat
>>>> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>>>>> On 02/26/2013 05:47 AM, Lai Jiangshan wrote:
>>>>>> On Tue, Feb 26, 2013 at 3:26 AM, Srivatsa S. Bhat
>>>>>> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>>>>>>> Hi Lai,
>>>>>>>
>>>>>>> On 02/25/2013 09:23 PM, Lai Jiangshan wrote:
>>>>>>>> Hi, Srivatsa,
>>>>>>>>
>>>>>>>> The target of the whole patchset is nice for me.
>>>>>>>
>>>>>>> Cool! Thanks :-)
>>>>>>>
>>>>> [...]
>>>>>
>>>>> Unfortunately, I see quite a few issues with the code above. IIUC, the
>>>>> writer and the reader both increment the same counters. So how will the
>>>>> unlock() code in the reader path know when to unlock which of the locks?
>>>>
>>>> The same as your code, the reader(which nested in write C.S.) just dec
>>>> the counters.
>>>
>>> And that works fine in my case because the writer and the reader update
>>> _two_ _different_ counters.
>>
>> I can't find any magic in your code, they are the same counter.
>>
>> /*
>> * It is desirable to allow the writer to acquire the percpu-rwlock
>> * for read (if necessary), without deadlocking or getting complaints
>> * from lockdep. To achieve that, just increment the reader_refcnt of
>> * this CPU - that way, any attempt by the writer to acquire the
>> * percpu-rwlock for read, will get treated as a case of nested percpu
>> * reader, which is safe, from a locking perspective.
>> */
>> this_cpu_inc(pcpu_rwlock->rw_state->reader_refcnt);
>>
>
> Whoa! Hold on, were you really referring to _this_ increment when you said
> that, in your patch you would increment the refcnt at the writer? Then I guess
> there is a major disconnect in our conversations. (I had assumed that you were
> referring to the update of writer_signal, and were just trying to have a single
> refcnt instead of reader_refcnt and writer_signal).
https://github.com/laijs/linux/commit/53e5053d5b724bea7c538b11743d0f420d98f38d
Sorry the name "fallback_reader_refcnt" misled you.
>
> So, please let me clarify things a bit here. Forget about the above increment
> of reader_refcnt at the writer side. Its almost utterly insignificant for our
> current discussion. We can simply replace it with a check as shown below, at
> the reader side:
>
> void percpu_read_lock_irqsafe()
> {
> if (current == active_writer)
> return;
>
> /* Rest of the code */
> }
>
> Now, assuming that, in your patch, you were trying to use the per-cpu refcnt
> to allow the writer to safely take the reader path, you can simply get rid of
> that percpu-refcnt, as demonstrated above.
>
> So that would reduce your code to the following (after simplification):
>
> lg_rwlock_local_read_lock()
> {
> if (current == active_writer)
> return;
> if (arch_spin_trylock(per-cpu-spinlock))
> return;
> read_lock(global-rwlock);
> }
>
> Now, let us assume that hotplug is not happening, meaning, nobody is running
> the writer side code. Now let's see what happens at the reader side in your
> patch. As I mentioned earlier, the readers are _very_ frequent and can be in
> very hot paths. And they also happen to be nested quite often.
>
> So, a non-nested reader acquires the per-cpu spinlock. Every subsequent nested
> reader on that CPU has to acquire the global rwlock for read. Right there you
> have 2 significant performance issues -
> 1. Acquiring the (spin) lock is costly
> 2. Acquiring the global rwlock causes cacheline bouncing, which hurts
> performance.
>
> And why do we care so much about performance here? Because, the existing
> kernel does an efficient preempt_disable() here - which is an optimized
> per-cpu counter increment. Replacing that with such heavy primitives on the
> reader side can be very bad.
>
> Now, how does my patchset tackle this? My scheme just requires an increment
> of a per-cpu refcnt (reader_refcnt) and memory barrier. Which is acceptable
> from a performance-perspective, because IMHO its not horrendously worse than
> a preempt_disable().
>
>>
>>> If both of them update the same counter, there
>>> will be a semantic clash - an increment of the counter can either mean that
>>> a new writer became active, or it can also indicate a nested reader. A decrement
>>> can also similarly have 2 meanings. And thus it will be difficult to decide
>>> the right action to take, based on the value of the counter.
>>>
>>>>
>>>>> (The counter-dropping-to-zero logic is not safe, since it can be updated
>>>>> due to different reasons). And now that I look at it again, in the absence
>>>>> of the writer, the reader is allowed to be recursive at the heavy cost of
>>>>> taking the global rwlock for read, every 2nd time you nest (because the
>>>>> spinlock is non-recursive).
>>>>
>>>> (I did not understand your comments of this part)
>>>> nested reader is considered seldom.
>>>
>>> No, nested readers can be _quite_ frequent. Because, potentially all users
>>> of preempt_disable() are readers - and its well-known how frequently we
>>> nest preempt_disable(). As a simple example, any atomic reader who calls
>>> smp_call_function() will become a nested reader, because smp_call_function()
>>> itself is a reader. So reader nesting is expected to be quite frequent.
>>>
>>>> But if N(>=2) nested readers happen,
>>>> the overhead is:
>>>> 1 spin_try_lock() + 1 read_lock() + (N-1) __this_cpu_inc()
>>>>
>>>
>>> In my patch, its just this_cpu_inc(). Note that these are _very_ hot paths.
>>> So every bit of optimization that you can add is worthwhile.
>>>
>>> And your read_lock() is a _global_ lock - thus, it can lead to a lot of
>>> cache-line bouncing. That's *exactly* why I have used per-cpu refcounts in
>>> my synchronization scheme, to avoid taking the global rwlock as much as possible.
>>>
>>> Another important point to note is that, the overhead we are talking about
>>> here, exists even when _not_ performing hotplug. And its the replacement to
>>> the super-fast preempt_disable(). So its extremely important to consciously
>>> minimize this overhead - else we'll end up slowing down the system significantly.
>>>
>>
>> All I was considered is "nested reader is seldom", so I always
>> fallback to rwlock when nested.
>> If you like, I can add 6 lines of code, the overhead is
>> 1 spin_try_lock()(fast path) + N __this_cpu_inc()
>>
>
> I'm assuming that calculation is no longer valid, considering that
> we just discussed how the per-cpu refcnt that you were using is quite
> unnecessary and can be removed.
>
> IIUC, the overhead with your code, as per above discussion would be:
> 1 spin_try_lock() [non-nested] + N read_lock(global_rwlock).
https://github.com/laijs/linux/commit/46334544bb7961550b7065e015da76f6dab21f16
Again, I'm so sorry the name "fallback_reader_refcnt" misled you.
>
> Note that I'm referring to the scenario when hotplug is _not_ happening
> (ie., nobody is running writer side code).
>
>> The overhead of your code is
>> 2 smp_mb() + N __this_cpu_inc()
>>
>
> Right. And as you can see, this is much much better than the overhead
> shown above.
I will write a test to compare it to "1 spin_try_lock()(fast path) +
N __this_cpu_inc()"
>
>> I don't see how much different.
>>
>>>>> Also, this lg_rwlock implementation uses 3
>>>>> different data-structures - a per-cpu spinlock, a global rwlock and
>>>>> a per-cpu refcnt, and its not immediately apparent why you need those many
>>>>> or even those many varieties.
>>>>
>>>> data-structures is the same as yours.
>>>> fallback_reader_refcnt <--> reader_refcnt
>>>
>>> This has semantic problems, as noted above.
>>>
>>>> per-cpu spinlock <--> write_signal
>>>
>>> Acquire/release of (spin) lock is costlier than inc/dec of a counter, IIUC.
>>>
>>>> fallback_rwlock <---> global_rwlock
>>>>
>>>>> Also I see that this doesn't handle the
>>>>> case of interrupt-handlers also being readers.
>>>>
>>>> handled. nested reader will see the ref or take the fallback_rwlock
>>>>
>>
>> Sorry, _reentrance_ read_lock() will see the ref or take the fallback_rwlock
>>
>>>
>>> I'm not referring to simple nested readers here, but interrupt handlers who
>>> can act as readers. For starters, the arch_spin_trylock() is not safe when
>>> interrupt handlers can also run the same code, right? You'll need to save
>>> and restore interrupts at critical points in the code. Also, the __foo()
>>> variants used to read/update the counters are not interrupt-safe.
>>
>> I must missed something.
>>
>> Could you elaborate more why arch_spin_trylock() is not safe when
>> interrupt handlers can also run the same code?
>>
>> Could you elaborate more why __this_cpu_op variants is not
>> interrupt-safe since they are always called paired.
>>
>
> Take a look at include/linux/percpu.h. You'll note that __this_cpu_*
> operations map to __this_cpu_generic_to_op(), which doesn't disable interrupts
> while doing the update. Hence you can get inconsistent results if an interrupt
> hits the CPU at that time and the interrupt handler tries to do the same thing.
> In contrast, if you use this_cpu_inc() for example, interrupts are explicitly
> disabled during the update and hence you won't get inconsistent results.
xx_lock()/xx_unlock() are must called paired, if interrupts happens
the value of the data is recovered after the interrupts return.
the same reason, preempt_disable() itself it is not irqsafe,
but preempt_disable()/preempt_enable() are called paired, so they are
all irqsafe.
>
>>
>>> And,
>>> the unlock() code in the reader path is again going to be confused about
>>> what to do when interrupt handlers interrupt regular readers, due to the
>>> messed up refcount.
>>
>> I still can't understand.
>>
>
> The primary reason _I_ was using the refcnt vs the reason _you_ were using the
> refcnt, appears to be very different. Maybe that's why the above statement
> didn't make sense. In your case, IIUC, you can simply get rid of the refcnt
> and replace it with the simple check I mentioned above. Whereas, I use
> refcnts to keep the reader-side synchronization fast (and for reader-writer
> communication).
>
>
> Regards,
> Srivatsa S. Bhat
>
^ permalink raw reply
* Re: [E1000-devel] [PATCH RESEND 3/3] e1000e: fix accessing to suspended device
From: Ben Hutchings @ 2013-02-26 23:35 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: Waskiewicz Jr, Peter P, linux-kernel, netdev, e1000-devel,
Rafael J. Wysocki, Bruce Allan
In-Reply-To: <512C8889.80107@openvz.org>
On Tue, 2013-02-26 at 14:03 +0400, Konstantin Khlebnikov wrote:
> Waskiewicz Jr, Peter P wrote:
> > On 2/24/2013 9:19 PM, Konstantin Khlebnikov wrote:
> >> This patch fixes some annoying messages like 'Error reading PHY register' and
> >> 'Hardware Erorr' and saves several seconds on reboot.
> >
> > Any networking-related patches should also include netdev@vger.kernel.org.
>
> Yeah, I forgot about this, since I came here from PCI-bus side, not from the network =)
>
> >
> > I'm also a bit confused how the changes below match the patch description.
> > Elaborating a bit more how the changes suppress the messages might be a good thing.
>
> Patch eliminates reason of these errors -- now driver will wake up
> the device before accessing to its registers.
[...]
But e1000e calls netif_device_detach() when entering run-time suspend.
That currently prevents any ethtool operations from running until it
resumes.
(This is definitely a misfeature of either the ethtool core or e1000e.
It is absolutely ridiculous that I can't check the PHY settings of an
e1000e - or even driver information! - when the link is down.)
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH v4] net: sh_eth: Add support of device tree probe
From: Nobuhiro Iwamatsu @ 2013-02-26 23:34 UTC (permalink / raw)
To: Denis Kirjanov
Cc: Nobuhiro Iwamatsu, netdev, horms+renesas, magnus.damm,
devicetree-discuss, kuninori.morimoto.gx
In-Reply-To: <CAOJe8K0BdhLUjkjpjdJH-tjBHbXJHdV-mWAkMFJ96zahhAfwOg@mail.gmail.com>
(2013/02/16 0:02), Denis Kirjanov wrote:
> On 2/15/13, Nobuhiro Iwamatsu<iwamatsu@nigauri.org> wrote:
>> From: Nobuhiro Iwamatsu<nobuhiro.iwamatsu.yj@renesas.com>
>>
>> This adds support of device tree probe for Renesas sh-ether driver.
>>
>> Signed-off-by: Nobuhiro Iwamatsu<nobuhiro.iwamatsu.yj@renesas.com>
>>
>> V4: - Remove empty sh_eth_parse_dt().
>> V3: - Removed sentnece of "needs-init" from document.
>> V2: - Removed ether_setup().
>> - Fixed typo from "sh-etn" to "sh-eth".
>> - Removed "needs-init" and sh-eth,endian from definition of DT.
>> - Changed "sh-eth,edmac-endian" instead of "sh-eth,edmac-big-endain"
>> in definition of DT.
>> ---
>> Documentation/devicetree/bindings/net/sh_ether.txt | 39 +++++
>> drivers/net/ethernet/renesas/sh_eth.c | 150
>> ++++++++++++++++----
>> 2 files changed, 165 insertions(+), 24 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/net/sh_ether.txt
>>
<snip>
>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c
>> b/drivers/net/ethernet/renesas/sh_eth.c
>> index 3d70586..b41249a 100644
>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>> @@ -1,7 +1,7 @@
>> /*
>> * SuperH Ethernet device driver
>> *
>> - * Copyright (C) 2006-2012 Nobuhiro Iwamatsu
>> + * Copyright (C) 2006-2013 Nobuhiro Iwamatsu
>> * Copyright (C) 2008-2012 Renesas Solutions Corp.
>> *
>> * This program is free software; you can redistribute it and/or modify
>> it
>> @@ -31,6 +31,12 @@
>> #include<linux/platform_device.h>
>> #include<linux/mdio-bitbang.h>
>> #include<linux/netdevice.h>
>> +#include<linux/of.h>
>> +#include<linux/of_device.h>
>> +#include<linux/of_platform.h>
>> +#include<linux/of_address.h>
>> +#include<linux/of_irq.h>
>> +#include<linux/of_net.h>
>> #include<linux/phy.h>
>> #include<linux/cache.h>
>> #include<linux/io.h>
>> @@ -2347,26 +2353,103 @@ static const struct net_device_ops
>> sh_eth_netdev_ops = {
>> .ndo_change_mtu = eth_change_mtu,
>> };
>>
>> +#ifdef CONFIG_OF
>> +
>> +static int
>> +sh_eth_of_get_register_type(struct device_node *np)
>> +{
>> + const char *of_str;
>> + int ret = of_property_read_string(np, "sh-eth,register-type",&of_str);
>> + if (ret)
>> + return ret;
>> +
>> + if (of_str&& !strcmp(of_str, "gigabit"))
>> + return SH_ETH_REG_GIGABIT;
>> +
>> + else if (of_str&& !strcmp(of_str, "fast-sh4"))
>> + return SH_ETH_REG_FAST_SH4;
>> + else if (of_str&& !strcmp(of_str, "fast-sh3-sh2"))
>> + return SH_ETH_REG_FAST_SH3_SH2;
>> + else
>> + return -EINVAL;
>> +}
>> +
>> +static struct sh_eth_plat_data *
>> +sh_eth_parse_dt(struct device *dev, struct net_device *ndev)
>> +{
>> + int ret;
>> + struct device_node *np = dev->of_node;
>> + struct sh_eth_plat_data *pdata;
>> +
>> + pdata = devm_kzalloc(dev, sizeof(struct sh_eth_plat_data),
>> + GFP_KERNEL);
>> + if (!pdata) {
>> + dev_err(dev, "%s: failed to allocate config data\n", __func__);
>> + return NULL;
>> + }
>> +
>> + pdata->phy_interface = of_get_phy_mode(np);
>> +
>> + of_property_read_u32(np, "sh-eth,phy-id",&pdata->phy);
>> +
>> + if (of_find_property(np, "sh-eth,edmac-big-endian", NULL))
>> + pdata->edmac_endian = EDMAC_BIG_ENDIAN;
>> + else
>> + pdata->edmac_endian = EDMAC_LITTLE_ENDIAN;
>> +
>> + if (of_find_property(np, "sh-eth,no-ether-link", NULL))
>> + pdata->no_ether_link = 1;
>> + else
>> + pdata->no_ether_link = 0;
>> +
>> + if (of_find_property(np, "sh-eth,ether-link-active-low", NULL))
>> + pdata->ether_link_active_low = 1;
>> + else
>> + pdata->ether_link_active_low = 0;
>> +
>> + ret = sh_eth_of_get_register_type(np);
>> + if (ret< 0)
>> + goto error;
>> + pdata->register_type = ret;
>> +
>> +#ifdef CONFIG_OF_NET
>> + if (!is_valid_ether_addr(ndev->dev_addr)) {
>> + const char *macaddr = of_get_mac_address(np);
>> + if (macaddr)
>> + memcpy(pdata->mac_addr, macaddr, ETH_ALEN);
>> + }
>> +#endif
>> +
>> + return pdata;
>> +
>> +error:
>> + devm_kfree(dev, pdata);
>> + return NULL;
>> +}
>> +#endif
>> +
>> static int sh_eth_drv_probe(struct platform_device *pdev)
>> {
>> - int ret, devno = 0;
>> + int ret = 0, devno = 0;
>> struct resource *res;
>> struct net_device *ndev = NULL;
>> struct sh_eth_private *mdp = NULL;
>> struct sh_eth_plat_data *pd;
>> + struct device_node *np = pdev->dev.of_node;
>> +
>> + ndev = alloc_etherdev(sizeof(struct sh_eth_private));
>> + if (!ndev) {
>> + ret = -ENOMEM;
>> + goto out;
>> + }
>>
>> + mdp = netdev_priv(ndev);
>> /* get base addr */
>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> if (unlikely(res == NULL)) {
>> dev_err(&pdev->dev, "invalid resource\n");
>> ret = -EINVAL;
>> - goto out;
>> - }
>> -
>> - ndev = alloc_etherdev(sizeof(struct sh_eth_private));
>> - if (!ndev) {
>> - ret = -ENOMEM;
>> - goto out;
>> + goto out_release;
>> }
>>
>> /* The sh Ether-specific entries in the device structure. */
>> @@ -2383,27 +2466,41 @@ static int sh_eth_drv_probe(struct platform_device
>> *pdev)
>> }
>> ndev->irq = ret;
>>
>> - SET_NETDEV_DEV(ndev,&pdev->dev);
>> -
>> - /* Fill in the fields of the device structure with ethernet values. */
>> - ether_setup(ndev);
>> -
>> - mdp = netdev_priv(ndev);
>> - mdp->num_tx_ring = TX_RING_SIZE;
>> - mdp->num_rx_ring = RX_RING_SIZE;
>> mdp->addr = ioremap(res->start, resource_size(res));
>> if (mdp->addr == NULL) {
>> ret = -ENOMEM;
>> dev_err(&pdev->dev, "ioremap failed.\n");
>> goto out_release;
>> }
>> +#ifdef CONFIG_OF
>> + if (np&& of_device_is_available(np)) {
>> + pd = sh_eth_parse_dt(&pdev->dev, ndev);
>> + if (pdev->dev.platform_data) {
>> + struct sh_eth_plat_data *tmp =
>> + pdev->dev.platform_data;
>> + pd->set_mdio_gate = tmp->set_mdio_gate;
>> + pd->needs_init = tmp->needs_init;
>> + }
>> + } else
>> +#endif
>> + pd = (struct sh_eth_plat_data *)(pdev->dev.platform_data);
>> +
>> + if (!pd) {
>> + dev_err(&pdev->dev, "no setup data defined\n");
>> + ret = -EINVAL;
>> + goto out_release;
>> + }
>> +
>> + SET_NETDEV_DEV(ndev,&pdev->dev);
>> +
>> + mdp->num_tx_ring = TX_RING_SIZE;
>> + mdp->num_rx_ring = RX_RING_SIZE;
>>
>> spin_lock_init(&mdp->lock);
>> mdp->pdev = pdev;
>> pm_runtime_enable(&pdev->dev);
>> pm_runtime_resume(&pdev->dev);
>>
>> - pd = (struct sh_eth_plat_data *)(pdev->dev.platform_data);
>> /* get PHY ID */
>> mdp->phy_id = pd->phy;
>> mdp->phy_interface = pd->phy_interface;
>> @@ -2412,6 +2509,8 @@ static int sh_eth_drv_probe(struct platform_device
>> *pdev)
>> mdp->no_ether_link = pd->no_ether_link;
>> mdp->ether_link_active_low = pd->ether_link_active_low;
>> mdp->reg_offset = sh_eth_get_register_offset(pd->register_type);
>> + /* read and set MAC address */
>> + read_mac_address(ndev, pd->mac_addr);
>>
>> /* set cpu data */
>> #if defined(SH_ETH_HAS_BOTH_MODULES)
>> @@ -2429,20 +2528,16 @@ static int sh_eth_drv_probe(struct platform_device
>> *pdev)
>> /* debug message level */
>> mdp->msg_enable = SH_ETH_DEF_MSG_ENABLE;
>>
>> - /* read and set MAC address */
>> - read_mac_address(ndev, pd->mac_addr);
>> -
>> /* ioremap the TSU registers */
>> if (mdp->cd->tsu) {
>> struct resource *rtsu;
>> rtsu = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> if (!rtsu) {
>> dev_err(&pdev->dev, "Not found TSU resource\n");
>> - ret = -ENODEV;
>> goto out_release;
>> }
>> mdp->tsu_addr = ioremap(rtsu->start,
>> - resource_size(rtsu));
>> + resource_size(rtsu));
>> mdp->port = devno % 2;
>> ndev->features = NETIF_F_HW_VLAN_FILTER;
>> }
>> @@ -2522,17 +2617,24 @@ static int sh_eth_runtime_nop(struct device *dev)
>> return 0;
>> }
>>
>> -static struct dev_pm_ops sh_eth_dev_pm_ops = {
> CONFIG_PM?
Yes, I will add this.
>
>> +static const struct dev_pm_ops sh_eth_dev_pm_ops = {
>> .runtime_suspend = sh_eth_runtime_nop,
>> .runtime_resume = sh_eth_runtime_nop,
>> };
>>
>> +static struct of_device_id sh_eth_match[] = {
>
> You have missed the const specifier here
OK, I will fix.
>
>> + { .compatible = "renesas,sh-eth",},
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, sh_eth_match);
>> +
>
> if CONFIG_OF is not defined it's not needed.
>
OK, I will add CONFIG_OF and update platform_driver.
>> static struct platform_driver sh_eth_driver = {
>> .probe = sh_eth_drv_probe,
>> .remove = sh_eth_drv_remove,
>> .driver = {
>> .name = CARDNAME,
>> .pm =&sh_eth_dev_pm_ops,
>> + .of_match_table = sh_eth_match,
>> },
>> };
Thank you for your review.
Best regards,
Nobuhiro
^ permalink raw reply
* Powered By Google.
From: Google Incorporation @ 2013-02-26 21:15 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 225 bytes --]
Dear Google User,
You have been selected as a winner for using Google services. Find attached email with more details.
Congratulations,
Matt Brittin.
CEO Google UK.
©2013 Google Corporation
[-- Attachment #2: Google UK.pdf --]
[-- Type: application/pdf, Size: 864834 bytes --]
^ permalink raw reply
* Re: LSM stacking and the network access controls
From: Casey Schaufler @ 2013-02-26 23:12 UTC (permalink / raw)
To: Paul Moore
Cc: netdev, linux-security-module, selinux, Andy King, Gerd Hoffmann,
Eric Paris, Casey Schaufler
In-Reply-To: <1495594.zrMMDNjium@sifl>
On 2/26/2013 1:21 PM, Paul Moore wrote:
> On Monday, February 25, 2013 03:06:14 PM Casey Schaufler wrote:
>> On 2/25/2013 1:05 PM, Paul Moore wrote:
>>> On Monday, February 25, 2013 10:02:42 AM Casey Schaufler wrote:
>>>> On 2/25/2013 8:55 AM, Paul Moore wrote:
>>>>> [NOTE/WARNING: we're veering away from the VSOCK discussion and towards
>>>>> a LSM stacking discussion; see my response to Gerd if you want to stay
>>>>> on topic.]
>>>>>
>>>>> On Saturday, February 23, 2013 03:43:23 PM Casey Schaufler wrote:
>>>>>> On 2/22/2013 4:45 PM, Paul Moore wrote:
>>>>>>> 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.
>>>>>> I suppose. My point, which you may refute if it is incorrect,
>>>>>> is that there are common, legitimate SELinux configurations which
>>>>>> eschew Netlabel in favor of XFRM.
>>>>> There are common, legitimate use cases which use exclusively NetLabel,
>>>>> exclusively labeled IPsec, and both. A LSM stacking design that forces
>>>>> SELinux to only operate with XFRM (labeled IPsec) is wrong. If you are
>>>>> giving admins the ability to selectively stack LSMs you should also
>>>>> allow them to selectively pick which non-shareable functionality they
>>>>> assign to each LSM.
>>>> That is the approach I'm taking. The kernel configuration
>>>> will specify which LSM gets Netlabel and which gets XFRM.
>>> Okay, it wasn't obvious to me in your previous emails that this was the
>>> case. Also, just so I'm clear, you configure the stacking and the stacked
>>> network access controls at the same time, yes? I want to avoid the
>>> situation where the stacked network access controls are determined at
>>> build time and the LSM stacking order/configuration is determined at
>>> boot.
>> The set of LSMs, the order they are invoked, which LSM
>> uses /proc/.../attr/current and which LSM uses Netlabel,
>> XFRM and secmark are all determined by Kconfig. You can
>> specify a limited set of LSMs using security= at boot,
>> but not the networking configuration.
> That's unfortunate. I'm _really_ not in favor of that, I would much rather
> see the non-shared LSM functionality assigned at the same time as the stacking
> order. I'm not sure I'd NACK the current approach, or even if anyone would
> care that I did, but that is how I'm currently leaning with this split (build
> vs runtime) selection.
I'm not against that approach. How would you see it working?
The distro compiles in all the LSMs.
They specify that SELinux gets xfrm and secmark.
They specify the Smack gets Netlabel.
They tell (the new and improved) AppArmor to eschew networking.
They specify a boot order of "selinux,smack,apparmor,yama"
(They left off tomoyo for tax purposes).
On the boot line, the user types "security=apparmor".
What should happen?
>
>> Tetsuo is lobbying for loadable security modules. I am
>> doing my best to leave everything in a state where some
>> soul braver than I might pick that up after I'm done.
>> I do not have any intention of supporting on the fly
>> or heuristically determined networking.
> Well, if that is the case I think the first-come-first-served approach to the
> non-shared functionality probably makes the most sense. I understand why it
> might not be ideal in ever situation but it is predictable.
Would you have the same opinion if SELinux were last,
instead of first? SELinux is the most feature rich of
the LSMs and no other LSM would ever get networking in
the presence of SELinux.
>>>> I'm trying not to make too many architectural changes to the
>>>> code around the LSM mechanism itself. I don't see that as cost
>>>> effective or likely to be popular. If the implication of that is
>>>> that there are certain configurations that are unsupportable but
>>>> that have plausible alternatives I think it will do for phase I.
>>> That statement is vague enough that I can't really say yes or no to it,
>>> but I will stick by my previous comments about the network access
>>> controls.
>> Ah. I want to create a useful system. I want to create it
>> reasonably short order. I am willing to forgo supporting
>> some configuration options to reduce the project time. In
>> particular, I hope to avoid mucking up other people's code
>> as that is a sure fire way to bog a project down.
> Usefulness is paramount of course
Agreed
> and quickness is nice,
If it drags on too long the boss gets cranky. And "labelled" NFSv4.2
gets done. And the list macros get reworked. And ....
> but I do think it takes a back seat to "the right solution".
If I had two years and didn't have to worry about anyone else's
changes and weren't beholden to retaining some of the past's more
interesting design choices I'd be more concerned about "right" than
"good".
> We're all going to have to live with this f-o-r-e-v-e-r,
Assuming it gets adopted. It will have to be good enough for that.
There is no guarantee on that.
> or at least what will surely seem like it, so I'd
> much rather we take the time to make sure we beat out as much ugly as we can
> before it is merged.
I'm with you there. I just hope the ugly hasn't been lifting weights.
>>>>>> Options I have considered include:
>>>>>> - Netlabel support for discriminating LSM use by host,
>>>>>>
>>>>>> just as it currently allows for unlabeled hosts.
>>>>> Hmm, interesting ... not sure what I think of this.
>>>> It breaks down on 127.0.0.1. Otherwise is looks pretty easy to do.
>>> When I said I wasn't sure what I thought of this, it was more along the
>>> lines of I'm not sure if it makes sense, not that it would be difficult
>>> to do.
>> It makes sense if you're in a network environment with heterogeneous
>> legacy multilevel secure systems and you want some of those machines
>> to talk to SELinux controlled processes and others to talk to Smack
>> controlled processes. That is unlikely to be an industry dominating
>> scenario.
> Well, you can talk MLS labeled networking to both SELinux and Smack, and you
> can talk MLS labeled networking between SELinux and Smack already (hip, hip,
> hooray for commonly accepted standards!)
And Paul gets a pat on the back! The crowd goes wild!
> so I'm not sure you need this.
> Although I suppose it might simplify the configuration in some cases or help
> if you're trying to migrate from one LSM to another.
It's a nutty scenario. It would allow Smack and SELinux to divide the
hosts amongst them, and if SELinux wasn't going to use Netlabel for
on the box (127.0.0.1) communications anyway, Smack could use that
and be very happy.
^ permalink raw reply
* Re: [PATCH 0/4] sctp: fix association hangs due to reassembly/ordering logic
From: David Miller @ 2013-02-26 22:37 UTC (permalink / raw)
To: lee.roberts; +Cc: netdev, vyasevich, sri, nhorman
In-Reply-To: <1361889376-22171-1-git-send-email-lee.roberts@hp.com>
From: "Lee A. Roberts" <lee.roberts@hp.com>
Date: Tue, 26 Feb 2013 07:36:12 -0700
> This series of patches resolves several SCTP association hangs observed during
> SCTP stress testing. Observable symptoms include communications hangs with
> data being held in the association reassembly and/or lobby (ordering) queues.
> Close examination of reassembly/ordering queues may show either duplicated
> or missing packets.
>
> Lee A. Roberts (4):
> sctp: fix association hangs due to off-by-one errors in
> sctp_tsnmap_grow()
> sctp: fix association hangs due to reneging packets below the
> cumulative TSN ACK point
> sctp: fix association hangs due to errors when reneging events from
> the ordering queue
> sctp: fix association hangs due to partial delivery errors
Vlad, Sridhar, and Neil, please review.
^ permalink raw reply
* Re: [PATCH BUGFIX] pkt_sched: fix little service anomalies and possible crashes of qfq+
From: David Miller @ 2013-02-26 22:37 UTC (permalink / raw)
To: paolo.valente; +Cc: jhs, shemminger, netdev, linux-kernel, fchecconi, rizzo
In-Reply-To: <1361898166-18295-1-git-send-email-paolo.valente@unimore.it>
From: Paolo valente <paolo.valente@unimore.it>
Date: Tue, 26 Feb 2013 18:02:46 +0100
> The portions of the code interested by each fix are small and do not
> overlap with each other, so I decided to provide just one patch
> (I hope that this was the right choice).
Please split this up into 6 patches, each with an appropriately
verbose analysis and explanation of each bug being fixed, thanks.
^ permalink raw reply
* Re: [PATCH] isdn: hisax: add missing usb_free_urb
From: David Miller @ 2013-02-26 22:35 UTC (permalink / raw)
To: makienko; +Cc: isdn, netdev, ldv-project
In-Reply-To: <1361867210-13309-1-git-send-email-makienko@ispras.ru>
From: Marina Makienko <makienko@ispras.ru>
Date: Tue, 26 Feb 2013 12:26:50 +0400
> Add missing usb_free_urb() on failure path in st5481_setup_usb().
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Marina Makienko <makienko@ispras.ru>
Applied, thanks.
^ permalink raw reply
* Re: [patch net] bond: check if slave count is 0 in case when deciding to take slave's mac
From: David Miller @ 2013-02-26 22:32 UTC (permalink / raw)
To: jiri; +Cc: netdev, fubar, andy, gregory.v.rose
In-Reply-To: <1361867175-4033-1-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 26 Feb 2013 09:26:15 +0100
> in bond_enslave(), check slave_cnt before actually using slave address.
>
> introduced by:
> commit 409cc1f8a41 (bond: have random dev address by default instead of zeroes)
>
> Reported-by: Greg Rose <gregory.v.rose@intel.com>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Applied, thanks Jiri.
^ permalink raw reply
* Re: [PATCH] usb/net/asix_devices: Add USBNET HG20F9 ethernet dongle
From: David Miller @ 2013-02-26 22:29 UTC (permalink / raw)
To: gdt; +Cc: gregkh, linux-usb, netdev, linux-kernel
In-Reply-To: <20130226.172856.235525833921140241.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Tue, 26 Feb 2013 17:28:56 -0500 (EST)
> Applied, thanks Glen.
Actually, I had to revert, this doesn't even compile against
current sources:
CC [M] drivers/net/usb/asix_devices.o
drivers/net/usb/asix_devices.c:941:14: error: ‘asix_rx_fixup’ undeclared here (not in a function)
make[1]: *** [drivers/net/usb/asix_devices.o] Error 1
make: *** [drivers/net/usb/asix_devices.o] Error 2
^ permalink raw reply
* Re: [PATCH] usb/net/asix_devices: Add USBNET HG20F9 ethernet dongle
From: David Miller @ 2013-02-26 22:28 UTC (permalink / raw)
To: gdt; +Cc: gregkh, linux-usb, netdev, linux-kernel
In-Reply-To: <1361852232.23197.4.camel@andromache.adelaide.aarnet.edu.au>
From: Glen Turner <gdt@gdt.id.au>
Date: Tue, 26 Feb 2013 14:47:12 +1030
> This USB ethernet adapter was purchased in anodyne packaging
> marked "USB2.0 to LAN" from the computer store adjacent to
> linux.conf.au 2013 in Canberra (Australia). A web search
> shows other recent purchasers in Lancaster (UK) and Seattle
> (USA). Just like an emergent virus, our age of e-commerce and
> airmail allows underdocumented hardware to spread around the
> world instantly using the vector of ridiculously low prices.
>
> Paige Thompson, infected via eBay, discovered that the HG20F9
> is a copy of the Asix 88772B; many viruses copy the RNA of
> other viruses. See Paige's work at
> <https://github.com/paigeadele/HG20F9>.
> This patch uses her discovery to update the restructured Asix
> driver in the current kernel.
>
> The spread of viruses is often accompanied by rumours. It is
> rumoured that the HG20F9 has extensions to to provide gigabit
> ethernet. This patch does not chase that chimera.
>
> Just as some viruses inhabit seemingly-healthy cells, the
> HG20F9 uses the Vendor ID 0x066b assigned to Linksys Inc.
> For the present there is no clash of Product ID 0x20f9.
>
> Signed-off-by: Glen Turner <gdt@gdt.id.au>
Applied, thanks Glen.
^ permalink raw reply
* Re: [PATCH] drivers: net: ethernet: cpsw: consider number of slaves in interation
From: David Miller @ 2013-02-26 22:26 UTC (permalink / raw)
To: mugunthanvnm; +Cc: zonque, netdev
In-Reply-To: <512CCCF2.7030306@ti.com>
From: Mugunthan V N <mugunthanvnm@ti.com>
Date: Tue, 26 Feb 2013 20:25:46 +0530
> On 2/26/2013 7:36 PM, Daniel Mack wrote:
>> Make cpsw_add_default_vlan() look at the actual number of slaves for
>> its
>> iteration, so boards with less than 2 slaves don't ooops at boot.
>>
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>> Cc: Mugunthan V N <mugunthanvnm@ti.com>
>> Cc: David S. Miller <davem@davemloft.net>
> Looks ok for me
>
> Acked-by: Mugunthan V N <mugunthanvnm@ti.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 0/2] netfilter fixes for net
From: David Miller @ 2013-02-26 22:24 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1361886320-9492-1-git-send-email-pablo@netfilter.org>
From: pablo@netfilter.org
Date: Tue, 26 Feb 2013 14:45:18 +0100
> From: Pablo Neira Ayuso <pablo@netfilter.org>
>
> Hi David,
>
> The following patchset contains two bugfixes for netfilter/ipset via
> Jozsef Kadlecsik, they are:
>
> * Fix timeout corruption if sets are resized, by Josh Hunt.
>
> * Fix bogus error report if the flag nomatch is set, from Jozsef.
>
> You can pull these changes from:
>
> git://1984.lsi.us.es/nf master
Pulled, thanks Pablo.
^ permalink raw reply
* Re: Pull request: sfc 2013-02-26
From: David Miller @ 2013-02-26 22:23 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers, scrum-linux
In-Reply-To: <1361905763.2996.9.camel@bwh-desktop.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue, 26 Feb 2013 19:09:23 +0000
> The following changes since commit eb970ff07c15f13eb474f643fd165ebe3e4e24b2:
>
> usbnet: smsc95xx: rename FEATURE_AUTOSUSPEND (2013-02-25 15:49:52 -0500)
>
> are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc.git sfc-3.9
>
> Some fixes that should go into 3.9:
>
> 1. Fix packet corruption when using non-coherent RX DMA buffers.
> 2. Fix occasional watchdog misfiring when changing MTU or ring size.
>
> These are longstanding bugs and should be fixed in stable as well, but
> I'd like to review other recent fixes first and send a separate request
> for stable inclusion.
Pulled, thanks Ben.
^ permalink raw reply
* Re: [PATCH] cirrus: Remove redundant NULL check before kfree
From: David Miller @ 2013-02-26 22:20 UTC (permalink / raw)
To: syamsidhardh; +Cc: netdev, hsweeten
In-Reply-To: <1361908173-3157-1-git-send-email-s.syam@samsung.com>
All of these NULL check removals are not bug fixes, and therefore
not appropriate for the 'net' tree. They should be targetted at
'net-next' which is closed at this time.
Please resubmit your patches when I announce on netdev that 'net-next'
is open for submissions once more.
Thanks.
^ permalink raw reply
* Re: [RFC PATCH] core: Add ioctls to control device unicast hw addresses
From: David Miller @ 2013-02-26 22:15 UTC (permalink / raw)
To: john.r.fastabend; +Cc: vyasevic, netdev
In-Reply-To: <512D300F.20405@intel.com>
From: John Fastabend <john.r.fastabend@intel.com>
Date: Tue, 26 Feb 2013 13:58:39 -0800
> [...]
>
>>>>
>>>>
>>>> [RFC PATCH] rtnetlink: Add support for adding/removing additional hw
>>>> addresses.
>>>>
>>>> Add an ability to add and remove HW addresses to the device
>>>> unicast and multicast address lists. Right now, we only have
>>>> an ioctl() to manage the multicast addresses and there is no
>>>> way the manage the unicast list.
>>>>
>>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>>>
>>> This is a step in the right direction, and you're right that there is
>>> a difficulty in detecting whether support exists or not.
>>>
>>> I am so surprised that we've have ->set_rx_mode() support for multiple
>>> unicast MAC addresses in so many drivers all this time, yet no way
>>> outside of FDB to make use of it at all.
>>
>> And even that is not always available. In most drivers it requires
>> module parameters or other explicit configuration steps. Meanwhile
>> set_rx_mode() doesn't seem to depend on any of those and just does the
>> right thing.
>>
>> For what I was trying to do ioctl() was a really easy way out for both
>> kernel and user space implementation, so I gave is shot.
>>
>> -vlad
>>
>
> Don't we already support this with
The whole point is that these multiple-unicast-address configuration
facilities are inaccessible without FDB, and there is no reason
whatsoever for that.
^ permalink raw reply
* Re: [RFC PATCH] core: Add ioctls to control device unicast hw addresses
From: John Fastabend @ 2013-02-26 21:58 UTC (permalink / raw)
To: vyasevic; +Cc: David Miller, netdev
In-Reply-To: <512D1A01.9020301@redhat.com>
[...]
>>>
>>>
>>> [RFC PATCH] rtnetlink: Add support for adding/removing additional hw
>>> addresses.
>>>
>>> Add an ability to add and remove HW addresses to the device
>>> unicast and multicast address lists. Right now, we only have
>>> an ioctl() to manage the multicast addresses and there is no
>>> way the manage the unicast list.
>>>
>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>>
>> This is a step in the right direction, and you're right that there is
>> a difficulty in detecting whether support exists or not.
>>
>> I am so surprised that we've have ->set_rx_mode() support for multiple
>> unicast MAC addresses in so many drivers all this time, yet no way
>> outside of FDB to make use of it at all.
>
> And even that is not always available. In most drivers it requires
> module parameters or other explicit configuration steps. Meanwhile
> set_rx_mode() doesn't seem to depend on any of those and just does the
> right thing.
>
> For what I was trying to do ioctl() was a really easy way out for both
> kernel and user space implementation, so I gave is shot.
>
> -vlad
>
Don't we already support this with
int (*ndo_fdb_add)(struct ndmsg *ndm,
struct nlattr *tb[],
struct net_device *dev,
const unsigned char *addr,
u16 flags);
int (*ndo_fdb_del)(struct ndmsg *ndm,
struct nlattr *tb[],
struct net_device *dev,
const unsigned char *addr);
int (*ndo_fdb_dump)(struct sk_buff *skb,
struct
netlink_callback *cb,
struct net_device *dev,
int idx);
^ 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