* [PATCH 06/17] can: xilinx_can: fix comparison of unsigned variable
From: Marc Kleine-Budde @ 2014-11-18 20:37 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, Sudip Mukherjee, Sudip Mukherjee,
Marc Kleine-Budde
In-Reply-To: <1416343067-9810-1-git-send-email-mkl@pengutronix.de>
From: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
The variable err was of the type u32. It was being compared with < 0, and being
an unsigned variable the comparison would have been always false.
Moreover, err was getting the return value from set_reset_mode() and
xcan_set_bittiming(), and both are returning int.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
Reviewed-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/xilinx_can.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index 5e8b5609c067..47b2f801d127 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -300,7 +300,8 @@ static int xcan_set_bittiming(struct net_device *ndev)
static int xcan_chip_start(struct net_device *ndev)
{
struct xcan_priv *priv = netdev_priv(ndev);
- u32 err, reg_msr, reg_sr_mask;
+ u32 reg_msr, reg_sr_mask;
+ int err;
unsigned long timeout;
/* Check if it is in reset mode */
--
2.1.3
^ permalink raw reply related
* [PATCH 07/17] can: xilinx_can: add .ndo_change_mtu function
From: Marc Kleine-Budde @ 2014-11-18 20:37 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel, Marc Kleine-Budde
In-Reply-To: <1416343067-9810-1-git-send-email-mkl@pengutronix.de>
Use common can_change_mtu function.
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/xilinx_can.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index 47b2f801d127..8a998e3884ce 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -962,6 +962,7 @@ static const struct net_device_ops xcan_netdev_ops = {
.ndo_open = xcan_open,
.ndo_stop = xcan_close,
.ndo_start_xmit = xcan_start_xmit,
+ .ndo_change_mtu = can_change_mtu,
};
/**
--
2.1.3
^ permalink raw reply related
* [PATCH 08/17] can: rcar_can: add .ndo_change_mtu function
From: Marc Kleine-Budde @ 2014-11-18 20:37 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel, Marc Kleine-Budde
In-Reply-To: <1416343067-9810-1-git-send-email-mkl@pengutronix.de>
Use common can_change_mtu function.
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/rcar_can.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/can/rcar_can.c b/drivers/net/can/rcar_can.c
index 1abe133d1594..9718248e55f1 100644
--- a/drivers/net/can/rcar_can.c
+++ b/drivers/net/can/rcar_can.c
@@ -628,6 +628,7 @@ static const struct net_device_ops rcar_can_netdev_ops = {
.ndo_open = rcar_can_open,
.ndo_stop = rcar_can_close,
.ndo_start_xmit = rcar_can_start_xmit,
+ .ndo_change_mtu = can_change_mtu,
};
static void rcar_can_rx_pkt(struct rcar_can_priv *priv)
--
2.1.3
^ permalink raw reply related
* [PATCH 09/17] can: gs_usb: add .ndo_change_mtu function
From: Marc Kleine-Budde @ 2014-11-18 20:37 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel, Marc Kleine-Budde
In-Reply-To: <1416343067-9810-1-git-send-email-mkl@pengutronix.de>
Use common can_change_mtu function.
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/usb/gs_usb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 04b0f84612f0..009acc8641fc 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -718,6 +718,7 @@ static const struct net_device_ops gs_usb_netdev_ops = {
.ndo_open = gs_can_open,
.ndo_stop = gs_can_close,
.ndo_start_xmit = gs_can_start_xmit,
+ .ndo_change_mtu = can_change_mtu,
};
static struct gs_can *gs_make_candev(unsigned int channel, struct usb_interface *intf)
--
2.1.3
^ permalink raw reply related
* Re: [PATCH 1/2] net: can: remove unused variable
From: Marc Kleine-Budde @ 2014-11-18 20:39 UTC (permalink / raw)
To: Sudip Mukherjee, Wolfgang Grandegger; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <1416318427-28676-1-git-send-email-sudipm.mukherjee@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 488 bytes --]
On 11/18/2014 02:47 PM, Sudip Mukherjee wrote:
> these variable were only assigned some values, but then never
> reused again.
> so they are safe to be removed.
Thanks, I've applied both patches.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: Question about Patch Submissions
From: rapier @ 2014-11-18 20:39 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev
In-Reply-To: <CAHA+R7P-G4P-AzTM248FqNEm=dcuQOB_fwdM0Us3gsQgSeY0UQ@mail.gmail.com>
Cong,
Thanks for the response.
It's a pretty extensive patch set and we're trying to make sure it makes
sense in context. It touches quite a few files and they're mostly
interdependent. That's why we're having problems trying to figure out
how to break this down for presentation. We can make sure each section
of the patch applies cleanly but the entirety of the patch set would
have to be applied for it to compile.
Chris
On 11/18/14, 3:21 PM, Cong Wang wrote:
> On Tue, Nov 18, 2014 at 11:13 AM, rapier <rapier@psc.edu> wrote:
>> I've read the documentation I've been able to find on the patch submission
>> process. I was hoping to clarify one thing though. When submitting a patch
>> that spans multiple files and functions is it better to break the submission
>> down by function or by file?
>>
>
> It depends on what you change, if your patch is trivial, say just cleaning up
> whitespaces, or a big single patch doesn't harm review, it is okay not
> to split it.
> Otherwise, probably you need to split it logically, each of which needs to at
> least compile.
>
^ permalink raw reply
* Re: [PATCH v2 net-next 0/7] implementation of eBPF maps
From: David Miller @ 2014-11-18 20:39 UTC (permalink / raw)
To: ast-uqk4Ao+rVK5Wk0Htik3J/w
Cc: mingo-DgEjT+Ai2ygdnm+yROfE0A, luto-kltTT9wpgjJwATOyAt5JVQ,
dborkman-H+wXaHxf7aLQT0dZR+AlfA,
hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1415929010-9361-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
From: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Date: Thu, 13 Nov 2014 17:36:43 -0800
> v1->v2:
> renamed flags for MAP_UPDATE_ELEM command to be more concise,
> clarified commit logs and improved comments in patches 1,3,7
> per discussions with Daniel
>
> Old v1 cover:
>
> this set of patches adds implementation of HASH and ARRAY types of eBPF maps
> which were described in manpage in commit b4fc1a460f30("Merge branch 'bpf-next'")
>
> The difference vs previous version of these patches from August:
> - added 'flags' attribute to BPF_MAP_UPDATE_ELEM
> - in HASH type implementation removed per-map kmem_cache.
> I was doing kmem_cache_create() for every map to enable selective slub
> debugging to check for overflows and leaks. Now it's not needed, so just
> use normal kmalloc() for map elements.
> - added ARRAY type which was mentioned in manpage, but wasn't public yet
> - added map testsuite and removed temporary bits from test_stubs
>
> Note, eBPF programs cannot be attached to events yet.
> It will come in the next set.
Series applied, thanks Alexei.
^ permalink raw reply
* Re: [PATCH V5 1/2] can: m_can: fix not set can_dlc for remote frame
From: Marc Kleine-Budde @ 2014-11-18 20:40 UTC (permalink / raw)
To: Dong Aisheng, linux-can
Cc: wg, varkabhadram, netdev, socketcan, linux-arm-kernel
In-Reply-To: <1416308455-29134-1-git-send-email-b29396@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 492 bytes --]
On 11/18/2014 12:00 PM, Dong Aisheng wrote:
> The original code missed to set the cf->can_dlc in the RTR case, so add it.
Applied both patches. Let's see what David says to the CAN FD support.
Thanks,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH 1/1] net: xfrm: Deletion of an unnecessary check before the function call "ipcomp_free_tfms"
From: SF Markus Elfring @ 2014-11-18 20:45 UTC (permalink / raw)
To: David S. Miller, Herbert Xu, Steffen Klassert, netdev
Cc: LKML, kernel-janitors, Coccinelle
In-Reply-To: <5317A59D.4@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 18 Nov 2014 21:41:26 +0100
The ipcomp_free_tfms() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
net/xfrm/xfrm_ipcomp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c
index ccfdc71..47863cd 100644
--- a/net/xfrm/xfrm_ipcomp.c
+++ b/net/xfrm/xfrm_ipcomp.c
@@ -320,8 +320,7 @@ error:
static void ipcomp_free_data(struct ipcomp_data *ipcd)
{
- if (ipcd->tfms)
- ipcomp_free_tfms(ipcd->tfms);
+ ipcomp_free_tfms(ipcd->tfms);
ipcomp_free_scratches();
}
--
2.1.3
^ permalink raw reply related
* Re: [PATCH net-next] netdevice: Neaten includes and forward declarations
From: David Miller @ 2014-11-18 20:48 UTC (permalink / raw)
To: joe; +Cc: netdev
In-Reply-To: <1416176491.24600.3.camel@perches.com>
From: Joe Perches <joe@perches.com>
Date: Sun, 16 Nov 2014 14:21:31 -0800
> Use the appropriate #include path for neighbour.h and
> add device.h which was indirectly #included by dmaengine.h
>
> Remove unnecessary forward declaration of struct device;
>
> Add comments for other forward struct declarations.
>
> Signed-off-by: Joe Perches <joe@perches.com>
This no longer applies to net-next, please respin.
Thanks.
^ permalink raw reply
* Re: [patch iproute2] tc: add support for vlan tc action
From: Jiri Pirko @ 2014-11-18 20:48 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: netdev, davem, pshelar, therbert, edumazet, willemb, dborkman,
mst, fw, Paul.Durrant, tgraf
In-Reply-To: <546906D6.7040400@mojatatu.com>
Sun, Nov 16, 2014 at 09:19:34PM CET, jhs@mojatatu.com wrote:
>On 11/12/14 09:55, Jiri Pirko wrote:
>>Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>>Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>
>
>Latest patches work great. If you want you can
>include these notes in the iproute2 commit log.
>
>There is only one small doubt when i add two vlan
>tags(Q followed by QandQ). Look at the very end
>of the text i have below...
I fixed that. Trouble was that tc action finds skb->data in different
state so I have to push and pull. Sending v3 shortly.
>
>cheers,
>jamal
>
>---
>export TC=/media/MT1/other-gits/iproute2-jiri/tc/tc
>export ETH=eth0
>#index supplied by kernel
>sudo $TC actions add action vlan pop
>#explicit add with our index
>sudo $TC actions add action vlan pop index 10
>sudo $TC actions add action vlan push id 123
>sudo $TC actions add action vlan push id 456 protocol 802.1Q
>sudo $TC actions add action vlan push id 789 protocol 802.1ad
>
>sudo $TC actions ls action vlan
>------ expect something like ----
> action order 0: vlan pop
> index 1 ref 1 bind 0
>
> action order 1: vlan push id 123 protocol 802.1Q
> index 2 ref 1 bind 0
>
> action order 2: vlan push id 456 protocol 802.1Q
> index 3 ref 1 bind 0
>
> action order 3: vlan push id 789 protocol 802.1ad
> index 4 ref 1 bind 0
>
> action order 4: vlan pop
> index 10 ref 1 bind 0
>-------
>#show stats
>sudo $TC -s actions ls action vlan
>
>-------
> action order 0: vlan pop
> index 1 ref 1 bind 0 installed 78 sec used 78 sec
> Action statistics:
> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>
> action order 1: vlan push id 123 protocol 802.1Q
> index 2 ref 1 bind 0 installed 44 sec used 44 sec
> Action statistics:
> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>
> action order 2: vlan push id 456 protocol 802.1Q
> index 3 ref 1 bind 0 installed 42 sec used 42 sec
> Action statistics:
> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>
> action order 3: vlan push id 789 protocol 802.1ad
> index 4 ref 1 bind 0 installed 39 sec used 39 sec
> Action statistics:
> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>
> action order 4: vlan pop
> index 10 ref 1 bind 0 installed 47 sec used 47 sec
> Action statistics:
> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>-----
>
>sudo $TC actions flush action vlan
>#expect all actions to be gone..
>sudo $TC actions ls action vlan
>
>#lets bind actions to filters...
>sudo ip li add dev dummy0 type dummy
>sudo ifconfig dummy0 up
>#
>sudo tc qdisc del dev $ETH ingress
>sudo tc qdisc add dev $ETH ingress
>#
>sudo $TC filter add dev $ETH parent ffff: pref 11 protocol ip \
>u32 match ip src 10.0.0.1 flowid 1:1 \
>action vlan push id 123 \
>action mirred egress redirect dev dummy0
>
>window 1> sudo tcpdump -n -i dummy0 -e -X
>window 2> ping -c 1 10.0.0.1
>
>Expect something like:
>52:54:00:c3:4b:c5 > 02:00:00:22:01:01, ethertype 802.1Q (0x8100), length 102:
>vlan 123, p 0, ethertype IPv4, ...
>#now look at the stats..
>sudo $TC -s filter ls dev $ETH parent ffff: protocol ip
>#
>sudo $TC filter add dev $ETH parent ffff: pref 12 protocol ip \
>u32 match ip src 10.0.0.2 flowid 1:2 \
>action vlan push id 456 protocol 802.1Q \
>action mirred egress redirect dev dummy0
>
>sudo tcpdump -n -i dummy0 -X -e
>ping -c 1 10.0.0.2
>#look at the stats..
>sudo $TC -s filter ls dev $ETH parent ffff: protocol ip
>#
>sudo $TC filter add dev $ETH parent ffff: pref 13 protocol ip \
>u32 match ip src 10.0.0.13 flowid 1:13 \
>action vlan push id 789 protocol 802.1ad \
>action mirred egress redirect dev dummy0
>ping -c 1 10.0.0.2
>
>sudo tcpdump -n -i dummy0 -X -e
>Expect ...
>52:54:00:c3:4b:c5 > 02:00:00:22:01:01, ethertype 802.1Q-QinQ (0x88a8), length
>102: vlan 789, p 0, ethertype IPv4,,,
>#
>sudo $TC -s filter ls dev $ETH parent 1: protocol ip
>#
># Speaking in New Brunswickian:
># For shits and giggles lets add two vlans ...
># match all pings this time...
>#
>sudo $TC filter add dev $ETH parent ffff: pref 11 protocol ip u32 \
>match ip protocol 1 0xff flowid 1:1 \
>action vlan push id 123 \
>action vlan push id 789 protocol 802.1ad \
>action mirred egress redirect dev dummy0
>
>sudo $TC -s filter ls dev $ETH parent ffff: protocol ip
>------
>filter pref 11 u32
>filter pref 11 u32 fh 800: ht divisor 1
>filter pref 11 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:1 (rule
>hit 2 success 0)
> match 00010000/00ff0000 at 8 (success 0 )
> action order 1: vlan push id 123 protocol 802.1Q
> index 13 ref 1 bind 1 installed 6 sec used 6 sec
> Action statistics:
> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>
> action order 2: vlan push id 789 protocol 802.1ad
> index 14 ref 1 bind 1 installed 6 sec used 6 sec
> Action statistics:
> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>
> action order 3: mirred (Egress Redirect to device dummy0) stolen
> index 9 ref 1 bind 1 installed 6 sec used 6 sec
> Action statistics:
> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>----
>
>Send 10 pings to 192.168.100.1
>ping 192.168.100.1 -c 10
>
>sudo $TC -s filter ls dev $ETH parent ffff: protocol ip
>------
>filter pref 11 u32
>filter pref 11 u32 fh 800: ht divisor 1
>filter pref 11 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:1 (rule
>hit 24 success 10)
> match 00010000/00ff0000 at 8 (success 10 )
> action order 1: vlan push id 123 protocol 802.1Q
> index 13 ref 1 bind 1 installed 143 sec used 60 sec
> Action statistics:
> Sent 840 bytes 10 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>
> action order 2: vlan push id 789 protocol 802.1ad
> index 14 ref 1 bind 1 installed 143 sec used 60 sec
> Action statistics:
> Sent 840 bytes 10 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>
> action order 3: mirred (Egress Redirect to device dummy0) stolen
> index 9 ref 1 bind 1 installed 143 sec used 60 sec
> Action statistics:
> Sent 840 bytes 10 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>--------
>
>As can be seen, the two vlan tags were supposedly added. I am
>not sure how well it worked. I see 4 more bytes added. tcpdump doesnt do
>a good job telling me if it worked...
>
>17:58:31.636816 00:22:01:01:52:54 > 00:00:00:00:02:00, ethertype 802.1Q-QinQ
>(0x88a8), length 106: vlan 789, p 0, LLC, dsap Unknown (0x44) Group, ssap
>Null (0x00) Command, ctrl 0x5400: Information, send seq 0, rcv seq 42, Flags
>[Command], length 88
> 0x0000: 0000 0000 0200 0022 0101 5254 88a8 0315
> 0x0010: 00c3 4500 0054 c06b 0000 4001 704c 8100
> 0x0020: 007b c0a8 6401 c0a8 649f 0000 24b8 027c
> 0x0030: 000a c7e5 6854 0000 0000 e0b4 0900 0000
> 0x0040: 0000 1011 1213 1415 1617 1819 1a1b 1c1d
> 0x0050: 1e1f 2021 2223 2425 2627 2829 2a2b 2c2d
> 0x0060: 2e2f 3031 3233 3435 3637
>
>Here it is with just 802.1q tag...
>17:53:38.198323 52:54:00:c3:4b:c5 > 02:00:00:22:01:01, ethertype 802.1Q
>(0x8100), length 102: vlan 456, p 0, ethertype IPv4, 192.168.100.1 >
>192.168.100.159: ICMP echo reply, id 620, seq 1, length 64
> 0x0000: 0200 0022 0101 5254 00c3 4bc5 8100 01c8
> 0x0010: 0800 4500 0054 c052 0000 4001 7065 c0a8
> 0x0020: 6401 c0a8 649f 0000 6482 026c 0001 a2e4
> 0x0030: 6854 0000 0000 cc04 0300 0000 0000 1011
> 0x0040: 1213 1415 1617 1819 1a1b 1c1d 1e1f 2021
> 0x0050: 2223 2425 2627 2829 2a2b 2c2d 2e2f 3031
> 0x0060: 3233 3435 3637
>
>------------
>
>
^ permalink raw reply
* Re: [RFC] situation with csum_and_copy_... API
From: Linus Torvalds @ 2014-11-18 20:49 UTC (permalink / raw)
To: Al Viro; +Cc: Network Development, David Miller, Linux Kernel Mailing List
In-Reply-To: <20141118084745.GT7996@ZenIV.linux.org.uk>
On Tue, Nov 18, 2014 at 12:47 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> The minimal implementations would be
>
> __wsum csum_and_copy_from_user(const void __user *src, void *dst, int len,
> __wsum sum, int *err_ptr)
> {
> if (unlikely(copy_from_user(dst, src, len) < 0)) {
No. That "< 0" should be "!= 0". The user copy functions return a
positive value of how many bytes they *failed* to copy.
> Note that we are *not* guaranteed that *err_ptr is touched in case of success
> - not all architectures zero it in that case. Callers must (and do) zero it
> before the call of csum_and_copy_{from,to}_user(). Furthermore, return value
> in case of error is undefined.
Yeah, easy to misuse.
> IMO the calling conventions are atrocious.
Yeah, not pretty. At the same time, the pain of changing what seems to
work might not be worth it.
And quite frankly, I *detest* your patch 3/5.
"access_ok()" isn't that expensive, and removing them as unnecessary
is fraught with errors. We've had several cases of "oops, we used
__get_user() in a loop, because it generates much better code, but
we'd forgotten to do access_ok(), so now people can read kernel data".
I really think that
(a) using "__get_user/__put_user/__memcpy_from/to_user" should be
avoided unless there is a clear and present performance issue
(b) when that performance issue is clear and unmistakable, you should
generally have the "access_ok()" check *locally* to the use of unsafe
ops, so that you can *locally* see that it's safe.
(c) if there is some upper-level check (ie the normal read/write
paths that make *sure* the addresses are fine), and there is some huge
performance issue that means that the local checks would be a problem,
then we should have a big comment about exactly where the checks are
done.
Your 3/5 violates pretty much all of these, imho.
The rest of the patches I have nothing against. I'm a bit worried that
this is stuff that can easily get stupid thinkos (ie exactly due to
things like the argument order thing), and I wonder how much it buys
us, but at least it seems to generally remove more lines than it adds,
and cleans some stuff up, so I'm not against it.
Linus
Linus
^ permalink raw reply
* Re: [PATCH] fix #51791 - bug? mac 00:00:00:00:00:00 with natsemi DP83815 after driver load
From: David Miller @ 2014-11-18 20:49 UTC (permalink / raw)
To: devzero; +Cc: netdev
In-Reply-To: <trinity-9c3b63ff-f474-4d99-84d8-29034921bb3c-1416183467662@3capp-webde-bs06>
From: "Roland Kletzing" <devzero@web.de>
Date: Mon, 17 Nov 2014 01:17:47 +0100
> This one should fix Bugzilla #51791 (details below).
>
> Natsemi driver does not read MAC correctly from eeprom, while natsemi-diag from nictools-pci does. Apparently, tt`s a timing issue in the kernel driver.
>
> According to ftp://ftp.gwdg.de/pub/linux/misc/donald.becker/diag/natsemi-diag.c , eeprom_delay(ee_addr) is defined as follows:
>
> /* Delay between EEPROM clock transitions.
> This flushes the write buffer to prevent quick double-writes.
> */
> #define eeprom_delay(ee_addr) inl(ee_addr); inl(ee_addr)
>
> while in the natsemi linux kernel driver, the delay is done this way :
>
> #define eeprom_delay(ee_addr) readl(ee_addr)
>
> , which results in the MAC being all zero`s.
>
> So i simply added a second readl() to increase delay (instead of turning into inl() as proposed before). This may look a little bit ugly, but it`s fixing the problem for me.
>
> I´m not sure how many natsemi users being left on this planet (probably few), but i guess this change does not do any harm on platforms where the driver does not behave buggy, so please consider adding it to mainline/stable/longterm.
>
> Signed-off-by: Roland Kletzing <devzero@web.de>
Please post your patch as a clean new email, without quoting things
after your signoff etc.
Also, please break your lines up to 80 columns max.
Thanks.
^ permalink raw reply
* Re: ipx: fix locking regression in ipx_sendmsg and ipx_recvmsg
From: David Miller @ 2014-11-18 20:49 UTC (permalink / raw)
To: arnd; +Cc: jbohac, acme, netdev
In-Reply-To: <8698539.evYG7fs8jS@wuerfel>
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 18 Nov 2014 14:37:26 +0100
> Does ipxrtr_route_packet() actually sleep while waiting for the network,
> or is it possible that you only need to change the recvmsg path?
I went over that code path and it doesn't appear to sleep at all.
^ permalink raw reply
* Re: [PATCH v4 0/4] Add lightweight memory barriers for coherent memory access
From: Linus Torvalds @ 2014-11-18 20:53 UTC (permalink / raw)
To: Alexander Duyck
Cc: linux-arch@vger.kernel.org, Network Development,
Linux Kernel Mailing List, Mathieu Desnoyers, Peter Zijlstra,
Benjamin Herrenschmidt, Heiko Carstens, Ingo Molnar,
Michael Neuling, Russell King - ARM Linux, donald.c.skidmore,
matthew.vick, Geert Uytterhoeven, Jeff Kirsher, Francois Romieu,
Paul McKenney, nic_swsd, Will Deacon, Michael Ellerman, Tony Luck,
Oleg Nesterov, Martin Schwidefsky <
In-Reply-To: <20141118172644.26303.37688.stgit@ahduyck-server>
On Tue, Nov 18, 2014 at 9:28 AM, Alexander Duyck
<alexander.h.duyck@redhat.com> wrote:
> These patches introduce two new primitives for synchronizing cache coherent
> memory writes and reads. These two new primitives are:
>
> coherent_rmb()
> coherent_wmb()
So I'm still not convinced about the name. I don't hate it, but if you
ever want to do "read_acquire", then that whole "coherent_" thing does
make for a big mouthful. I don't see why "dma" isn't simpler and more
to the point, and has the advantage of lining up (in documentation
etc) with "smp".
Why would you ever use "coherent_xyz()" on something that isn't about
dma? If it's cache-coherent memory without DMA, you'd use "smp_xyz()",
so I really do prefer that whole "dma-vs-smp" issue, because it talks
about what is actually the important issue. All sane memory is
coherent, after all (and if it isn't, you have other issues than
memory ordering).
Linus
^ permalink raw reply
* Re: [PATCH net-next v2] ipvlan: Initial check-in of the IPVLAN driver.
From: David Miller @ 2014-11-18 20:54 UTC (permalink / raw)
To: maheshb; +Cc: netdev, edumazet, maze, chavey, thockin, brandon.philips, xemul
In-Reply-To: <1416205634-26611-1-git-send-email-maheshb@google.com>
From: Mahesh Bandewar <maheshb@google.com>
Date: Sun, 16 Nov 2014 22:27:14 -0800
> +/* Define IPVL_DEBUG and set the appropriate dbg_level for debugging. */
> +#ifdef IPVL_DEBUG
> +/*
> + * 1 : non-datapath debugging
> + * 2 : Custom
> + * 3 : function enters and exists.
> + * 4 : printk in data path (be careful!)
> + */
> +#define IPVL_DBG_LEVEL 1
> +#define ipvlan_dbg(level, msg...) do { \
> + if (level <= IPVL_DBG_LEVEL) \
> + printk(KERN_DEBUG msg); \
> + } while (0)
> +#else
> +#define ipvlan_dbg(level, msg...) do { ; } while (0)
> +#endif
The day of having code use custom local debug logging facilities is long
gone, please use a standard mechanism for this rather than home cooked
reimplementations.
> +/* ---- Prototype declarations ---- */
> +/* ---- ipvlan_main.c ---- */
Everone knows these are prototype declarations by virtue of them being
functions declarations in a header file.
Anyone who wants to know where these are defined can run grep.
So these comments are superfluous, please remove them across this
entire submission.
> + const struct in_addr *ip4_addr = iaddr;
> + return jhash_1word(ip4_addr->s_addr, ipvlan_jhash_secret)
> + & IPVLAN_HASH_MASK;
When an expression spans multiple lines, lines end rather than begin
with an operator.
> + if (is_v6 && addr->atype == IPVL_IPV6 &&
> + ipv6_addr_equal(&addr->ip6addr, iaddr))
For multi-line if() statements, the second line and subsequent lines
must begine exactly at the first column after the openning parenthesis
of the first line.
If you are purely using TAB characters to indent, you are likely doing
it wrong.
> + if ((is_v6 && addr->atype == IPVL_IPV6 &&
> + ipv6_addr_equal(&addr->ip6addr, iaddr))
> + || (!is_v6 && addr->atype == IPVL_IPV4 &&
> + addr->ip4addr.s_addr == ((struct in_addr *)iaddr)->s_addr)
Lines end, not begin, with operators.
> --- /dev/null
> +++ b/drivers/net/ipvlan/ipvlan_sysfs.c
Please do not add sysfs configuration knobs, netlink is sufficient.
^ permalink raw reply
* Re: [PATCH 0/8] net: fec: assorted cleanup patches
From: David Miller @ 2014-11-18 20:54 UTC (permalink / raw)
To: LW; +Cc: netdev, Frank.Li, fugang.duan, rmk+kernel
In-Reply-To: <1416217884-20911-1-git-send-email-LW@KARO-electronics.de>
From: Lothar Waßmann <LW@KARO-electronics.de>
Date: Mon, 17 Nov 2014 10:51:16 +0100
> This patch series is a followup to:
> <1415350967-2238-1-git-send-email-LW@KARO-electronics.de>
> [PATCHv4 1/1] net: fec: fix regression on i.MX28 introduced by rx_copybreak support
> to apply the cleanup patches that were originally sent along with the
> bugfix patch.
Series applied, thanks.
^ permalink raw reply
* Re: pull-request: can-next 2014-11-17
From: David Miller @ 2014-11-18 20:54 UTC (permalink / raw)
To: mkl; +Cc: netdev, linux-can, kernel
In-Reply-To: <546A0920.4060706@pengutronix.de>
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Mon, 17 Nov 2014 15:41:36 +0100
> this is a pull request of 9 patches for net-next/master.
>
> All 9 patches are by Roger Quadros and update the c_can platform
> driver. First by improving the initialization sequence of the message
> RAM, making use of syscon/regmap. In the later patches support for
> various TI SoCs is added.
Pulled, thanks Marc.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] dccp: kerneldoc warning fixes
From: David Miller @ 2014-11-18 20:55 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, gerrit, dccp, netdev
In-Reply-To: <1416257481-5690-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Mon, 17 Nov 2014 21:51:21 +0100
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] dccp: remove blank lines between function/EXPORT_SYMBOL
From: David Miller @ 2014-11-18 20:55 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, gerrit, dccp, netdev
In-Reply-To: <1416257698-5889-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Mon, 17 Nov 2014 21:54:58 +0100
> See Documentation/CodingStyle chapter 6.
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] dccp: replace min/casting by min_t
From: David Miller @ 2014-11-18 20:55 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, gerrit, dccp, netdev
In-Reply-To: <1416257917-7041-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Mon, 17 Nov 2014 21:58:37 +0100
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] dccp: spelling s/reseting/resetting
From: David Miller @ 2014-11-18 20:55 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, gerrit, dccp, netdev
In-Reply-To: <1416258022-7320-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Mon, 17 Nov 2014 22:00:22 +0100
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] net: fix spelling for synchronized
From: David Miller @ 2014-11-18 20:55 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, netdev
In-Reply-To: <1416258243-7685-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Mon, 17 Nov 2014 22:04:03 +0100
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] net/core: include linux/types.h instead of asm/types.h
From: David Miller @ 2014-11-18 20:55 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, netdev
In-Reply-To: <1416258502-9001-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Mon, 17 Nov 2014 22:08:22 +0100
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] dev_ioctl: use sizeof(x) instead of sizeof x
From: David Miller @ 2014-11-18 20:55 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, netdev
In-Reply-To: <1416259397-10954-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Mon, 17 Nov 2014 22:23:17 +0100
> Also remove spaces after cast.
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied.
^ 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