* Re: [PATCH net] bpf: don't select potentially stale ri->map from buggy xdp progs
From: Jesper Dangaard Brouer @ 2017-09-08 11:52 UTC (permalink / raw)
To: Daniel Borkmann, andy; +Cc: davem, ast, john.fastabend, netdev, brouer
In-Reply-To: <59B27234.4050703@iogearbox.net>
On Fri, 08 Sep 2017 12:34:28 +0200 Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 09/08/2017 07:06 AM, Jesper Dangaard Brouer wrote:
> > On Fri, 8 Sep 2017 00:14:51 +0200
> > Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> >> + /* This is really only caused by a deliberately crappy
> >> + * BPF program, normally we would never hit that case,
> >> + * so no need to inform someone via tracepoints either,
> >> + * just bail out.
> >> + */
> >> + if (unlikely(map_owner != xdp_prog))
> >> + return -EINVAL;
> >
> > IMHO we do need to call the tracepoint here. It is not just crappy
> > BPF-progs that cause this situation, it is also drivers not implementing
> > XDP_REDIRECT yet (which is all but ixgbe). Due to the level XDP
> > operates at, tracepoints are the only way users can runtime troubleshoot
> > their XDP programs.
>
> Drivers not implementing XDP_REDIRECT don't even get there in
> the first place. What they will do is to hit the 'default' case
> when they check for the action code from the BPF program. Then
> call into bpf_warn_invalid_xdp_action(act), and fall-through
> to hit the tracepoint at trace_xdp_exception() which is also
> triggered by XDP_ABORTED usually. So when that happens we do
> complain loudly and call a tracepoint already. We should probably
> tweak the bpf_warn_invalid_xdp_action() message a little to make
> it clear that the action could also just be unsupported by the
> driver instead of being illegal.
Yes. drivers not implementing XDP_REDIRECT will cause a tracepoint
trace_xdp_exception() to be called for its _own_ packets.
But it will still setup and leave map and map_owner pointer dangling.
Another NIC can load an xdp_prog that return XDP_REDIRECT, which will hit
above if-statement, and its packets will disappear, without getting
recorded by a tracepoint (thus hard to debug!).
The fundamental point is that tracepoints is the way we choose to
handle debugging XDP programs. Thus, we must trigger a tracepoint when
a packet gets dropped. Even in this unlikely case.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH] net:netfilter alloc xt_byteslimit_htable with wrong size
From: Pablo Neira Ayuso @ 2017-09-08 11:58 UTC (permalink / raw)
To: zhizhou.tian
Cc: kadlec, fw, davem, netfilter-devel, coreteam, netdev,
linux-kernel
In-Reply-To: <1504839616-14519-1-git-send-email-zhizhou.tian@gmail.com>
On Fri, Sep 08, 2017 at 11:00:16AM +0800, zhizhou.tian@gmail.com wrote:
> From: Zhizhou Tian <zhizhou.tian@gmail.com>
>
> struct xt_byteslimit_htable used hlist_head,
> but alloc memory with sizeof(struct list_head)
Applied, thanks.
For the record, I have mangled the patch titled to:
netfilter: xt_hashlimit: alloc hashtable with right size
^ permalink raw reply
* Re: [PATCH iproute2 0/2] malloc correct buff at run time
From: Michal Kubecek @ 2017-09-08 12:03 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, Stephen Hemminger, Phil Sutter
In-Reply-To: <1504865697-27274-1-git-send-email-liuhangbin@gmail.com>
On Fri, Sep 08, 2017 at 06:14:55PM +0800, Hangbin Liu wrote:
> With commit 72b365e8e0fd ("libnetlink: Double the dump buffer size") and
> 460c03f3f3cc ("iplink: double the buffer size also in iplink_get()"), we
> extend the buffer size to avoid truncated message with large numbers of
> VFs. But just as Michal said, this is not future-proof since the NIC
> number is increasing. We have customer even has 220+ VFs now.
This sounds like the moment we hit the bigger problem with
IFLA_VFINFO_LIST is closer than I thought. The info block for one VF is
already 236 bytes long (or was in 4.4) so that 278 VFs would be over the
natural limit for IFLA_VFINFO_LIST attribute given by 16-bit nla_len
field.
> This is not make sense to hard code the buffer and increase it all the time.
> So let's just malloc the correct buff size at run time.
>
> I'm not sure what init size would be suitable, so I keep use the original
> size. I have tried with a small size like 1024, and it also works.
I don't think it's that important as the command usually runs only for
short time so allocating a 32KB buffer even if we could do with less is
not a big issue. (I didn't really like the idea of a 32KB buffer on
stack but with malloc() it's OK, I would say.)
Michal Kubecek
^ permalink raw reply
* [PATCH 1/1] forcedeth: remove tx_stop variable
From: Zhu Yanjun @ 2017-09-08 12:28 UTC (permalink / raw)
To: davem, netdev
The variable tx_stop is used to indicate the tx queue state: started
or stopped. In fact, the inline function netif_queue_stopped can do
the same work. So replace the variable tx_stop with the
function netif_queue_stopped.
Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
---
drivers/net/ethernet/nvidia/forcedeth.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 994a83a..e6e0de4 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -834,7 +834,6 @@ struct fe_priv {
u32 tx_pkts_in_progress;
struct nv_skb_map *tx_change_owner;
struct nv_skb_map *tx_end_flip;
- int tx_stop;
/* TX software stats */
struct u64_stats_sync swstats_tx_syncp;
@@ -1939,7 +1938,6 @@ static void nv_init_tx(struct net_device *dev)
np->tx_pkts_in_progress = 0;
np->tx_change_owner = NULL;
np->tx_end_flip = NULL;
- np->tx_stop = 0;
for (i = 0; i < np->tx_ring_size; i++) {
if (!nv_optimized(np)) {
@@ -2211,7 +2209,6 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev)
empty_slots = nv_get_empty_tx_slots(np);
if (unlikely(empty_slots <= entries)) {
netif_stop_queue(dev);
- np->tx_stop = 1;
spin_unlock_irqrestore(&np->lock, flags);
return NETDEV_TX_BUSY;
}
@@ -2359,7 +2356,6 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb,
empty_slots = nv_get_empty_tx_slots(np);
if (unlikely(empty_slots <= entries)) {
netif_stop_queue(dev);
- np->tx_stop = 1;
spin_unlock_irqrestore(&np->lock, flags);
return NETDEV_TX_BUSY;
}
@@ -2583,8 +2579,8 @@ static int nv_tx_done(struct net_device *dev, int limit)
netdev_completed_queue(np->dev, tx_work, bytes_compl);
- if (unlikely((np->tx_stop == 1) && (np->get_tx.orig != orig_get_tx))) {
- np->tx_stop = 0;
+ if (unlikely(netif_queue_stopped(dev) &&
+ (np->get_tx.orig != orig_get_tx))) {
netif_wake_queue(dev);
}
return tx_work;
@@ -2637,8 +2633,8 @@ static int nv_tx_done_optimized(struct net_device *dev, int limit)
netdev_completed_queue(np->dev, tx_work, bytes_cleaned);
- if (unlikely((np->tx_stop == 1) && (np->get_tx.ex != orig_get_tx))) {
- np->tx_stop = 0;
+ if (unlikely(netif_queue_stopped(dev) &&
+ (np->get_tx.ex != orig_get_tx))) {
netif_wake_queue(dev);
}
return tx_work;
@@ -2724,7 +2720,6 @@ static void nv_tx_timeout(struct net_device *dev)
/* 2) complete any outstanding tx and do not give HW any limited tx pkts */
saved_tx_limit = np->tx_limit;
np->tx_limit = 0; /* prevent giving HW any limited pkts */
- np->tx_stop = 0; /* prevent waking tx queue */
if (!nv_optimized(np))
nv_tx_done(dev, np->tx_ring_size);
else
--
2.7.4
^ permalink raw reply related
* Re: [PATCH iproute2 1/2] lib/libnetlink: re malloc buff if size is not enough
From: Michal Kubecek @ 2017-09-08 12:32 UTC (permalink / raw)
To: Phil Sutter, Hangbin Liu, netdev, Stephen Hemminger
In-Reply-To: <20170908110247.GS2399@orbyte.nwl.cc>
On Fri, Sep 08, 2017 at 01:02:47PM +0200, Phil Sutter wrote:
> Hi Hangbin,
>
> On Fri, Sep 08, 2017 at 06:14:56PM +0800, Hangbin Liu wrote:
> [...]
> > diff --git a/lib/libnetlink.c b/lib/libnetlink.c
> > index be7ac86..37cfb5a 100644
> > --- a/lib/libnetlink.c
> > +++ b/lib/libnetlink.c
> > @@ -402,6 +402,59 @@ static void rtnl_dump_error(const struct rtnl_handle *rth,
> > }
> > }
> >
> > +static int rtnl_recvmsg(int fd, struct msghdr *msg, char **buf)
> > +{
> > + struct iovec *iov;
> > + int len = -1, buf_len = 32768;
> > + char *buffer = *buf;
>
> Isn't it possible to make 'buffer' static instead of the two 'buf'
> variables in rtnl_dump_filter_l() and __rtnl_talk()? Then we would have
> only a single buffer which is shared between both functions instead of
> two which are independently allocated.
Do we have to worry about reentrancy? Only arpd is multithreaded in
iproute2 but there might be also other users of libnetlink.
> > +
> > + int flag = MSG_PEEK | MSG_TRUNC;
> > +
> > + if (buffer == NULL)
> > +re_malloc:
> > + buffer = malloc(buf_len);
>
> I think using realloc() here is more appropriate since there is no need
> to free the buffer in beforehand and calling realloc(NULL, len) is
> equivalent to calling malloc(len). I think 'realloc' is also a better
> name for the goto label.
>
> > + if (buffer == NULL) {
> > + fprintf(stderr, "malloc error: no enough buffer\n");
>
> Minor typo here: s/no/not/
>
> > + return -1;
>
> Return -ENOMEM?
Hm... the only caller of rtnl_dump_filter_l only checks if the return
value is negative. Even worse, at least some of the functions calling
__rtnl_talk() check errno (or use perror()) instead, even if it's not
always preserved. That's something for a wider cleanup, I would say.
>
> > + }
> > +
> > + iov = msg->msg_iov;
> > + iov->iov_base = buffer;
> > + iov->iov_len = buf_len;
> > +
> > +re_recv:
>
> Just call this 'recv'? (Not really important though.)
>
> > + len = recvmsg(fd, msg, flag);
> > +
> > + if (len < 0) {
> > + if (errno == EINTR || errno == EAGAIN)
> > + return 0;
>
> Instead of returning 0 (which makes callers retry), goto re_recv?
>
> > + fprintf(stderr, "netlink receive error %s (%d)\n",
> > + strerror(errno), errno);
> > + return len;
> > + }
> > +
> > + if (len == 0) {
> > + fprintf(stderr, "EOF on netlink\n");
> > + return -1;
>
> Return -ENODATA here? (Initially I though about -EOF, but EOF is -1 so
> that would be incorrect).
>
> > + }
> > +
> > + if (len > buf_len) {
> > + free(buffer);
>
> If you use realloc() above, this can be dropped.
>
> > + buf_len = len;
>
> For this to work you have to make buf_len static too, otherwise you will
> unnecessarily reallocate the buffer. Oh, and that also requires the
> single buffer (as pointed out above) because you will otherwise use a
> common buf_len for both static buffers passed to this function.
>
> > + flag = 0;
> > + goto re_malloc;
> > + }
> > +
> > + if (flag != 0) {
> > + flag = 0;
> > + goto re_recv;
> > + }
> > +
> > + *buf = buffer;
> > + return len;
> > +}
> > +
> > int rtnl_dump_filter_l(struct rtnl_handle *rth,
> > const struct rtnl_dump_filter_arg *arg)
> > {
> > @@ -413,31 +466,20 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
> > .msg_iov = &iov,
> > .msg_iovlen = 1,
> > };
> > - char buf[32768];
> > + static char *buf = NULL;
>
> If you keep the static buffer in rtnl_recvmsg(), there is no need to
> assign NULL here.
>
> > int dump_intr = 0;
> >
> > - iov.iov_base = buf;
> > while (1) {
> > int status;
> > const struct rtnl_dump_filter_arg *a;
> > int found_done = 0;
> > int msglen = 0;
> >
> > - iov.iov_len = sizeof(buf);
> > - status = recvmsg(rth->fd, &msg, 0);
> > -
> > - if (status < 0) {
> > - if (errno == EINTR || errno == EAGAIN)
> > - continue;
> > - fprintf(stderr, "netlink receive error %s (%d)\n",
> > - strerror(errno), errno);
> > - return -1;
> > - }
> > -
> > - if (status == 0) {
> > - fprintf(stderr, "EOF on netlink\n");
> > - return -1;
> > - }
> > + status = rtnl_recvmsg(rth->fd, &msg, &buf);
> > + if (status < 0)
> > + return status;
> > + else if (status == 0)
> > + continue;
>
> When retrying inside rtnl_recvmsg(), it won't return 0 anymore. I
> believe the whole 'while (1)' loop could go away then.
Doesn't this loop also handle the response divided into multiple
packets?
Michal Kubecek
^ permalink raw reply
* Re: [PATCH net] bpf: don't select potentially stale ri->map from buggy xdp progs
From: Daniel Borkmann @ 2017-09-08 12:34 UTC (permalink / raw)
To: Jesper Dangaard Brouer, andy; +Cc: davem, ast, john.fastabend, netdev
In-Reply-To: <20170908135242.55f99177@redhat.com>
On 09/08/2017 01:52 PM, Jesper Dangaard Brouer wrote:
> On Fri, 08 Sep 2017 12:34:28 +0200 Daniel Borkmann <daniel@iogearbox.net> wrote:
>> On 09/08/2017 07:06 AM, Jesper Dangaard Brouer wrote:
>>> On Fri, 8 Sep 2017 00:14:51 +0200
>>> Daniel Borkmann <daniel@iogearbox.net> wrote:
>>>
>>>> + /* This is really only caused by a deliberately crappy
>>>> + * BPF program, normally we would never hit that case,
>>>> + * so no need to inform someone via tracepoints either,
>>>> + * just bail out.
>>>> + */
>>>> + if (unlikely(map_owner != xdp_prog))
>>>> + return -EINVAL;
>>>
>>> IMHO we do need to call the tracepoint here. It is not just crappy
>>> BPF-progs that cause this situation, it is also drivers not implementing
>>> XDP_REDIRECT yet (which is all but ixgbe). Due to the level XDP
>>> operates at, tracepoints are the only way users can runtime troubleshoot
>>> their XDP programs.
>>
>> Drivers not implementing XDP_REDIRECT don't even get there in
>> the first place. What they will do is to hit the 'default' case
>> when they check for the action code from the BPF program. Then
>> call into bpf_warn_invalid_xdp_action(act), and fall-through
>> to hit the tracepoint at trace_xdp_exception() which is also
>> triggered by XDP_ABORTED usually. So when that happens we do
>> complain loudly and call a tracepoint already. We should probably
>> tweak the bpf_warn_invalid_xdp_action() message a little to make
>> it clear that the action could also just be unsupported by the
>> driver instead of being illegal.
>
> Yes. drivers not implementing XDP_REDIRECT will cause a tracepoint
> trace_xdp_exception() to be called for its _own_ packets.
Yep, plus a big one time warning for the case a user doesn't
look at tracepoints initially.
> But it will still setup and leave map and map_owner pointer dangling.
> Another NIC can load an xdp_prog that return XDP_REDIRECT, which will hit
> above if-statement, and its packets will disappear, without getting
> recorded by a tracepoint (thus hard to debug!).
If a user wants to reproduce this exact error, he would need
to go and reload the program on the driver not supporting the
XDP_REDIRECT in the first place, and then reload his buggy program
on the other driver supporting XDP_REDIRECT but w/o having called
bpf_xdp_redirect_map(), so exactly once on the switch from one
driver to another with this misuse, any subsequent packets will
trigger _trace_xdp_redirect_err(), same way as if the buggy
program was loaded to the 2nd driver from the beginning since
the map and ifindex etc will be zero, hence my comment on this.
^ permalink raw reply
* Re: [PATCH v2] netfilter: xt_hashlimit: fix build error caused by 64bit division
From: Pablo Neira Ayuso @ 2017-09-08 12:56 UTC (permalink / raw)
To: Vishwanath Pai
Cc: netfilter-devel, torvalds, davem, kadlec, johunt, fw, netdev,
pai.vishwain, mingo, ilubashe, bp, luto, x86, linux-kernel,
brgerst, andrew.cooper3, jgross, boris.ostrovsky, keescook, akpm,
arnd
In-Reply-To: <1504849138-17427-1-git-send-email-vpai@akamai.com>
On Fri, Sep 08, 2017 at 01:38:58AM -0400, Vishwanath Pai wrote:
> 64bit division causes build/link errors on 32bit architectures. It
> prints out error messages like:
>
> ERROR: "__aeabi_uldivmod" [net/netfilter/xt_hashlimit.ko] undefined!
>
> The value of avg passed through by userspace in BYTE mode cannot exceed
> U32_MAX. Which means 64bit division in user2rate_bytes is unnecessary.
> To fix this I have changed the type of param 'user' to u32.
>
> Since anything greater than U32_MAX is an invalid input we error out in
> hashlimit_mt_check_common() when this is the case.
>
> Changes in v2:
> Making return type as u32 would cause an overflow for small
> values of 'user' (for example 2, 3 etc). To avoid this I bumped up
> 'r' to u64 again as well as the return type. This is OK since the
> variable that stores the result is u64. We still avoid 64bit
> division here since 'user' is u32.
Applied, thanks.
^ permalink raw reply
* [PATCH net-next 0/3] add UniPhier AVE ethernet support
From: Kunihiko Hayashi @ 2017-09-08 13:02 UTC (permalink / raw)
To: netdev, David S. Miller, Andrew Lunn, Florian Fainelli
Cc: Rob Herring, Mark Rutland, linux-arm-kernel, linux-kernel,
devicetree, Masahiro Yamada, Masami Hiramatsu, Jassi Brar,
Kunihiko Hayashi
This series adds support for Socionext AVE ethernet controller implemented
on UniPhier SoCs. This driver supports RGMII/RMII modes.
Furthermore, this series includes support for realtek RTL8201F PHY to be
implemented on some supported boards.
Jassi Brar (1):
net: phy: realtek: add RTL8201F phy-id and functions
Kunihiko Hayashi (2):
dt-bindings: net: add DT bindings for Socionext UniPhier AVE
net: ethernet: socionext: add AVE ethernet driver
.../bindings/net/socionext,uniphier-ave4.txt | 44 +
drivers/net/ethernet/Kconfig | 1 +
drivers/net/ethernet/Makefile | 1 +
drivers/net/ethernet/socionext/Kconfig | 22 +
drivers/net/ethernet/socionext/Makefile | 4 +
drivers/net/ethernet/socionext/sni_ave.c | 1618 ++++++++++++++++++++
drivers/net/phy/realtek.c | 45 +
7 files changed, 1735 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
create mode 100644 drivers/net/ethernet/socionext/Kconfig
create mode 100644 drivers/net/ethernet/socionext/Makefile
create mode 100644 drivers/net/ethernet/socionext/sni_ave.c
--
2.7.4
^ permalink raw reply
* [PATCH net-next 1/3] dt-bindings: net: add DT bindings for Socionext UniPhier AVE
From: Kunihiko Hayashi @ 2017-09-08 13:02 UTC (permalink / raw)
To: netdev, David S. Miller, Andrew Lunn, Florian Fainelli
Cc: Rob Herring, Mark Rutland, linux-arm-kernel, linux-kernel,
devicetree, Masahiro Yamada, Masami Hiramatsu, Jassi Brar,
Kunihiko Hayashi
In-Reply-To: <1504875731-3680-1-git-send-email-hayashi.kunihiko@socionext.com>
DT bindings for the AVE ethernet controller found on Socionext's
UniPhier platforms.
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
.../bindings/net/socionext,uniphier-ave4.txt | 44 ++++++++++++++++++++++
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
new file mode 100644
index 0000000..57ae96d
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
@@ -0,0 +1,44 @@
+* Socionext AVE ethernet controller
+
+This describes the devicetree bindings for AVE ethernet controller
+implemented on Socionext UniPhier SoCs.
+
+Required properties:
+ - compatible: Should be "socionext,uniphier-ave4"
+ - reg: Address where registers are mapped and size of region.
+ - interrupts: IRQ common for mac and phy interrupts.
+ - phy-mode: See ethernet.txt in the same directory.
+ - #address-cells: Must be <1>.
+ - #size-cells: Must be <0>.
+ - Node, with 'reg' property, for each PHY on the MDIO bus.
+
+Optional properties:
+ - socionext,desc-bits: 32/64 descriptor size. Default 32.
+ - local-mac-address: See ethernet.txt in the same directory.
+ - pinctrl-names: List of assigned state names, see pinctrl
+ binding documentation.
+ - pinctrl-0: List of phandles to configure the GPIO pin used
+ as interrupt line, see also generic and your platform
+ specific pinctrl binding documentation.
+ - socionext,internal-phy-interrupt: Boolean to denote if the
+ PHY interrupt is internally handled by the MAC.
+
+
+Example:
+
+ eth: ethernet@65000000 {
+ compatible = "socionext,uniphier-ave4";
+ reg = <0x65000000 0x8500>;
+ interrupts = <0 66 4>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_ether_rgmii>;
+ phy-mode = "rgmii";
+ socionext,desc-bits = <64>;
+ local-mac-address = [00 00 00 00 00 00];
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ethphy: ethphy@1 {
+ reg = <1>;
+ };
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 3/3] net: phy: realtek: add RTL8201F phy-id and functions
From: Kunihiko Hayashi @ 2017-09-08 13:02 UTC (permalink / raw)
To: netdev, David S. Miller, Andrew Lunn, Florian Fainelli
Cc: Rob Herring, Mark Rutland, linux-arm-kernel, linux-kernel,
devicetree, Masahiro Yamada, Masami Hiramatsu, Jassi Brar,
Jongsung Kim, Kunihiko Hayashi
In-Reply-To: <1504875731-3680-1-git-send-email-hayashi.kunihiko@socionext.com>
From: Jassi Brar <jaswinder.singh@linaro.org>
Add RTL8201F phy-id and the related functions to the driver.
The original patch is as follows:
https://patchwork.kernel.org/patch/2538341/
Signed-off-by: Jongsung Kim <neidhard.kim@lge.com>
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
drivers/net/phy/realtek.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 9cbe645..d9974ce 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -29,10 +29,23 @@
#define RTL8211F_PAGE_SELECT 0x1f
#define RTL8211F_TX_DELAY 0x100
+#define RTL8201F_ISR 0x1e
+#define RTL8201F_PAGE_SELECT 0x1f
+#define RTL8201F_IER 0x13
+
MODULE_DESCRIPTION("Realtek PHY driver");
MODULE_AUTHOR("Johnson Leung");
MODULE_LICENSE("GPL");
+static int rtl8201_ack_interrupt(struct phy_device *phydev)
+{
+ int err;
+
+ err = phy_read(phydev, RTL8201F_ISR);
+
+ return (err < 0) ? err : 0;
+}
+
static int rtl821x_ack_interrupt(struct phy_device *phydev)
{
int err;
@@ -54,6 +67,25 @@ static int rtl8211f_ack_interrupt(struct phy_device *phydev)
return (err < 0) ? err : 0;
}
+static int rtl8201_config_intr(struct phy_device *phydev)
+{
+ int err;
+
+ /* switch to page 7 */
+ phy_write(phydev, RTL8201F_PAGE_SELECT, 0x7);
+
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+ err = phy_write(phydev, RTL8201F_IER,
+ BIT(13) | BIT(12) | BIT(11));
+ else
+ err = phy_write(phydev, RTL8201F_IER, 0);
+
+ /* restore to default page 0 */
+ phy_write(phydev, RTL8201F_PAGE_SELECT, 0x0);
+
+ return err;
+}
+
static int rtl8211b_config_intr(struct phy_device *phydev)
{
int err;
@@ -129,6 +161,18 @@ static struct phy_driver realtek_drvs[] = {
.config_aneg = &genphy_config_aneg,
.read_status = &genphy_read_status,
}, {
+ .phy_id = 0x001cc816,
+ .name = "RTL8201F 10/100Mbps Ethernet",
+ .phy_id_mask = 0x001fffff,
+ .features = PHY_BASIC_FEATURES,
+ .flags = PHY_HAS_INTERRUPT,
+ .config_aneg = &genphy_config_aneg,
+ .read_status = &genphy_read_status,
+ .ack_interrupt = &rtl8201_ack_interrupt,
+ .config_intr = &rtl8201_config_intr,
+ .suspend = genphy_suspend,
+ .resume = genphy_resume,
+ }, {
.phy_id = 0x001cc912,
.name = "RTL8211B Gigabit Ethernet",
.phy_id_mask = 0x001fffff,
@@ -181,6 +225,7 @@ static struct phy_driver realtek_drvs[] = {
module_phy_driver(realtek_drvs);
static struct mdio_device_id __maybe_unused realtek_tbl[] = {
+ { 0x001cc816, 0x001fffff },
{ 0x001cc912, 0x001fffff },
{ 0x001cc914, 0x001fffff },
{ 0x001cc915, 0x001fffff },
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 2/3] net: ethernet: socionext: add AVE ethernet driver
From: Kunihiko Hayashi @ 2017-09-08 13:02 UTC (permalink / raw)
To: netdev, David S. Miller, Andrew Lunn, Florian Fainelli
Cc: Rob Herring, Mark Rutland, linux-arm-kernel, linux-kernel,
devicetree, Masahiro Yamada, Masami Hiramatsu, Jassi Brar,
Kunihiko Hayashi
In-Reply-To: <1504875731-3680-1-git-send-email-hayashi.kunihiko@socionext.com>
The UniPhier platform from Socionext provides the AVE ethernet
controller that includes MAC and MDIO bus supporting RGMII/RMII
modes. The controller is named AVE.
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
drivers/net/ethernet/Kconfig | 1 +
drivers/net/ethernet/Makefile | 1 +
drivers/net/ethernet/socionext/Kconfig | 22 +
drivers/net/ethernet/socionext/Makefile | 4 +
drivers/net/ethernet/socionext/sni_ave.c | 1618 ++++++++++++++++++++++++++++++
5 files changed, 1646 insertions(+)
create mode 100644 drivers/net/ethernet/socionext/Kconfig
create mode 100644 drivers/net/ethernet/socionext/Makefile
create mode 100644 drivers/net/ethernet/socionext/sni_ave.c
diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index c604213..d50519e 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -170,6 +170,7 @@ source "drivers/net/ethernet/sis/Kconfig"
source "drivers/net/ethernet/sfc/Kconfig"
source "drivers/net/ethernet/sgi/Kconfig"
source "drivers/net/ethernet/smsc/Kconfig"
+source "drivers/net/ethernet/socionext/Kconfig"
source "drivers/net/ethernet/stmicro/Kconfig"
source "drivers/net/ethernet/sun/Kconfig"
source "drivers/net/ethernet/tehuti/Kconfig"
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index a0a03d4..9f55b36 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_SFC) += sfc/
obj-$(CONFIG_SFC_FALCON) += sfc/falcon/
obj-$(CONFIG_NET_VENDOR_SGI) += sgi/
obj-$(CONFIG_NET_VENDOR_SMSC) += smsc/
+obj-$(CONFIG_NET_VENDOR_SOCIONEXT) += socionext/
obj-$(CONFIG_NET_VENDOR_STMICRO) += stmicro/
obj-$(CONFIG_NET_VENDOR_SUN) += sun/
obj-$(CONFIG_NET_VENDOR_TEHUTI) += tehuti/
diff --git a/drivers/net/ethernet/socionext/Kconfig b/drivers/net/ethernet/socionext/Kconfig
new file mode 100644
index 0000000..788f26f
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Kconfig
@@ -0,0 +1,22 @@
+config NET_VENDOR_SOCIONEXT
+ bool "Socionext ethernet drivers"
+ default y
+ ---help---
+ Option to select ethernet drivers for Socionext platforms.
+
+ Note that the answer to this question doesn't directly affect the
+ kernel: saying N will just cause the configurator to skip all
+ the questions about Agere devices. If you say Y, you will be asked
+ for your specific card in the following questions.
+
+if NET_VENDOR_SOCIONEXT
+
+config SNI_AVE
+ tristate "Socionext AVE ethernet support"
+ depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF
+ select PHYLIB
+ ---help---
+ Driver for gigabit ethernet MACs, called AVE, in the
+ Socionext UniPhier family.
+
+endif #NET_VENDOR_SOCIONEXT
diff --git a/drivers/net/ethernet/socionext/Makefile b/drivers/net/ethernet/socionext/Makefile
new file mode 100644
index 0000000..0356341
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for all ethernet ip drivers on Socionext platforms
+#
+obj-$(CONFIG_SNI_AVE) += sni_ave.o
diff --git a/drivers/net/ethernet/socionext/sni_ave.c b/drivers/net/ethernet/socionext/sni_ave.c
new file mode 100644
index 0000000..c870777
--- /dev/null
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -0,0 +1,1618 @@
+/**
+ * sni_ave.c - Socionext UniPhier AVE ethernet driver
+ *
+ * Copyright 2014 Panasonic Corporation
+ * Copyright 2015-2017 Socionext Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 of
+ * the License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/etherdevice.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/mii.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/of_net.h>
+#include <linux/of_mdio.h>
+#include <linux/of_platform.h>
+#include <linux/phy.h>
+#include <linux/reset.h>
+#include <linux/types.h>
+
+/* General Register Group */
+#define AVE_IDR 0x0 /* ID */
+#define AVE_VR 0x4 /* Version */
+#define AVE_GRR 0x8 /* Global Reset */
+#define AVE_CFGR 0xc /* Configuration */
+
+/* Interrupt Register Group */
+#define AVE_GIMR 0x100 /* Global Interrupt Mask */
+#define AVE_GISR 0x104 /* Global Interrupt Status */
+
+/* MAC Register Group */
+#define AVE_TXCR 0x200 /* TX Setup */
+#define AVE_RXCR 0x204 /* RX Setup */
+#define AVE_RXMAC1R 0x208 /* MAC address (lower) */
+#define AVE_RXMAC2R 0x20c /* MAC address (upper) */
+#define AVE_MDIOCTR 0x214 /* MDIO Control */
+#define AVE_MDIOAR 0x218 /* MDIO Address */
+#define AVE_MDIOWDR 0x21c /* MDIO Data */
+#define AVE_MDIOSR 0x220 /* MDIO Status */
+#define AVE_MDIORDR 0x224 /* MDIO Rd Data */
+
+/* Descriptor Control Register Group */
+#define AVE_DESCC 0x300 /* Descriptor Control */
+#define AVE_TXDC 0x304 /* TX Descriptor Configuration */
+#define AVE_RXDC0 0x308 /* RX Descriptor Ring0 Configuration */
+#define AVE_IIRQC 0x34c /* Interval IRQ Control */
+
+/* Frame Counter Register Group */
+#define AVE_BFCR 0x400 /* Bad Frame Counter */
+#define AVE_RX0OVFFC 0x414 /* OVF Frame Counter (Ring0) */
+#define AVE_SN5FC 0x438 /* Frame Counter No5 */
+#define AVE_SN6FC 0x43c /* Frame Counter No6 */
+#define AVE_SN7FC 0x440 /* Frame Counter No7 */
+
+/* Packet Filter Register Group */
+#define AVE_PKTF_BASE 0x800 /* PF Base Address */
+#define AVE_PFMBYTE_BASE 0xd00 /* PF Mask Byte Base Address */
+#define AVE_PFMBIT_BASE 0xe00 /* PF Mask Bit Base Address */
+#define AVE_PFSEL_BASE 0xf00 /* PF Selector Base Address */
+#define AVE_PFEN 0xffc /* Packet Filter Enable */
+#define AVE_PKTF(ent) (AVE_PKTF_BASE + (ent) * 0x40)
+#define AVE_PFMBYTE(ent) (AVE_PFMBYTE_BASE + (ent) * 8)
+#define AVE_PFMBIT(ent) (AVE_PFMBIT_BASE + (ent) * 4)
+#define AVE_PFSEL(ent) (AVE_PFSEL_BASE + (ent) * 4)
+
+/* 64bit descriptor memory */
+#define AVE_DESC_SIZE_64 12 /* Descriptor Size */
+
+#define AVE_TXDM_64 0x1000 /* Tx Descriptor Memory */
+#define AVE_RXDM_64 0x1c00 /* Rx Descriptor Memory */
+
+#define AVE_TXDM_SIZE_64 0x0ba0 /* Tx Descriptor Memory Size 3KB */
+#define AVE_RXDM_SIZE_64 0x6000 /* Rx Descriptor Memory Size 24KB */
+
+/* 32bit descriptor memory */
+#define AVE_DESC_SIZE_32 8 /* Descriptor Size */
+
+#define AVE_TXDM_32 0x1000 /* Tx Descriptor Memory */
+#define AVE_RXDM_32 0x1800 /* Rx Descriptor Memory */
+
+#define AVE_TXDM_SIZE_32 0x07c0 /* Tx Descriptor Memory Size 2KB */
+#define AVE_RXDM_SIZE_32 0x4000 /* Rx Descriptor Memory Size 16KB */
+
+/* RMII Bridge Register Group */
+#define AVE_RSTCTRL 0x8028 /* Reset control */
+#define AVE_RSTCTRL_RMIIRST BIT(16)
+#define AVE_LINKSEL 0x8034 /* Link speed setting */
+#define AVE_LINKSEL_100M BIT(0)
+
+/* AVE_GRR */
+#define AVE_GRR_RXFFR BIT(5) /* Reset RxFIFO */
+#define AVE_GRR_PHYRST BIT(4) /* Reset external PHY */
+#define AVE_GRR_GRST BIT(0) /* Reset all MAC */
+
+/* AVE_GISR (common with GIMR) */
+#define AVE_GI_PHY BIT(24) /* PHY interrupt */
+#define AVE_GI_TX BIT(16) /* Tx complete */
+#define AVE_GI_RXERR BIT(8) /* Receive frame more than max size */
+#define AVE_GI_RXOVF BIT(7) /* Overflow at the RxFIFO */
+#define AVE_GI_RXDROP BIT(6) /* Drop packet */
+#define AVE_GI_RXIINT BIT(5) /* Interval interrupt */
+
+/* AVE_TXCR */
+#define AVE_TXCR_FLOCTR BIT(18) /* Flow control */
+#define AVE_TXCR_TXSPD_1G BIT(17)
+#define AVE_TXCR_TXSPD_100 BIT(16)
+
+/* AVE_RXCR */
+#define AVE_RXCR_RXEN BIT(30) /* Rx enable */
+#define AVE_RXCR_FDUPEN BIT(22) /* Interface mode */
+#define AVE_RXCR_FLOCTR BIT(21) /* Flow control */
+#define AVE_RXCR_AFEN BIT(19) /* MAC address filter */
+#define AVE_RXCR_DRPEN BIT(18) /* Drop pause frame */
+#define AVE_RXCR_MPSIZ_MASK GENMASK(10, 0)
+
+/* AVE_MDIOCTR */
+#define AVE_MDIOCTR_RREQ BIT(3) /* Read request */
+#define AVE_MDIOCTR_WREQ BIT(2) /* Write request */
+
+/* AVE_MDIOSR */
+#define AVE_MDIOSR_STS BIT(0) /* access status */
+
+/* AVE_DESCC */
+#define AVE_DESCC_TD BIT(0) /* Enable Tx descriptor */
+#define AVE_DESCC_RDSTP BIT(4) /* Pause Rx descriptor */
+#define AVE_DESCC_RD0 BIT(8) /* Enable Rx descriptor Ring0 */
+
+#define AVE_DESCC_CTRL_MASK GENMASK(15, 0)
+
+/* AVE_TXDC */
+#define AVE_TXDC_SIZE GENMASK(27, 16) /* Size of Tx descriptor */
+#define AVE_TXDC_ADDR GENMASK(11, 0) /* Start address */
+#define AVE_TXDC_ADDR_START 0
+
+/* AVE_RXDC0 */
+#define AVE_RXDC0_SIZE GENMASK(30, 16) /* Size of Rx descriptor */
+#define AVE_RXDC0_ADDR GENMASK(14, 0) /* Start address */
+#define AVE_RXDC0_ADDR_START 0
+
+/* AVE_IIRQC */
+#define AVE_IIRQC_EN0 BIT(27) /* Enable interval interrupt Ring0 */
+#define AVE_IIRQC_BSCK GENMASK(15, 0) /* Interval count unit */
+
+/* Command status for Descriptor */
+#define AVE_STS_OWN BIT(31) /* Descriptor ownership */
+#define AVE_STS_INTR BIT(29) /* Request for interrupt */
+#define AVE_STS_OK BIT(27) /* Normal transmit */
+/* TX */
+#define AVE_STS_NOCSUM BIT(28) /* No use HW checksum */
+#define AVE_STS_1ST BIT(26) /* Head of buffer chain */
+#define AVE_STS_LAST BIT(25) /* Tail of buffer chain */
+#define AVE_STS_OWC BIT(21) /* Out of window,Late Collision */
+#define AVE_STS_EC BIT(20) /* Excess collision occurred */
+#define AVE_STS_PKTLEN_TX GENMASK(15, 0)
+/* RX */
+#define AVE_STS_CSSV BIT(21) /* Checksum check performed */
+#define AVE_STS_CSER BIT(20) /* Checksum error detected */
+#define AVE_STS_PKTLEN_RX GENMASK(10, 0)
+
+/* AVE_CFGR */
+#define AVE_CFGR_FLE BIT(31) /* Filter Function */
+#define AVE_CFGR_CHE BIT(30) /* Checksum Function */
+#define AVE_CFGR_MII BIT(27) /* Func mode (1:MII/RMII, 0:RGMII) */
+#define AVE_CFGR_IPFCEN BIT(24) /* IP fragment sum Enable */
+
+#define AVE_MAX_ETHFRAME 1518
+
+/* Packet filter size */
+#define AVE_PF_SIZE 17 /* Number of all packet filter */
+#define AVE_PF_MULTICAST_SIZE 7 /* Number of multicast filter */
+
+/* Packet filter definition */
+#define AVE_PFNUM_FILTER 0 /* No.0 */
+#define AVE_PFNUM_UNICAST 1 /* No.1 */
+#define AVE_PFNUM_BROADCAST 2 /* No.2 */
+#define AVE_PFNUM_MULTICAST 11 /* No.11-17 */
+
+/* NETIF Message control */
+#define AVE_DEFAULT_MSG_ENABLE (NETIF_MSG_DRV | \
+ NETIF_MSG_PROBE | \
+ NETIF_MSG_LINK | \
+ NETIF_MSG_TIMER | \
+ NETIF_MSG_IFDOWN | \
+ NETIF_MSG_IFUP | \
+ NETIF_MSG_RX_ERR | \
+ NETIF_MSG_TX_ERR)
+
+/* Parameter for descriptor */
+#define AVE_NR_TXDESC 32 /* Tx descriptor */
+#define AVE_NR_RXDESC 64 /* Rx descriptor */
+
+/* Parameter for interrupt */
+#define AVE_INTM_COUNT 20
+#define AVE_FORCE_TXINTCNT 1
+
+#define IS_DESC_64BIT(p) ((p)->desc_size == AVE_DESC_SIZE_64)
+
+enum desc_id {
+ AVE_DESCID_TX = 0,
+ AVE_DESCID_RX,
+};
+
+enum desc_state {
+ AVE_DESC_STOP = 0,
+ AVE_DESC_START,
+ AVE_DESC_RX_SUSPEND,
+ AVE_DESC_RX_PERMIT,
+};
+
+struct ave_desc {
+ struct sk_buff *skbs;
+ dma_addr_t skbs_dma;
+ size_t skbs_dmalen;
+};
+
+struct ave_desc_info {
+ u32 ndesc; /* number of descriptor */
+ u32 daddr; /* start address of descriptor */
+ u32 proc_idx; /* index of processing packet */
+ u32 done_idx; /* index of processed packet */
+ struct ave_desc *desc; /* skb info related descriptor */
+};
+
+struct ave_private {
+ int phy_id;
+ unsigned int desc_size;
+ u32 msg_enable;
+ struct clk *clk;
+ phy_interface_t phy_mode;
+ struct phy_device *phydev;
+ struct mii_bus *mdio;
+ struct net_device_stats stats;
+ bool internal_phy_interrupt;
+
+ /* NAPI support */
+ struct net_device *ndev;
+ struct napi_struct napi_rx;
+ struct napi_struct napi_tx;
+
+ /* descriptor */
+ struct ave_desc_info rx;
+ struct ave_desc_info tx;
+};
+
+static inline u32 ave_r32(struct net_device *ndev, u32 offset)
+{
+ void __iomem *addr = (void __iomem *)ndev->base_addr;
+
+ return readl_relaxed(addr + offset);
+}
+
+static inline void ave_w32(struct net_device *ndev, u32 offset, u32 value)
+{
+ void __iomem *addr = (void __iomem *)ndev->base_addr;
+
+ writel_relaxed(value, addr + offset);
+}
+
+static inline u32 ave_rdesc(struct net_device *ndev, enum desc_id id,
+ int entry, int offset)
+{
+ struct ave_private *priv = netdev_priv(ndev);
+ u32 addr = (id == AVE_DESCID_TX) ? priv->tx.daddr : priv->rx.daddr;
+
+ addr += entry * priv->desc_size + offset;
+
+ return ave_r32(ndev, addr);
+}
+
+static inline void ave_wdesc(struct net_device *ndev, enum desc_id id,
+ int entry, int offset, u32 val)
+{
+ struct ave_private *priv = netdev_priv(ndev);
+ u32 addr = (id == AVE_DESCID_TX) ? priv->tx.daddr : priv->rx.daddr;
+
+ addr += entry * priv->desc_size + offset;
+
+ ave_w32(ndev, addr, val);
+}
+
+static void ave_wdesc_addr(struct net_device *ndev, enum desc_id id,
+ int entry, int offset, dma_addr_t paddr)
+{
+ struct ave_private *priv = netdev_priv(ndev);
+
+ ave_wdesc(ndev, id, entry, offset, (u32)((u64)paddr & 0xFFFFFFFFULL));
+ if (IS_DESC_64BIT(priv))
+ ave_wdesc(ndev, id,
+ entry, offset + 4, (u32)((u64)paddr >> 32));
+ else if ((u64)paddr > (u64)U32_MAX)
+ netdev_warn(ndev, "DMA address exceeds the address space\n");
+}
+
+static void ave_get_fwversion(struct net_device *ndev, char *buf, int len)
+{
+ u32 major, minor;
+
+ major = (ave_r32(ndev, AVE_VR) & GENMASK(15, 8)) >> 8;
+ minor = (ave_r32(ndev, AVE_VR) & GENMASK(7, 0));
+ snprintf(buf, len, "v%u.%u", major, minor);
+}
+
+static void ave_get_drvinfo(struct net_device *ndev,
+ struct ethtool_drvinfo *info)
+{
+ struct device *dev = ndev->dev.parent;
+
+ strlcpy(info->driver, dev->driver->name, sizeof(info->driver));
+ strlcpy(info->bus_info, dev_name(dev), sizeof(info->bus_info));
+ ave_get_fwversion(ndev, info->fw_version, sizeof(info->fw_version));
+}
+
+static int ave_nway_reset(struct net_device *ndev)
+{
+ return genphy_restart_aneg(ndev->phydev);
+}
+
+static u32 ave_get_link(struct net_device *ndev)
+{
+ int err;
+
+ err = genphy_update_link(ndev->phydev);
+ if (err)
+ return ethtool_op_get_link(ndev);
+
+ return ndev->phydev->link;
+}
+
+static u32 ave_get_msglevel(struct net_device *ndev)
+{
+ struct ave_private *priv = netdev_priv(ndev);
+
+ return priv->msg_enable;
+}
+
+static void ave_set_msglevel(struct net_device *ndev, u32 val)
+{
+ struct ave_private *priv = netdev_priv(ndev);
+
+ priv->msg_enable = val;
+}
+
+static void ave_get_wol(struct net_device *ndev,
+ struct ethtool_wolinfo *wol)
+{
+ wol->supported = 0;
+ wol->wolopts = 0;
+ phy_ethtool_get_wol(ndev->phydev, wol);
+}
+
+static int ave_set_wol(struct net_device *ndev,
+ struct ethtool_wolinfo *wol)
+{
+ if (wol->wolopts & (WAKE_ARP | WAKE_MAGICSECURE))
+ return -EOPNOTSUPP;
+
+ return phy_ethtool_set_wol(ndev->phydev, wol);
+}
+
+static const struct ethtool_ops ave_ethtool_ops = {
+ .get_link_ksettings = phy_ethtool_get_link_ksettings,
+ .set_link_ksettings = phy_ethtool_set_link_ksettings,
+ .get_drvinfo = ave_get_drvinfo,
+ .nway_reset = ave_nway_reset,
+ .get_link = ave_get_link,
+ .get_msglevel = ave_get_msglevel,
+ .set_msglevel = ave_set_msglevel,
+ .get_wol = ave_get_wol,
+ .set_wol = ave_set_wol,
+};
+
+static int ave_pfsel_start(struct net_device *ndev, unsigned int entry)
+{
+ u32 val;
+
+ if (WARN_ON(entry > AVE_PF_SIZE))
+ return -EINVAL;
+
+ val = ave_r32(ndev, AVE_PFEN);
+ ave_w32(ndev, AVE_PFEN, val | BIT(entry));
+
+ return 0;
+}
+
+static int ave_pfsel_stop(struct net_device *ndev, unsigned int entry)
+{
+ u32 val;
+
+ if (WARN_ON(entry > AVE_PF_SIZE))
+ return -EINVAL;
+
+ val = ave_r32(ndev, AVE_PFEN);
+ ave_w32(ndev, AVE_PFEN, val & ~BIT(entry));
+
+ return 0;
+}
+
+static int ave_pfsel_macaddr_set(struct net_device *ndev,
+ unsigned int entry, unsigned char *mac_addr,
+ unsigned int set_size)
+{
+ u32 val;
+
+ if (WARN_ON(entry > AVE_PF_SIZE))
+ return -EINVAL;
+ if (WARN_ON(set_size > 6))
+ return -EINVAL;
+
+ ave_pfsel_stop(ndev, entry);
+
+ /* set MAC address for the filter */
+ val = le32_to_cpu(((u32 *)mac_addr)[0]);
+ ave_w32(ndev, AVE_PKTF(entry), val);
+ val = (u32)le16_to_cpu(((u16 *)mac_addr)[2]);
+ ave_w32(ndev, AVE_PKTF(entry) + 4, val);
+
+ /* set byte mask */
+ ave_w32(ndev, AVE_PFMBYTE(entry), GENMASK(31, set_size) & ~0xc0);
+ ave_w32(ndev, AVE_PFMBYTE(entry) + 4, 0xFFFFFFFF);
+
+ /* set bit mask filter */
+ ave_w32(ndev, AVE_PFMBIT(entry), 0x0000FFFF);
+
+ /* set selector to ring 0 */
+ ave_w32(ndev, AVE_PFSEL(entry), 0);
+
+ /* restart filter */
+ ave_pfsel_start(ndev, entry);
+
+ return 0;
+}
+
+static int ave_mdio_busywait(struct net_device *ndev)
+{
+ int ret = 1, loop = 100;
+ u32 mdiosr;
+
+ /* wait until completion */
+ while (1) {
+ mdiosr = ave_r32(ndev, AVE_MDIOSR);
+ if (!(mdiosr & AVE_MDIOSR_STS))
+ break;
+
+ usleep_range(10, 20);
+ if (!loop--) {
+ netdev_err(ndev,
+ "failed to read from MDIO (status:0x%08x)\n",
+ mdiosr);
+ ret = 0;
+ break;
+ }
+ }
+
+ return ret;
+}
+
+static int ave_mdiobus_read(struct mii_bus *bus, int phyid, int regnum)
+{
+ struct net_device *ndev = bus->priv;
+ u32 mdioctl;
+
+ /* write address */
+ ave_w32(ndev, AVE_MDIOAR, (phyid << 8) | regnum);
+
+ /* read request */
+ mdioctl = ave_r32(ndev, AVE_MDIOCTR);
+ ave_w32(ndev, AVE_MDIOCTR, mdioctl | AVE_MDIOCTR_RREQ);
+
+ if (!ave_mdio_busywait(ndev)) {
+ netdev_err(ndev, "phy-%d reg-%x read failed\n",
+ phyid, regnum);
+ return 0xffff;
+ }
+
+ return ave_r32(ndev, AVE_MDIORDR) & GENMASK(15, 0);
+}
+
+static int ave_mdiobus_write(struct mii_bus *bus,
+ int phyid, int regnum, u16 val)
+{
+ struct net_device *ndev = bus->priv;
+ u32 mdioctl;
+
+ /* write address */
+ ave_w32(ndev, AVE_MDIOAR, (phyid << 8) | regnum);
+
+ /* write data */
+ ave_w32(ndev, AVE_MDIOWDR, val);
+
+ /* write request */
+ mdioctl = ave_r32(ndev, AVE_MDIOCTR);
+ ave_w32(ndev, AVE_MDIOCTR, mdioctl | AVE_MDIOCTR_WREQ);
+
+ if (!ave_mdio_busywait(ndev)) {
+ netdev_err(ndev, "phy-%d reg-%x write failed\n",
+ phyid, regnum);
+ return -1;
+ }
+
+ return 0;
+}
+
+static dma_addr_t ave_dma_map(struct net_device *ndev, struct ave_desc *desc,
+ void *ptr, size_t len,
+ enum dma_data_direction dir)
+{
+ dma_addr_t paddr;
+
+ paddr = dma_map_single(ndev->dev.parent, ptr, len, dir);
+ if (dma_mapping_error(ndev->dev.parent, paddr)) {
+ desc->skbs_dma = 0;
+ paddr = (dma_addr_t)virt_to_phys(ptr);
+ } else {
+ desc->skbs_dma = paddr;
+ desc->skbs_dmalen = len;
+ }
+
+ return paddr;
+}
+
+static void ave_dma_unmap(struct net_device *ndev, struct ave_desc *desc,
+ enum dma_data_direction dir)
+{
+ if (!desc->skbs_dma)
+ return;
+
+ dma_unmap_single(ndev->dev.parent,
+ desc->skbs_dma, desc->skbs_dmalen, dir);
+ desc->skbs_dma = 0;
+}
+
+/* Set Rx descriptor and memory */
+static int ave_set_rxdesc(struct net_device *ndev, int entry)
+{
+ struct ave_private *priv = netdev_priv(ndev);
+ struct sk_buff *skb;
+ unsigned long align;
+ dma_addr_t paddr;
+ void *buffptr;
+ int ret = 0;
+
+ skb = priv->rx.desc[entry].skbs;
+ if (!skb) {
+ skb = netdev_alloc_skb_ip_align(ndev,
+ AVE_MAX_ETHFRAME + NET_SKB_PAD);
+ if (!skb) {
+ netdev_err(ndev, "can't allocate skb for Rx\n");
+ return -ENOMEM;
+ }
+ }
+
+ /* set disable to cmdsts */
+ ave_wdesc(ndev, AVE_DESCID_RX, entry, 0, AVE_STS_INTR | AVE_STS_OWN);
+
+ /* align skb data for cache size */
+ align = (unsigned long)skb_tail_pointer(skb) & (NET_SKB_PAD - 1);
+ align = NET_SKB_PAD - align;
+ skb_reserve(skb, align);
+ buffptr = (void *)skb_tail_pointer(skb);
+
+ paddr = ave_dma_map(ndev, &priv->rx.desc[entry], buffptr,
+ AVE_MAX_ETHFRAME + NET_IP_ALIGN, DMA_FROM_DEVICE);
+ priv->rx.desc[entry].skbs = skb;
+
+ /* set buffer pointer */
+ ave_wdesc_addr(ndev, AVE_DESCID_RX, entry, 4, paddr);
+
+ /* set enable to cmdsts */
+ ave_wdesc(ndev, AVE_DESCID_RX,
+ entry, 0, AVE_STS_INTR | AVE_MAX_ETHFRAME);
+
+ return ret;
+}
+
+/* Switch state of descriptor */
+static int ave_desc_switch(struct net_device *ndev, enum desc_state state)
+{
+ int counter;
+ u32 val;
+
+ switch (state) {
+ case AVE_DESC_START:
+ ave_w32(ndev, AVE_DESCC, AVE_DESCC_TD | AVE_DESCC_RD0);
+ break;
+
+ case AVE_DESC_STOP:
+ ave_w32(ndev, AVE_DESCC, 0);
+ counter = 0;
+ while (1) {
+ usleep_range(100, 150);
+ if (!ave_r32(ndev, AVE_DESCC))
+ break;
+
+ counter++;
+ if (counter > 100) {
+ netdev_err(ndev, "can't stop descriptor\n");
+ return -EBUSY;
+ }
+ }
+ break;
+
+ case AVE_DESC_RX_SUSPEND:
+ val = ave_r32(ndev, AVE_DESCC);
+ val |= AVE_DESCC_RDSTP;
+ val &= AVE_DESCC_CTRL_MASK;
+ ave_w32(ndev, AVE_DESCC, val);
+
+ counter = 0;
+ while (1) {
+ usleep_range(100, 150);
+ val = ave_r32(ndev, AVE_DESCC);
+ if (val & (AVE_DESCC_RDSTP << 16))
+ break;
+
+ counter++;
+ if (counter > 1000) {
+ netdev_err(ndev, "can't suspend descriptor\n");
+ return -EBUSY;
+ }
+ }
+ break;
+
+ case AVE_DESC_RX_PERMIT:
+ val = ave_r32(ndev, AVE_DESCC);
+ val &= ~AVE_DESCC_RDSTP;
+ val &= AVE_DESCC_CTRL_MASK;
+ ave_w32(ndev, AVE_DESCC, val);
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int ave_tx_completion(struct net_device *ndev)
+{
+ struct ave_private *priv = netdev_priv(ndev);
+ u32 proc_idx, done_idx, ndesc, val;
+ int freebuf_flag = 0;
+
+ proc_idx = priv->tx.proc_idx;
+ done_idx = priv->tx.done_idx;
+ ndesc = priv->tx.ndesc;
+
+ /* free pre stored skb from done to proc */
+ while (proc_idx != done_idx) {
+ /* get cmdsts */
+ val = ave_rdesc(ndev, AVE_DESCID_TX, done_idx, 0);
+
+ /* do nothing if owner is HW */
+ if (val & AVE_STS_OWN)
+ break;
+
+ /* check Tx status and updates statistics */
+ if (val & AVE_STS_OK) {
+ priv->stats.tx_bytes += val & AVE_STS_PKTLEN_TX;
+
+ /* success */
+ if (val & AVE_STS_LAST)
+ priv->stats.tx_packets++;
+ } else {
+ /* error */
+ if (val & AVE_STS_LAST) {
+ priv->stats.tx_errors++;
+ if (val & (AVE_STS_OWC | AVE_STS_EC))
+ priv->stats.collisions++;
+ }
+ }
+
+ /* release skb */
+ if (priv->tx.desc[done_idx].skbs) {
+ ave_dma_unmap(ndev, &priv->tx.desc[done_idx],
+ DMA_TO_DEVICE);
+ dev_consume_skb_any(priv->tx.desc[done_idx].skbs);
+ priv->tx.desc[done_idx].skbs = NULL;
+ freebuf_flag++;
+ }
+ done_idx = (done_idx + 1) % ndesc;
+ }
+
+ priv->tx.done_idx = done_idx;
+
+ /* wake queue for freeing buffer */
+ if (netif_queue_stopped(ndev))
+ if (netif_running(ndev))
+ if (freebuf_flag)
+ netif_wake_queue(ndev);
+
+ return freebuf_flag;
+}
+
+static int ave_rx(struct net_device *ndev, int num)
+{
+ struct ave_private *priv = netdev_priv(ndev);
+ struct sk_buff *skb;
+ u32 proc_idx, done_idx, ndesc, cmdsts;
+ int restpkt, npkts;
+ unsigned int pktlen;
+
+ proc_idx = priv->rx.proc_idx;
+ done_idx = priv->rx.done_idx;
+ ndesc = priv->rx.ndesc;
+ restpkt = ((proc_idx + ndesc - 1) - done_idx) % ndesc;
+
+ for (npkts = 0; npkts < num; npkts++) {
+ /* we can't receive more packet, so fill desc quickly */
+ if (--restpkt < 0)
+ break;
+
+ cmdsts = ave_rdesc(ndev, AVE_DESCID_RX, proc_idx, 0);
+
+ /* owner is hardware */
+ if ((cmdsts & AVE_STS_OWN) != AVE_STS_OWN)
+ break;
+
+ if ((cmdsts & AVE_STS_OK) != AVE_STS_OK) {
+ priv->stats.rx_errors++;
+ proc_idx = (proc_idx + 1) % ndesc;
+ continue;
+ }
+
+ pktlen = cmdsts & AVE_STS_PKTLEN_RX;
+
+ /* get skbuff for rx */
+ skb = priv->rx.desc[proc_idx].skbs;
+ priv->rx.desc[proc_idx].skbs = NULL;
+
+ ave_dma_unmap(ndev, &priv->rx.desc[proc_idx], DMA_FROM_DEVICE);
+
+ skb->dev = ndev;
+ skb_reserve(skb, NET_IP_ALIGN);
+ skb_put(skb, pktlen);
+
+ skb->protocol = eth_type_trans(skb, ndev);
+
+ if ((cmdsts & AVE_STS_CSSV) && (!(cmdsts & AVE_STS_CSER)))
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
+
+ /* update stats */
+ priv->stats.rx_packets++;
+ priv->stats.rx_bytes += pktlen;
+
+ netif_receive_skb(skb);
+
+ proc_idx = (proc_idx + 1) % ndesc;
+ }
+
+ priv->rx.proc_idx = proc_idx;
+
+ /* refill the Rx buffers */
+ while (proc_idx != done_idx) {
+ if (ave_set_rxdesc(ndev, done_idx))
+ break;
+ done_idx = (done_idx + 1) % ndesc;
+ }
+
+ priv->rx.done_idx = done_idx;
+
+ return npkts;
+}
+
+static inline u32 ave_irq_disable_all(struct net_device *ndev)
+{
+ u32 ret;
+
+ ret = ave_r32(ndev, AVE_GIMR);
+ ave_w32(ndev, AVE_GIMR, 0);
+
+ return ret;
+}
+
+static inline void ave_irq_restore(struct net_device *ndev, u32 val)
+{
+ ave_w32(ndev, AVE_GIMR, val);
+}
+
+static inline void ave_irq_enable(struct net_device *ndev, u32 bitflag)
+{
+ ave_w32(ndev, AVE_GIMR, ave_r32(ndev, AVE_GIMR) | bitflag);
+ ave_w32(ndev, AVE_GISR, bitflag);
+}
+
+static inline void ave_irq_disable(struct net_device *ndev, u32 bitflag)
+{
+ ave_w32(ndev, AVE_GIMR, ave_r32(ndev, AVE_GIMR) & ~bitflag);
+}
+
+static void ave_global_reset(struct net_device *ndev)
+{
+ struct ave_private *priv = netdev_priv(ndev);
+ u32 grr, descc;
+ u32 val;
+
+ grr = ave_r32(ndev, AVE_GRR);
+ if ((grr & AVE_GRR_GRST) != AVE_GRR_GRST) {
+ descc = ave_r32(ndev, AVE_DESCC);
+ if ((descc & AVE_DESCC_RD0) == AVE_DESCC_RD0)
+ /* suspend Rx descriptor */
+ ave_desc_switch(ndev, AVE_DESC_RX_SUSPEND);
+ }
+
+ /* set config register */
+ val = AVE_CFGR_FLE | AVE_CFGR_IPFCEN | AVE_CFGR_CHE;
+ if (priv->phy_mode != PHY_INTERFACE_MODE_RGMII)
+ val |= AVE_CFGR_MII;
+ ave_w32(ndev, AVE_CFGR, val);
+
+ /* reset RMII register */
+ val = ave_r32(ndev, AVE_RSTCTRL);
+ val &= ~AVE_RSTCTRL_RMIIRST;
+ ave_w32(ndev, AVE_RSTCTRL, val);
+
+ /* assert reset */
+ ave_w32(ndev, AVE_GRR, AVE_GRR_GRST | AVE_GRR_PHYRST);
+ msleep(20);
+
+ /* 1st, negate PHY reset only */
+ ave_w32(ndev, AVE_GRR, AVE_GRR_GRST);
+ msleep(40);
+
+ /* negate reset */
+ ave_w32(ndev, AVE_GRR, 0);
+ msleep(40);
+
+ /* negate RMII register */
+ val = ave_r32(ndev, AVE_RSTCTRL);
+ val |= AVE_RSTCTRL_RMIIRST;
+ ave_w32(ndev, AVE_RSTCTRL, val);
+
+ /* disable all interrupt */
+ ave_irq_disable_all(ndev);
+}
+
+static void ave_rxf_reset(struct net_device *ndev)
+{
+ struct ave_private *priv = netdev_priv(ndev);
+ u32 rxcr_org;
+
+ /* save and disable MAC receive op */
+ rxcr_org = ave_r32(ndev, AVE_RXCR);
+ ave_w32(ndev, AVE_RXCR, rxcr_org & (~AVE_RXCR_RXEN));
+
+ /* suspend Rx descriptor */
+ ave_desc_switch(ndev, AVE_DESC_RX_SUSPEND);
+
+ /* receive all packets before descriptor starts */
+ ave_rx(ndev, priv->rx.ndesc);
+
+ /* assert reset */
+ ave_w32(ndev, AVE_GRR, AVE_GRR_RXFFR);
+ usleep_range(40, 50);
+
+ /* negate reset */
+ ave_w32(ndev, AVE_GRR, 0);
+ usleep_range(10, 20);
+
+ /* negate interrupt status */
+ ave_w32(ndev, AVE_GISR, AVE_GI_RXOVF);
+
+ /* permit descriptor */
+ ave_desc_switch(ndev, AVE_DESC_RX_PERMIT);
+
+ /* restore MAC reccieve op */
+ ave_w32(ndev, AVE_RXCR, rxcr_org);
+}
+
+static irqreturn_t ave_interrupt(int irq, void *netdev)
+{
+ struct net_device *ndev = (struct net_device *)netdev;
+ struct ave_private *priv = netdev_priv(ndev);
+ u32 gimr_val, gisr_val;
+
+ gimr_val = ave_irq_disable_all(ndev);
+
+ /* get interrupt status */
+ gisr_val = ave_r32(ndev, AVE_GISR);
+
+ /* PHY */
+ if (gisr_val & AVE_GI_PHY) {
+ ave_w32(ndev, AVE_GISR, AVE_GI_PHY);
+ if (priv->internal_phy_interrupt)
+ phy_mac_interrupt(ndev->phydev, ndev->phydev->link);
+ }
+
+ /* check exceeding packet */
+ if (gisr_val & AVE_GI_RXERR) {
+ ave_w32(ndev, AVE_GISR, AVE_GI_RXERR);
+ netdev_err(ndev, "receive a packet exceeding frame buffer\n");
+ }
+
+ gisr_val &= gimr_val;
+ if (!gisr_val)
+ goto exit_isr;
+
+ /* RxFIFO overflow */
+ if (gisr_val & AVE_GI_RXOVF) {
+ priv->stats.rx_fifo_errors++;
+ ave_rxf_reset(ndev);
+ goto exit_isr;
+ }
+
+ /* Rx drop */
+ if (gisr_val & AVE_GI_RXDROP) {
+ priv->stats.rx_dropped++;
+ ave_w32(ndev, AVE_GISR, AVE_GI_RXDROP);
+ }
+
+ /* Rx interval */
+ if (gisr_val & AVE_GI_RXIINT) {
+ napi_schedule(&priv->napi_rx);
+ /* still force to disable Rx interrupt until NAPI finishes */
+ gimr_val &= ~AVE_GI_RXIINT;
+ }
+
+ /* Tx completed */
+ if (gisr_val & AVE_GI_TX) {
+ napi_schedule(&priv->napi_tx);
+ /* still force to disable Tx interrupt until NAPI finishes */
+ gimr_val &= ~AVE_GI_TX;
+ }
+
+exit_isr:
+ ave_irq_restore(ndev, gimr_val);
+
+ return IRQ_HANDLED;
+}
+
+static int ave_poll_rx(struct napi_struct *napi, int budget)
+{
+ struct ave_private *priv;
+ struct net_device *ndev;
+ int num;
+
+ priv = container_of(napi, struct ave_private, napi_rx);
+ ndev = priv->ndev;
+
+ num = ave_rx(ndev, budget);
+ if (num < budget) {
+ napi_complete_done(napi, num);
+
+ /* enable Rx interrupt when NAPI finishes */
+ ave_irq_enable(ndev, AVE_GI_RXIINT);
+ }
+
+ return num;
+}
+
+static int ave_poll_tx(struct napi_struct *napi, int budget)
+{
+ struct ave_private *priv;
+ struct net_device *ndev;
+ int num;
+
+ priv = container_of(napi, struct ave_private, napi_tx);
+ ndev = priv->ndev;
+
+ num = ave_tx_completion(ndev);
+ if (num < budget) {
+ napi_complete(napi);
+
+ /* enable Tx interrupt when NAPI finishes */
+ ave_irq_enable(ndev, AVE_GI_TX);
+ }
+
+ return num;
+}
+
+static void ave_pfsel_promisc_set(struct net_device *ndev,
+ unsigned int entry, u32 rxring)
+{
+ if (WARN_ON(entry > AVE_PF_SIZE))
+ return;
+
+ ave_pfsel_stop(ndev, entry);
+
+ /* set byte mask */
+ ave_w32(ndev, AVE_PFMBYTE(entry), 0xFFFFFFFF);
+ ave_w32(ndev, AVE_PFMBYTE(entry) + 4, 0xFFFFFFFF);
+
+ /* set bit mask filter */
+ ave_w32(ndev, AVE_PFMBIT(entry), 0x0000FFFF);
+
+ /* set selector to rxring */
+ ave_w32(ndev, AVE_PFSEL(entry), rxring);
+
+ ave_pfsel_start(ndev, entry);
+}
+
+static void ave_pfsel_init(struct net_device *ndev)
+{
+ int i;
+ unsigned char bcast_mac[ETH_ALEN];
+
+ eth_broadcast_addr(bcast_mac);
+
+ /* Stop all filter */
+ for (i = 0; i < AVE_PF_SIZE; i++)
+ ave_pfsel_stop(ndev, i);
+
+ /* promiscious entry, select ring 0 */
+ ave_pfsel_promisc_set(ndev, AVE_PFNUM_FILTER, 0);
+
+ /* unicast entry */
+ ave_pfsel_macaddr_set(ndev, AVE_PFNUM_UNICAST, ndev->dev_addr, 6);
+
+ /* broadcast entry */
+ ave_pfsel_macaddr_set(ndev, AVE_PFNUM_BROADCAST, bcast_mac, 6);
+}
+
+static void ave_adjust_link(struct net_device *ndev)
+{
+ struct ave_private *priv = netdev_priv(ndev);
+ struct phy_device *phydev = ndev->phydev;
+ u32 val, txcr, rxcr, rxcr_org;
+
+ /* set RGMII speed */
+ val = ave_r32(ndev, AVE_TXCR);
+ val &= ~(AVE_TXCR_TXSPD_100 | AVE_TXCR_TXSPD_1G);
+
+ if (priv->phy_mode == PHY_INTERFACE_MODE_RGMII &&
+ phydev->speed == SPEED_1000)
+ val |= AVE_TXCR_TXSPD_1G;
+ else if (phydev->speed == SPEED_100)
+ val |= AVE_TXCR_TXSPD_100;
+
+ ave_w32(ndev, AVE_TXCR, val);
+
+ /* set RMII speed (100M/10M only) */
+ if (priv->phy_mode != PHY_INTERFACE_MODE_RGMII) {
+ val = ave_r32(ndev, AVE_LINKSEL);
+ if (phydev->speed == SPEED_10)
+ val &= ~AVE_LINKSEL_100M;
+ else
+ val |= AVE_LINKSEL_100M;
+ ave_w32(ndev, AVE_LINKSEL, val);
+ }
+
+ /* check current RXCR/TXCR */
+ rxcr = ave_r32(ndev, AVE_RXCR);
+ txcr = ave_r32(ndev, AVE_TXCR);
+ rxcr_org = rxcr;
+
+ if (phydev->duplex)
+ rxcr |= AVE_RXCR_FDUPEN;
+ else
+ rxcr &= ~AVE_RXCR_FDUPEN;
+
+ if (phydev->pause) {
+ rxcr |= AVE_RXCR_FLOCTR;
+ txcr |= AVE_TXCR_FLOCTR;
+ } else {
+ rxcr &= ~AVE_RXCR_FLOCTR;
+ txcr &= ~AVE_TXCR_FLOCTR;
+ }
+
+ if (rxcr_org != rxcr) {
+ /* disable Rx mac */
+ ave_w32(ndev, AVE_RXCR, rxcr & ~AVE_RXCR_RXEN);
+ /* change and enable TX/Rx mac */
+ ave_w32(ndev, AVE_TXCR, txcr);
+ ave_w32(ndev, AVE_RXCR, rxcr);
+ }
+
+ if (phydev->link)
+ netif_carrier_on(ndev);
+ else
+ netif_carrier_off(ndev);
+
+ phy_print_status(phydev);
+}
+
+static int ave_init(struct net_device *ndev)
+{
+ struct ave_private *priv = netdev_priv(ndev);
+ struct device *dev = ndev->dev.parent;
+ struct device_node *phy_node, *np = dev->of_node;
+ struct phy_device *phydev;
+ const void *mac_addr;
+ u32 supported;
+
+ /* get mac address */
+ mac_addr = of_get_mac_address(np);
+ if (mac_addr)
+ ether_addr_copy(ndev->dev_addr, mac_addr);
+
+ /* if the mac address is invalid, use random mac address */
+ if (!is_valid_ether_addr(ndev->dev_addr)) {
+ eth_hw_addr_random(ndev);
+ dev_warn(dev, "Using random MAC address: %pM\n",
+ ndev->dev_addr);
+ }
+
+ /* attach PHY with MAC */
+ phy_node = of_get_next_available_child(np, NULL);
+ phydev = of_phy_connect(ndev, phy_node,
+ ave_adjust_link, 0, priv->phy_mode);
+ if (!phydev) {
+ dev_err(dev, "could not attach to PHY\n");
+ return -ENODEV;
+ }
+ of_node_put(phy_node);
+
+ priv->phydev = phydev;
+ phydev->autoneg = AUTONEG_ENABLE;
+ phydev->speed = 0;
+ phydev->duplex = 0;
+
+ dev_info(dev, "connected to %s phy with id 0x%x\n",
+ phydev->drv->name, phydev->phy_id);
+
+ if (priv->phy_mode != PHY_INTERFACE_MODE_RGMII) {
+ supported = phydev->supported;
+ phydev->supported &= ~PHY_GBIT_FEATURES;
+ phydev->supported |= supported & PHY_BASIC_FEATURES;
+ }
+
+ /* PHY interrupt stop instruction is needed because the interrupt
+ * continues to assert.
+ */
+ phy_stop_interrupts(phydev);
+
+ /* When PHY driver can't handle its interrupt directly,
+ * interrupt request always fails and polling method is used
+ * alternatively. In this case, the libphy says
+ * "libphy: uniphier-mdio: Can't get IRQ -1 (PHY)".
+ */
+ phy_start_interrupts(phydev);
+
+ phy_start_aneg(phydev);
+
+ return 0;
+}
+
+static void ave_uninit(struct net_device *ndev)
+{
+ struct ave_private *priv = netdev_priv(ndev);
+
+ phy_stop_interrupts(priv->phydev);
+ phy_disconnect(priv->phydev);
+}
+
+static int ave_open(struct net_device *ndev)
+{
+ struct ave_private *priv = netdev_priv(ndev);
+ int entry;
+ u32 val;
+
+ /* initialize Tx work */
+ priv->tx.proc_idx = 0;
+ priv->tx.done_idx = 0;
+ memset(priv->tx.desc, 0, sizeof(struct ave_desc) * priv->tx.ndesc);
+
+ /* initialize Tx descriptor */
+ for (entry = 0; entry < priv->tx.ndesc; entry++) {
+ ave_wdesc(ndev, AVE_DESCID_TX, entry, 0, 0);
+ ave_wdesc_addr(ndev, AVE_DESCID_TX, entry, 4, 0);
+ }
+ ave_w32(ndev, AVE_TXDC, AVE_TXDC_ADDR_START
+ | (((priv->tx.ndesc * priv->desc_size) << 16) & AVE_TXDC_SIZE));
+
+ /* initialize Rx work */
+ priv->rx.proc_idx = 0;
+ priv->rx.done_idx = 0;
+ memset(priv->rx.desc, 0, sizeof(struct ave_desc) * priv->rx.ndesc);
+
+ /* initialize Rx descriptor and buffer */
+ for (entry = 0; entry < priv->rx.ndesc; entry++) {
+ if (ave_set_rxdesc(ndev, entry))
+ break;
+ }
+ ave_w32(ndev, AVE_RXDC0, AVE_RXDC0_ADDR_START
+ | (((priv->rx.ndesc * priv->desc_size) << 16) & AVE_RXDC0_SIZE));
+
+ /* enable descriptor */
+ ave_desc_switch(ndev, AVE_DESC_START);
+
+ /* initialize filter */
+ ave_pfsel_init(ndev);
+
+ /* set macaddr */
+ val = le32_to_cpu(((u32 *)ndev->dev_addr)[0]);
+ ave_w32(ndev, AVE_RXMAC1R, val);
+ val = (u32)le16_to_cpu(((u16 *)ndev->dev_addr)[2]);
+ ave_w32(ndev, AVE_RXMAC2R, val);
+
+ /* set Rx configuration */
+ /* full duplex, enable pause drop, enalbe flow control */
+ val = AVE_RXCR_RXEN | AVE_RXCR_FDUPEN | AVE_RXCR_DRPEN |
+ AVE_RXCR_FLOCTR | (AVE_MAX_ETHFRAME & AVE_RXCR_MPSIZ_MASK);
+ ave_w32(ndev, AVE_RXCR, val);
+
+ /* set Tx configuration */
+ /* enable flow control, disable loopback */
+ ave_w32(ndev, AVE_TXCR, AVE_TXCR_FLOCTR);
+
+ /* enable timer, clear EN,INTM, and mask interval unit(BSCK) */
+ val = ave_r32(ndev, AVE_IIRQC) & AVE_IIRQC_BSCK;
+ val |= AVE_IIRQC_EN0 | (AVE_INTM_COUNT << 16);
+ ave_w32(ndev, AVE_IIRQC, val);
+
+ /* set interrupt mask */
+ val = AVE_GI_RXIINT | AVE_GI_RXOVF | AVE_GI_TX;
+ val |= (priv->internal_phy_interrupt) ? AVE_GI_PHY : 0;
+ ave_irq_restore(ndev, val);
+
+ napi_enable(&priv->napi_rx);
+ napi_enable(&priv->napi_tx);
+
+ phy_start(ndev->phydev);
+ netif_start_queue(ndev);
+
+ return 0;
+}
+
+static int ave_stop(struct net_device *ndev)
+{
+ struct ave_private *priv = netdev_priv(ndev);
+ int entry;
+
+ /* disable all interrupt */
+ ave_irq_disable_all(ndev);
+ disable_irq(ndev->irq);
+
+ netif_tx_disable(ndev);
+ phy_stop(ndev->phydev);
+ napi_disable(&priv->napi_tx);
+ napi_disable(&priv->napi_rx);
+
+ /* free Tx buffer */
+ for (entry = 0; entry < priv->tx.ndesc; entry++) {
+ if (!priv->tx.desc[entry].skbs)
+ continue;
+
+ ave_dma_unmap(ndev, &priv->tx.desc[entry], DMA_TO_DEVICE);
+ dev_kfree_skb_any(priv->tx.desc[entry].skbs);
+ priv->tx.desc[entry].skbs = NULL;
+ }
+ priv->tx.proc_idx = 0;
+ priv->tx.done_idx = 0;
+
+ /* free Rx buffer */
+ for (entry = 0; entry < priv->rx.ndesc; entry++) {
+ if (!priv->rx.desc[entry].skbs)
+ continue;
+
+ ave_dma_unmap(ndev, &priv->rx.desc[entry], DMA_FROM_DEVICE);
+ dev_kfree_skb_any(priv->rx.desc[entry].skbs);
+ priv->rx.desc[entry].skbs = NULL;
+ }
+ priv->rx.proc_idx = 0;
+ priv->rx.done_idx = 0;
+
+ return 0;
+}
+
+static int ave_start_xmit(struct sk_buff *skb, struct net_device *ndev)
+{
+ struct ave_private *priv = netdev_priv(ndev);
+ u32 proc_idx, done_idx, ndesc, cmdsts;
+ int freepkt;
+ unsigned char *buffptr = NULL; /* buffptr for descriptor */
+ unsigned int len;
+ dma_addr_t paddr;
+
+ proc_idx = priv->tx.proc_idx;
+ done_idx = priv->tx.done_idx;
+ ndesc = priv->tx.ndesc;
+ freepkt = ((done_idx + ndesc - 1) - proc_idx) % ndesc;
+
+ /* not enough entry, then we stop queue */
+ if (unlikely(freepkt < 2)) {
+ netif_stop_queue(ndev);
+ if (unlikely(freepkt < 1))
+ return NETDEV_TX_BUSY;
+ }
+
+ priv->tx.desc[proc_idx].skbs = skb;
+
+ /* add padding for short packet */
+ if (skb_padto(skb, ETH_ZLEN)) {
+ dev_kfree_skb_any(skb);
+ return NETDEV_TX_OK;
+ }
+
+ buffptr = skb->data - NET_IP_ALIGN;
+ len = max_t(unsigned int, ETH_ZLEN, skb->len);
+
+ paddr = ave_dma_map(ndev, &priv->tx.desc[proc_idx], buffptr,
+ len + NET_IP_ALIGN, DMA_TO_DEVICE);
+ paddr += NET_IP_ALIGN;
+
+ /* set buffer address to descriptor */
+ ave_wdesc_addr(ndev, AVE_DESCID_TX, proc_idx, 4, paddr);
+
+ /* set flag and length to send */
+ cmdsts = AVE_STS_OWN | AVE_STS_1ST | AVE_STS_LAST
+ | (len & AVE_STS_PKTLEN_TX);
+
+ /* set interrupt per AVE_FORCE_TXINTCNT or when queue is stopped */
+ if (!(proc_idx % AVE_FORCE_TXINTCNT) || netif_queue_stopped(ndev))
+ cmdsts |= AVE_STS_INTR;
+
+ /* disable checksum calculation when skb doesn't calurate checksum */
+ if (skb->ip_summed == CHECKSUM_NONE ||
+ skb->ip_summed == CHECKSUM_UNNECESSARY)
+ cmdsts |= AVE_STS_NOCSUM;
+
+ /* set cmdsts */
+ ave_wdesc(ndev, AVE_DESCID_TX, proc_idx, 0, cmdsts);
+
+ priv->tx.proc_idx = (proc_idx + 1) % ndesc;
+
+ return NETDEV_TX_OK;
+}
+
+static int ave_ioctl(struct net_device *ndev, struct ifreq *ifr, int cmd)
+{
+ return phy_mii_ioctl(ndev->phydev, ifr, cmd);
+}
+
+static void ave_set_rx_mode(struct net_device *ndev)
+{
+ int count, mc_cnt = netdev_mc_count(ndev);
+ struct netdev_hw_addr *hw_adr;
+ u32 val;
+ u8 v4multi_macadr[6] = { 0x01, 0x00, 0x00, 0x00, 0x00, 0x00 };
+ u8 v6multi_macadr[6] = { 0x33, 0x00, 0x00, 0x00, 0x00, 0x00 };
+
+ /* MAC addr filter enable for promiscious mode */
+ val = ave_r32(ndev, AVE_RXCR);
+ if (ndev->flags & IFF_PROMISC || !mc_cnt)
+ val &= ~AVE_RXCR_AFEN;
+ else
+ val |= AVE_RXCR_AFEN;
+ ave_w32(ndev, AVE_RXCR, val);
+
+ /* set all multicast address */
+ if ((ndev->flags & IFF_ALLMULTI) || (mc_cnt > AVE_PF_MULTICAST_SIZE)) {
+ ave_pfsel_macaddr_set(ndev, AVE_PFNUM_MULTICAST,
+ v4multi_macadr, 1);
+ ave_pfsel_macaddr_set(ndev, AVE_PFNUM_MULTICAST + 1,
+ v6multi_macadr, 1);
+ } else {
+ /* stop all multicast filter */
+ for (count = 0; count < AVE_PF_MULTICAST_SIZE; count++)
+ ave_pfsel_stop(ndev, AVE_PFNUM_MULTICAST + count);
+
+ /* set multicast addresses */
+ count = 0;
+ netdev_for_each_mc_addr(hw_adr, ndev) {
+ if (count == mc_cnt)
+ break;
+ ave_pfsel_macaddr_set(ndev, AVE_PFNUM_MULTICAST + count,
+ hw_adr->addr, 6);
+ count++;
+ }
+ }
+}
+
+static struct net_device_stats *ave_stats(struct net_device *ndev)
+{
+ struct ave_private *priv = netdev_priv(ndev);
+ u32 drop_num = 0;
+
+ priv->stats.rx_errors = ave_r32(ndev, AVE_BFCR);
+
+ drop_num += ave_r32(ndev, AVE_RX0OVFFC);
+ drop_num += ave_r32(ndev, AVE_SN5FC);
+ drop_num += ave_r32(ndev, AVE_SN6FC);
+ drop_num += ave_r32(ndev, AVE_SN7FC);
+ priv->stats.rx_dropped = drop_num;
+
+ return &priv->stats;
+}
+
+static int ave_set_mac_address(struct net_device *ndev, void *p)
+{
+ int ret = eth_mac_addr(ndev, p);
+ u32 val;
+
+ if (ret)
+ return ret;
+
+ /* set macaddr */
+ val = le32_to_cpu(((u32 *)ndev->dev_addr)[0]);
+ ave_w32(ndev, AVE_RXMAC1R, val);
+ val = (u32)le16_to_cpu(((u16 *)ndev->dev_addr)[2]);
+ ave_w32(ndev, AVE_RXMAC2R, val);
+
+ /* pfsel unicast entry */
+ ave_pfsel_macaddr_set(ndev, AVE_PFNUM_UNICAST, ndev->dev_addr, 6);
+
+ return 0;
+}
+
+static const struct net_device_ops ave_netdev_ops = {
+ .ndo_init = ave_init,
+ .ndo_uninit = ave_uninit,
+ .ndo_open = ave_open,
+ .ndo_stop = ave_stop,
+ .ndo_start_xmit = ave_start_xmit,
+ .ndo_do_ioctl = ave_ioctl,
+ .ndo_set_rx_mode = ave_set_rx_mode,
+ .ndo_get_stats = ave_stats,
+ .ndo_set_mac_address = ave_set_mac_address,
+};
+
+static int ave_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ u32 desc_bits, ave_id;
+ struct reset_control *rst;
+ struct ave_private *priv;
+ phy_interface_t phy_mode;
+ struct net_device *ndev;
+ struct resource *res;
+ void __iomem *base;
+ int irq, ret = 0;
+ char buf[ETHTOOL_FWVERS_LEN];
+
+ /* get phy mode */
+ phy_mode = of_get_phy_mode(np);
+ if (phy_mode < 0) {
+ dev_err(dev, "phy-mode not found\n");
+ return -EINVAL;
+ }
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(dev, "IRQ not found\n");
+ return irq;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ /* alloc netdevice */
+ ndev = alloc_etherdev(sizeof(struct ave_private));
+ if (!ndev) {
+ dev_err(dev, "can't allocate ethernet device\n");
+ return -ENOMEM;
+ }
+
+ ndev->base_addr = (unsigned long)base;
+ ndev->irq = irq;
+ ndev->netdev_ops = &ave_netdev_ops;
+ ndev->ethtool_ops = &ave_ethtool_ops;
+ SET_NETDEV_DEV(ndev, dev);
+
+ /* support hardware checksum */
+ ndev->features |= (NETIF_F_IP_CSUM | NETIF_F_RXCSUM);
+ ndev->hw_features |= (NETIF_F_IP_CSUM | NETIF_F_RXCSUM);
+
+ ndev->max_mtu = AVE_MAX_ETHFRAME - (ETH_HLEN + ETH_FCS_LEN);
+
+ priv = netdev_priv(ndev);
+ priv->ndev = ndev;
+ priv->msg_enable = netif_msg_init(-1, AVE_DEFAULT_MSG_ENABLE);
+ priv->phy_mode = phy_mode;
+
+ /* get bits of descriptor from devicetree */
+ of_property_read_u32(np, "socionext,desc-bits", &desc_bits);
+ priv->desc_size = AVE_DESC_SIZE_32;
+ if (desc_bits == 64)
+ priv->desc_size = AVE_DESC_SIZE_64;
+ else if (desc_bits != 32)
+ dev_warn(dev, "Defaulting to 32bit descriptor\n");
+
+ /* use internal phy interrupt */
+ priv->internal_phy_interrupt =
+ of_property_read_bool(np, "socionext,internal-phy-interrupt");
+
+ /* setting private data for descriptor */
+ priv->tx.daddr = IS_DESC_64BIT(priv) ? AVE_TXDM_64 : AVE_TXDM_32;
+ priv->tx.ndesc = AVE_NR_TXDESC;
+ priv->tx.desc = devm_kzalloc(dev,
+ sizeof(struct ave_desc) * priv->tx.ndesc,
+ GFP_KERNEL);
+ if (!priv->tx.desc)
+ return -ENOMEM;
+
+ priv->rx.daddr = IS_DESC_64BIT(priv) ? AVE_RXDM_64 : AVE_RXDM_32;
+ priv->rx.ndesc = AVE_NR_RXDESC;
+ priv->rx.desc = devm_kzalloc(dev,
+ sizeof(struct ave_desc) * priv->rx.ndesc,
+ GFP_KERNEL);
+ if (!priv->rx.desc)
+ return -ENOMEM;
+
+ /* enable clock */
+ priv->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(priv->clk))
+ priv->clk = NULL;
+ clk_prepare_enable(priv->clk);
+
+ /* reset block */
+ rst = devm_reset_control_get(dev, NULL);
+ if (!IS_ERR_OR_NULL(rst)) {
+ reset_control_deassert(rst);
+ reset_control_put(rst);
+ }
+
+ /* reset mac and phy */
+ ave_global_reset(ndev);
+
+ /* request interrupt */
+ ret = devm_request_irq(dev, irq, ave_interrupt,
+ IRQF_SHARED, ndev->name, ndev);
+ if (ret)
+ goto err_req_irq;
+
+ /* create MDIO bus */
+ priv->mdio = devm_mdiobus_alloc(dev);
+ if (!priv->mdio) {
+ ret = -ENOMEM;
+ goto err_mdiobus_alloc;
+ }
+
+ priv->mdio->priv = ndev;
+ priv->mdio->parent = dev;
+ priv->mdio->read = ave_mdiobus_read;
+ priv->mdio->write = ave_mdiobus_write;
+ priv->mdio->name = "uniphier-mdio";
+ snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "%s-%x",
+ pdev->name, pdev->id);
+
+ ret = of_mdiobus_register(priv->mdio, np);
+ if (ret) {
+ dev_err(dev, "of_mdiobus_register(%s) failed\n", np->full_name);
+ ret = -ENOMEM;
+ goto err_mdiobus_register;
+ }
+
+ /* Register as a NAPI supported driver */
+ netif_napi_add(ndev, &priv->napi_rx, ave_poll_rx, priv->rx.ndesc);
+ netif_tx_napi_add(ndev, &priv->napi_tx, ave_poll_tx, priv->tx.ndesc);
+
+ ret = register_netdev(ndev);
+ if (ret) {
+ dev_err(dev, "failed to register netdevice\n");
+ goto err_netdev_register;
+ }
+
+ platform_set_drvdata(pdev, ndev);
+
+ /* get ID and version */
+ ave_id = ave_r32(ndev, AVE_IDR);
+ ave_get_fwversion(ndev, buf, sizeof(buf));
+
+ dev_info(dev, "Socionext %c%c%c%c Ethernet IP %s (irq=%d, phy=%s)\n",
+ (ave_id >> 24) & 0xff, (ave_id >> 16) & 0xff,
+ (ave_id >> 8) & 0xff, (ave_id >> 0) & 0xff,
+ buf, ndev->irq, phy_modes(phy_mode));
+
+ return 0;
+
+err_netdev_register:
+ netif_napi_del(&priv->napi_rx);
+ netif_napi_del(&priv->napi_tx);
+ mdiobus_unregister(priv->mdio);
+err_mdiobus_register:
+err_mdiobus_alloc:
+err_req_irq:
+ free_netdev(ndev);
+ clk_disable_unprepare(priv->clk);
+
+ return ret;
+}
+
+static int ave_remove(struct platform_device *pdev)
+{
+ struct net_device *ndev = platform_get_drvdata(pdev);
+ struct ave_private *priv = netdev_priv(ndev);
+
+ unregister_netdev(ndev);
+ netif_napi_del(&priv->napi_rx);
+ netif_napi_del(&priv->napi_tx);
+ mdiobus_unregister(priv->mdio);
+ free_netdev(ndev);
+ clk_disable_unprepare(priv->clk);
+
+ return 0;
+}
+
+static const struct of_device_id of_ave_match[] = {
+ { .compatible = "socionext,uniphier-ave4" },
+ { /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, of_ave_match);
+
+static struct platform_driver ave_driver = {
+ .probe = ave_probe,
+ .remove = ave_remove,
+ .driver = {
+ .name = "ave",
+ .of_match_table = of_ave_match,
+ },
+};
+module_platform_driver(ave_driver);
+
+MODULE_DESCRIPTION("Socionext UniPhier AVE ethernet driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs
From: Andrew Lunn @ 2017-09-08 13:05 UTC (permalink / raw)
To: Corentin Labbe
Cc: robh+dt, mark.rutland, maxime.ripard, wens, linux,
catalin.marinas, will.deacon, peppe.cavallaro, alexandre.torgue,
f.fainelli, netdev, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20170908071156.5115-11-clabbe.montjoie@gmail.com>
> +#define DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID 0
> +#define DWMAC_sUN8I_MDIO_MUX_EXTERNAL_ID 1
>
> /* H3/A64 specific bits */
> #define SYSCON_RMII_EN BIT(13) /* 1: enable RMII (overrides EPIT) */
> @@ -634,6 +639,76 @@ static int sun8i_dwmac_reset(struct stmmac_priv *priv)
> return 0;
> }
>
> +/* MDIO multiplexing switch function
> + * This function is called by the mdio-mux layer when it thinks the mdio bus
> + * multiplexer needs to switch.
> + * 'current_child' is the current value of the mux register
> + * 'desired_child' is the value of the 'reg' property of the target child MDIO
> + * node.
> + * The first time this function is called, current_child == -1.
> + * If current_child == desired_child, then the mux is already set to the
> + * correct bus.
> + *
> + * Note that we do not use reg/mask like mdio-mux-mmioreg because we need to
> + * know easily which bus is used (reset must be done only for desired bus).
> + */
> +static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
> + void *data)
> +{
> + struct stmmac_priv *priv = data;
> + struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
> + u32 reg, val;
> + int ret = 0;
> + bool need_reset = false;
> +
> + if (current_child ^ desired_child) {
> + regmap_read(gmac->regmap, SYSCON_EMAC_REG, ®);
> + switch (desired_child) {
> + case DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID:
> + dev_info(priv->device, "Switch mux to internal PHY");
> + val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SELECT;
> + if (gmac->use_internal_phy)
> + need_reset = true;
> + break;
This i don't get. Why do you need use_internal_phy? Isn't that
implicit from DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID? Is it even possible to
use an external PHY on the internal MDIO bus?
> + case DWMAC_sUN8I_MDIO_MUX_EXTERNAL_ID:
> + dev_info(priv->device, "Switch mux to external PHY");
> + val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SHUTDOWN;
> + if (!gmac->use_internal_phy)
> + need_reset = true;
> + break;
And is it possible to use the internal PHY on the external bus?
Andrew
^ permalink raw reply
* Re: [PATCH net] bpf: don't select potentially stale ri->map from buggy xdp progs
From: Jesper Dangaard Brouer @ 2017-09-08 13:07 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: andy, davem, ast, john.fastabend, netdev, brouer
In-Reply-To: <59B28E45.40700@iogearbox.net>
On Fri, 08 Sep 2017 14:34:13 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 09/08/2017 01:52 PM, Jesper Dangaard Brouer wrote:
> > On Fri, 08 Sep 2017 12:34:28 +0200 Daniel Borkmann <daniel@iogearbox.net> wrote:
> >> On 09/08/2017 07:06 AM, Jesper Dangaard Brouer wrote:
> >>> On Fri, 8 Sep 2017 00:14:51 +0200
> >>> Daniel Borkmann <daniel@iogearbox.net> wrote:
> >>>
> >>>> + /* This is really only caused by a deliberately crappy
> >>>> + * BPF program, normally we would never hit that case,
> >>>> + * so no need to inform someone via tracepoints either,
> >>>> + * just bail out.
> >>>> + */
> >>>> + if (unlikely(map_owner != xdp_prog))
> >>>> + return -EINVAL;
> >>>
> >>> IMHO we do need to call the tracepoint here. It is not just crappy
> >>> BPF-progs that cause this situation, it is also drivers not implementing
> >>> XDP_REDIRECT yet (which is all but ixgbe). Due to the level XDP
> >>> operates at, tracepoints are the only way users can runtime troubleshoot
> >>> their XDP programs.
> >>
> >> Drivers not implementing XDP_REDIRECT don't even get there in
> >> the first place. What they will do is to hit the 'default' case
> >> when they check for the action code from the BPF program. Then
> >> call into bpf_warn_invalid_xdp_action(act), and fall-through
> >> to hit the tracepoint at trace_xdp_exception() which is also
> >> triggered by XDP_ABORTED usually. So when that happens we do
> >> complain loudly and call a tracepoint already. We should probably
> >> tweak the bpf_warn_invalid_xdp_action() message a little to make
> >> it clear that the action could also just be unsupported by the
> >> driver instead of being illegal.
> >
> > Yes. drivers not implementing XDP_REDIRECT will cause a tracepoint
> > trace_xdp_exception() to be called for its _own_ packets.
>
> Yep, plus a big one time warning for the case a user doesn't
> look at tracepoints initially.
>
> > But it will still setup and leave map and map_owner pointer dangling.
> > Another NIC can load an xdp_prog that return XDP_REDIRECT, which will hit
> > above if-statement, and its packets will disappear, without getting
> > recorded by a tracepoint (thus hard to debug!).
>
> If a user wants to reproduce this exact error, he would need
> to go and reload the program on the driver not supporting the
> XDP_REDIRECT in the first place, and then reload his buggy program
> on the other driver supporting XDP_REDIRECT but w/o having called
> bpf_xdp_redirect_map(), so exactly once on the switch from one
> driver to another with this misuse, any subsequent packets will
> trigger _trace_xdp_redirect_err(), same way as if the buggy
> program was loaded to the 2nd driver from the beginning since
> the map and ifindex etc will be zero, hence my comment on this.
We can agree that the second program that experience the side-effect is
also buggy, as just returning XDP_REDIRECT without calling
bpf_xdp_redirect_map() or bpf_xdp_redirect(), is a bug in the bpf
program. Thus, the comment about a "deliberately crappy BPF program"
is not wrong.
You don't have to load and unload xdp programs. My test is simply
having two XDP programs running. 1. xdp_redirect_map on mlx5 which
doesn't implement XDP_REDIRECT, and 2. a "deliberately crappy BPF
program" on ixgbe that just returns XDP_REDIRECT.
In below output I've used -EFAULT == -14 to capture this situation
happening:
ksoftirqd/3 28 [003] 3437.829882: xdp:xdp_redirect_err: prog_id=3 action=REDIRECT ifindex=2 to_ifindex=0 err=-22
swapper 0 [005] 3437.829882: xdp:xdp_redirect_err: prog_id=3 action=REDIRECT ifindex=2 to_ifindex=0 err=-22
ksoftirqd/0 7 [000] 3437.829882: xdp:xdp_exception: prog_id=5 action=REDIRECT ifindex=7
ksoftirqd/4 34 [004] 3437.829882: xdp:xdp_redirect_err: prog_id=3 action=REDIRECT ifindex=2 to_ifindex=0 err=-22
ksoftirqd/2 22 [002] 3437.829882: xdp:xdp_redirect_err: prog_id=3 action=REDIRECT ifindex=2 to_ifindex=0 err=-22
ksoftirqd/3 28 [003] 3437.829882: xdp:xdp_redirect_err: prog_id=3 action=REDIRECT ifindex=2 to_ifindex=0 err=-22
swapper 0 [005] 3437.829882: xdp:xdp_redirect_err: prog_id=3 action=REDIRECT ifindex=2 to_ifindex=0 err=-22
swapper 0 [005] 3437.829882: xdp:xdp_redirect_err: prog_id=3 action=REDIRECT ifindex=2 to_ifindex=0 err=-22
ksoftirqd/3 28 [003] 3437.829882: xdp:xdp_redirect_err: prog_id=3 action=REDIRECT ifindex=2 to_ifindex=0 err=-22
ksoftirqd/2 22 [002] 3437.829882: xdp:xdp_redirect_err: prog_id=3 action=REDIRECT ifindex=2 to_ifindex=0 err=-22
ksoftirqd/4 34 [004] 3437.829882: xdp:xdp_redirect_err: prog_id=3 action=REDIRECT ifindex=2 to_ifindex=0 err=-22
ksoftirqd/3 28 [003] 3437.829882: xdp:xdp_redirect_err: prog_id=3 action=REDIRECT ifindex=2 to_ifindex=0 err=-22
ksoftirqd/2 22 [002] 3437.829882: xdp:xdp_redirect_err: prog_id=3 action=REDIRECT ifindex=2 to_ifindex=0 err=-22
ksoftirqd/0 7 [000] 3437.829882: xdp:xdp_redirect_map_err: prog_id=3 action=REDIRECT ifindex=2 to_ifindex=0 err=-14 map_id=5 map_index=0
swapper 0 [005] 3437.829883: xdp:xdp_redirect_err: prog_id=3 action=REDIRECT ifindex=2 to_ifindex=0 err=-22
ksoftirqd/3 28 [003] 3437.829883: xdp:xdp_redirect_err: prog_id=3 action=REDIRECT ifindex=2 to_ifindex=0 err=-22
And I can see I made a mistake and dereference the map_id ;-)
BTW, just to make it clear, I love the rest of the patch. And I love
how you solved this. Cool trick. You also closed a hole where someone
could set the map in one bpf_prog and cause the next bpf program to
forward using this map (this could be a policy violation).
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH 2/2] netfilter/libxt_hashlimit: new feature/algorithm for xt_hashlimit
From: Pablo Neira Ayuso @ 2017-09-08 13:10 UTC (permalink / raw)
To: Vishwanath Pai; +Cc: kadlec, netfilter-devel, johunt, fw, netdev, pai.vishwain
In-Reply-To: <1503089946-15996-1-git-send-email-vpai@akamai.com>
On Fri, Aug 18, 2017 at 04:59:06PM -0400, Vishwanath Pai wrote:
> This patch adds a new feature to hashlimit that allows matching on the
> current packet/byte rate without rate limiting. This can be enabled
> with a new flag --hashlimit-rate-match. The match returns true if the
> current rate of packets is above/below the user specified value.
Applied to iptables:
http://git.netfilter.org/iptables/commit/?id=1c32e5606fdf53856cba0cd9bc7b3f8b584b2cc2
Thanks.
^ permalink raw reply
* Re: [PATCH iproute2 2/2] lib/libnetlink: update rtnl_talk to support malloc buff at run time
From: Hangbin Liu @ 2017-09-08 13:26 UTC (permalink / raw)
To: Phil Sutter, netdev, Stephen Hemminger, Michal Kubecek
In-Reply-To: <20170908110652.GU2399@orbyte.nwl.cc>
On Fri, Sep 08, 2017 at 01:06:52PM +0200, Phil Sutter wrote:
> Hi Hangbin,
>
> On Fri, Sep 08, 2017 at 06:14:57PM +0800, Hangbin Liu wrote:
> [...]
> > diff --git a/genl/ctrl.c b/genl/ctrl.c
> > index 448988e..699657b 100644
> > --- a/genl/ctrl.c
> > +++ b/genl/ctrl.c
> > @@ -55,6 +55,7 @@ int genl_ctrl_resolve_family(const char *family)
> > };
> > struct nlmsghdr *nlh = &req.n;
> > struct genlmsghdr *ghdr = &req.g;
> > + struct nlmsghdr *answer = NULL;
>
> I don't think it's necessary to assign NULL here or in any of the other
> cases.
OK
>
> > if (rtnl_open_byproto(&rth, 0, NETLINK_GENERIC) < 0) {
> > fprintf(stderr, "Cannot open generic netlink socket\n");
> > @@ -63,19 +64,19 @@ int genl_ctrl_resolve_family(const char *family)
> >
> > addattr_l(nlh, 128, CTRL_ATTR_FAMILY_NAME, family, strlen(family) + 1);
> >
> > - if (rtnl_talk(&rth, nlh, nlh, sizeof(req)) < 0) {
> > + if (rtnl_talk(&rth, nlh, &answer) < 0) {
>
> I didn't check, but it's likely that at least in some cases 'nlh'
> contains some buffer space to hold the answer (if it will be larger than
> the input). If so, this could be dropped since the buffer is no longer
> reused.
Yes, I will remove the buffer.
Thanks
Hangbin
^ permalink raw reply
* Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs
From: Corentin Labbe @ 2017-09-08 13:26 UTC (permalink / raw)
To: Andrew Lunn
Cc: robh+dt, mark.rutland, maxime.ripard, wens, linux,
catalin.marinas, will.deacon, peppe.cavallaro, alexandre.torgue,
f.fainelli, netdev, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20170908130520.GA11248@lunn.ch>
On Fri, Sep 08, 2017 at 03:05:20PM +0200, Andrew Lunn wrote:
> > +#define DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID 0
> > +#define DWMAC_sUN8I_MDIO_MUX_EXTERNAL_ID 1
> >
> > /* H3/A64 specific bits */
> > #define SYSCON_RMII_EN BIT(13) /* 1: enable RMII (overrides EPIT) */
> > @@ -634,6 +639,76 @@ static int sun8i_dwmac_reset(struct stmmac_priv *priv)
> > return 0;
> > }
> >
> > +/* MDIO multiplexing switch function
> > + * This function is called by the mdio-mux layer when it thinks the mdio bus
> > + * multiplexer needs to switch.
> > + * 'current_child' is the current value of the mux register
> > + * 'desired_child' is the value of the 'reg' property of the target child MDIO
> > + * node.
> > + * The first time this function is called, current_child == -1.
> > + * If current_child == desired_child, then the mux is already set to the
> > + * correct bus.
> > + *
> > + * Note that we do not use reg/mask like mdio-mux-mmioreg because we need to
> > + * know easily which bus is used (reset must be done only for desired bus).
> > + */
> > +static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
> > + void *data)
> > +{
> > + struct stmmac_priv *priv = data;
> > + struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
> > + u32 reg, val;
> > + int ret = 0;
> > + bool need_reset = false;
> > +
> > + if (current_child ^ desired_child) {
> > + regmap_read(gmac->regmap, SYSCON_EMAC_REG, ®);
> > + switch (desired_child) {
> > + case DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID:
> > + dev_info(priv->device, "Switch mux to internal PHY");
> > + val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SELECT;
> > + if (gmac->use_internal_phy)
> > + need_reset = true;
> > + break;
>
> This i don't get. Why do you need use_internal_phy? Isn't that
> implicit from DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID? Is it even possible to
> use an external PHY on the internal MDIO bus?
>
On my H3 box with external PHY, the MDIO mux library first select (for scan ?) the internal MDIO.
Without use_internal_phy usage, this board will launch a reset to use the internal MDIO... and this reset timeout/fail.
After the MDIO mux select the external MDIO.
> > + case DWMAC_sUN8I_MDIO_MUX_EXTERNAL_ID:
> > + dev_info(priv->device, "Switch mux to external PHY");
> > + val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SHUTDOWN;
> > + if (!gmac->use_internal_phy)
> > + need_reset = true;
> > + break;
>
> And is it possible to use the internal PHY on the external bus?
>
I need to check that.
Regards
^ permalink raw reply
* Re: [PATCH RFC 0/6] Modify KSZ9477 DSA driver in preparation to add other KSZ switch drivers.
From: Vivien Didelot @ 2017-09-08 13:30 UTC (permalink / raw)
To: Tristram.Ha, andrew, muvarov, pavel, nathan.leigh.conrad,
f.fainelli, netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD411219F8@CHN-SV-EXMX02.mchp-main.com>
Hi Tristram,
Tristram.Ha@microchip.com writes:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> This series of patches is to modify the original KSZ9477 DSA driver so that other KSZ switch drivers can be added and use the common code.
Please see Documentation/process/submitting-patches.rst. Use a correct
formatting for the messages, and send you patchset as a thread.
> This patch set is against net-next.
The net subsystem is somehow special. Please see
Documentation/networking/netdev-FAQ.txt. The tree must be indicated in
the subject prefix of the patch series.
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added
From: Maxim Uvarov @ 2017-09-08 13:32 UTC (permalink / raw)
To: Andrew Lunn
Cc: Tristram.Ha, Pavel Machek, Nathan Conrad, Vivien Didelot,
Florian Fainelli, netdev, linux-kernel, Woojung.Huh
In-Reply-To: <20170907215417.GU11248@lunn.ch>
2017-09-08 0:54 GMT+03:00 Andrew Lunn <andrew@lunn.ch>:
>> -- compatible: For external switch chips, compatible string must be exactly one
>> - of: "microchip,ksz9477"
>> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip,
>> + "microchip,ksz8795" for KSZ8795 chip,
>> + "microchip,ksz8794" for KSZ8794 chip,
>> + "microchip,ksz8765" for KSZ8765 chip,
>> + "microchip,ksz8895" for KSZ8895 chip,
>> + "microchip,ksz8864" for KSZ8864 chip,
>> + "microchip,ksz8873" for KSZ8873 chip,
>> + "microchip,ksz8863" for KSZ8863 chip,
>> + "microchip,ksz8463" for KSZ8463 chip
>
all that chips have the same spi access to get chip id on probe(). I
prefer common microship,ksz-spi rather than somebody will always
maintain that list.
Maxim.
> This part of this patch should be in a patch of the series that
> actually adds support for these chips. Don't document chips until you
> actually support them.
>
>> See Documentation/devicetree/bindings/dsa/dsa.txt for a list of additional required and optional properties.
>> @@ -13,60 +20,60 @@ Examples:
>>
>> Ethernet switch connected via SPI to the host, CPU port wired to eth0:
>>
>> - eth0: ethernet@10001000 {
>> - fixed-link {
>> - speed = <1000>;
>> - full-duplex;
>> - };
>> - };
>> + eth0: ethernet@10001000 {
>> + fixed-link {
>> + speed = <1000>;
>> + full-duplex;
>> + };
>> + };
>>
>> - spi1: spi@f8008000 {
>> - pinctrl-0 = <&pinctrl_spi_ksz>;
>> - cs-gpios = <&pioC 25 0>;
>> - id = <1>;
>> - status = "okay";
>> + spi1: spi@f8008000 {
>> + cs-gpios = <&pioC 25 0>;
>> + id = <1>;
>> + status = "okay";
>>
>> - ksz9477: ksz9477@0 {
>> - compatible = "microchip,ksz9477";
>> - reg = <0>;
>> + ksz9477: ksz9477@0 {
>> + compatible = "microchip,ksz9477";
>> + reg = <0>;
>>
>> - spi-max-frequency = <44000000>;
>> - spi-cpha;
>> - spi-cpol;
>> + spi-max-frequency = <44000000>;
>> + spi-cpha;
>> + spi-cpol;
>> +
>> + status = "okay";
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + port@0 {
>> + reg = <0>;
>> + label = "lan1";
>> + };
>> + port@1 {
>> + reg = <1>;
>> + label = "lan2";
>> + };
>> + port@2 {
>> + reg = <2>;
>> + label = "lan3";
>> + };
>> + port@3 {
>> + reg = <3>;
>> + label = "lan4";
>> + };
>> + port@4 {
>> + reg = <4>;
>> + label = "lan5";
>> + };
>> + port@5 {
>> + reg = <5>;
>> + label = "cpu";
>> + ethernet = <ð0>;
>> + fixed-link {
>> + speed = <1000>;
>> + full-duplex;
>> + };
>> + };
>> + };
>> + };
>> + };
>>
>> - status = "okay";
>> - ports {
>> - #address-cells = <1>;
>> - #size-cells = <0>;
>> - port@0 {
>> - reg = <0>;
>> - label = "lan1";
>> - };
>> - port@1 {
>> - reg = <1>;
>> - label = "lan2";
>> - };
>> - port@2 {
>> - reg = <2>;
>> - label = "lan3";
>> - };
>> - port@3 {
>> - reg = <3>;
>> - label = "lan4";
>> - };
>> - port@4 {
>> - reg = <4>;
>> - label = "lan5";
>> - };
>> - port@5 {
>> - reg = <5>;
>> - label = "cpu";
>> - ethernet = <ð0>;
>> - fixed-link {
>> - speed = <1000>;
>> - full-duplex;
>> - };
>> - };
>> - };
>> - };
>> - };
>
> This part however is a nice cleanup. You can submit this patch as a
> separate patch, once netdev has opened again in about 10 days time.
>
> Andrew
--
Best regards,
Maxim Uvarov
^ permalink raw reply
* Re: [RFC net-next 2/5] net/sched: Introduce Credit Based Shaper (CBS) qdisc
From: Henrik Austad @ 2017-09-08 13:43 UTC (permalink / raw)
To: Vinicius Costa Gomes
Cc: netdev, jhs, xiyou.wangcong, jiri, intel-wired-lan, andre.guedes,
ivan.briano, jesus.sanchez-palencia, boon.leong.ong,
richardcochran
In-Reply-To: <20170901012625.14838-3-vinicius.gomes@intel.com>
[-- Attachment #1: Type: text/plain, Size: 11796 bytes --]
On Thu, Aug 31, 2017 at 06:26:22PM -0700, Vinicius Costa Gomes wrote:
> This queueing discipline implements the shaper algorithm defined by
> the 802.1Q-2014 Section 8.6.8.2 and detailed in Annex L.
>
> It's primary usage is to apply some bandwidth reservation to user
> defined traffic classes, which are mapped to different queues via the
> mqprio qdisc.
>
> Initially, it only supports offloading the traffic shaping work to
> supporting controllers.
>
> Later, when a software implementation is added, the current dependency
> on being installed "under" mqprio can be lifted.
>
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
So, I started testing this in my VM to make sure I didn't do anything silly
and it exploded spectacularly as it used the underlying e1000 driver which
does not have a ndo_setup_tc present.
I commented inline below, but as a tl;dr
The cbs_init() would call into cbs_change() that would correctly detect
that ndo_setup_tc is missing and abort. However, at this stage it would try
to desroy the qdisc and in cbs_destroy() there's no such guard leading to a
BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
Fixing that, another NULL would be found when the code walks into
qdisc_destroy(q->qdisc).
Apart from this, it loaded fine after some wrestling with tc :)
Awesome! :D
> ---
> include/linux/netdevice.h | 1 +
> net/sched/Kconfig | 11 ++
> net/sched/Makefile | 1 +
> net/sched/sch_cbs.c | 286 ++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 299 insertions(+)
> create mode 100644 net/sched/sch_cbs.c
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 35de8312e0b5..dd9a2ecd0c03 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -775,6 +775,7 @@ enum tc_setup_type {
> TC_SETUP_CLSFLOWER,
> TC_SETUP_CLSMATCHALL,
> TC_SETUP_CLSBPF,
> + TC_SETUP_CBS,
> };
>
> /* These structures hold the attributes of xdp state that are being passed
> diff --git a/net/sched/Kconfig b/net/sched/Kconfig
> index e70ed26485a2..c03d86a7775e 100644
> --- a/net/sched/Kconfig
> +++ b/net/sched/Kconfig
> @@ -172,6 +172,17 @@ config NET_SCH_TBF
> To compile this code as a module, choose M here: the
> module will be called sch_tbf.
>
> +config NET_SCH_CBS
Shouldn't this depend on NET_SCH_MQPRIO as it is supposed to hook into
that?
@@ -173,6 +173,7 @@ config NET_SCH_TBF
module will be called sch_tbf.
config NET_SCH_CBS
+ depends on NET_SCH_MQPRIO
tristate "Credit Based Shaper (CBS)"
---help---
Say Y here if you want to use the Credit Based Shaper (CBS) packet
> + tristate "Credit Based Shaper (CBS)"
> + ---help---
> + Say Y here if you want to use the Credit Based Shaper (CBS) packet
> + scheduling algorithm.
> +
> + See the top of <file:net/sched/sch_cbs.c> for more details.
> +
> + To compile this code as a module, choose M here: the
> + module will be called sch_cbs.
> +
> config NET_SCH_GRED
> tristate "Generic Random Early Detection (GRED)"
> ---help---
> diff --git a/net/sched/Makefile b/net/sched/Makefile
> index 7b915d226de7..80c8f92d162d 100644
> --- a/net/sched/Makefile
> +++ b/net/sched/Makefile
> @@ -52,6 +52,7 @@ obj-$(CONFIG_NET_SCH_FQ_CODEL) += sch_fq_codel.o
> obj-$(CONFIG_NET_SCH_FQ) += sch_fq.o
> obj-$(CONFIG_NET_SCH_HHF) += sch_hhf.o
> obj-$(CONFIG_NET_SCH_PIE) += sch_pie.o
> +obj-$(CONFIG_NET_SCH_CBS) += sch_cbs.o
>
> obj-$(CONFIG_NET_CLS_U32) += cls_u32.o
> obj-$(CONFIG_NET_CLS_ROUTE4) += cls_route.o
> diff --git a/net/sched/sch_cbs.c b/net/sched/sch_cbs.c
> new file mode 100644
> index 000000000000..1c86a9e14150
> --- /dev/null
> +++ b/net/sched/sch_cbs.c
> @@ -0,0 +1,286 @@
> +/*
> + * net/sched/sch_cbs.c Credit Based Shaper
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + *
> + * Authors: Vininicius Costa Gomes <vinicius.gomes@intel.com>
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/string.h>
> +#include <linux/errno.h>
> +#include <linux/skbuff.h>
> +#include <net/netlink.h>
> +#include <net/sch_generic.h>
> +#include <net/pkt_sched.h>
> +
> +struct cbs_sched_data {
> + struct Qdisc *qdisc; /* Inner qdisc, default - pfifo queue */
> + s32 queue;
> + s32 locredit;
> + s32 hicredit;
> + s32 sendslope;
> + s32 idleslope;
> +};
> +
> +static int cbs_enqueue(struct sk_buff *skb, struct Qdisc *sch,
> + struct sk_buff **to_free)
> +{
> + struct cbs_sched_data *q = qdisc_priv(sch);
> + int ret;
> +
> + ret = qdisc_enqueue(skb, q->qdisc, to_free);
> + if (ret != NET_XMIT_SUCCESS) {
> + if (net_xmit_drop_count(ret))
> + qdisc_qstats_drop(sch);
> + return ret;
> + }
> +
> + qdisc_qstats_backlog_inc(sch, skb);
> + sch->q.qlen++;
> + return NET_XMIT_SUCCESS;
> +}
> +
> +static struct sk_buff *cbs_dequeue(struct Qdisc *sch)
> +{
> + struct cbs_sched_data *q = qdisc_priv(sch);
> + struct sk_buff *skb;
> +
> + skb = q->qdisc->ops->peek(q->qdisc);
> + if (skb) {
> + skb = qdisc_dequeue_peeked(q->qdisc);
> + if (unlikely(!skb))
> + return NULL;
> +
> + qdisc_qstats_backlog_dec(sch, skb);
> + sch->q.qlen--;
> + qdisc_bstats_update(sch, skb);
> +
> + return skb;
> + }
> + return NULL;
> +}
> +
> +static void cbs_reset(struct Qdisc *sch)
> +{
> + struct cbs_sched_data *q = qdisc_priv(sch);
> +
> + qdisc_reset(q->qdisc);
> +}
> +
> +static const struct nla_policy cbs_policy[TCA_CBS_MAX + 1] = {
> + [TCA_CBS_PARMS] = { .len = sizeof(struct tc_cbs_qopt) },
> +};
> +
> +static int cbs_change(struct Qdisc *sch, struct nlattr *opt)
> +{
> + struct cbs_sched_data *q = qdisc_priv(sch);
> + struct tc_cbs_qopt_offload cbs = { };
> + struct nlattr *tb[TCA_CBS_MAX + 1];
> + const struct net_device_ops *ops;
> + struct tc_cbs_qopt *qopt;
> + struct net_device *dev;
> + int err;
> +
> + err = nla_parse_nested(tb, TCA_CBS_MAX, opt, cbs_policy, NULL);
> + if (err < 0)
> + return err;
> +
> + err = -EINVAL;
> + if (!tb[TCA_CBS_PARMS])
> + goto done;
> +
> + qopt = nla_data(tb[TCA_CBS_PARMS]);
> +
> + dev = qdisc_dev(sch);
> + ops = dev->netdev_ops;
> +
> + /* FIXME: this means that we can only install this qdisc
> + * "under" mqprio. Do we need a more generic way to retrieve
> + * the queue, or do we pass the netdev_queue to the driver?
> + */
> + cbs.queue = TC_H_MIN(sch->parent) - 1 - netdev_get_num_tc(dev);
> +
> + cbs.enable = 1;
> + cbs.hicredit = qopt->hicredit;
> + cbs.locredit = qopt->locredit;
> + cbs.idleslope = qopt->idleslope;
> + cbs.sendslope = qopt->sendslope;
> +
> + err = -ENOTSUPP;
> + if (!ops->ndo_setup_tc)
> + goto done;
This confuses tc a bit, and looking at other qdisc schedulers, it seems
like EOPNOTSUPP is an alternative, this changes the output from
RTNETLINK answers: Unknown error 524
- to
RTNETLINK answers: Operation not supported
which perhaps is more informative.
> +
> + err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_CBS, &cbs);
> + if (err < 0)
> + goto done;
> +
> + q->queue = cbs.queue;
> + q->hicredit = cbs.hicredit;
> + q->locredit = cbs.locredit;
> + q->idleslope = cbs.idleslope;
> + q->sendslope = cbs.sendslope;
> +
> +done:
> + return err;
> +}
> +
> +static int cbs_init(struct Qdisc *sch, struct nlattr *opt)
> +{
> + struct cbs_sched_data *q = qdisc_priv(sch);
> +
> + if (!opt)
> + return -EINVAL;
> +
> + q->qdisc = fifo_create_dflt(sch, &pfifo_qdisc_ops, 1024);
> + qdisc_hash_add(q->qdisc, true);
> +
> + return cbs_change(sch, opt);
> +}
> +
> +static void cbs_destroy(struct Qdisc *sch)
> +{
> + struct cbs_sched_data *q = qdisc_priv(sch);
> + struct tc_cbs_qopt_offload cbs = { };
> + struct net_device *dev;
> + int err;
> +
> + q->hicredit = 0;
> + q->locredit = 0;
> + q->idleslope = 0;
> + q->sendslope = 0;
> +
> + dev = qdisc_dev(sch);
> +
> + cbs.queue = q->queue;
> + cbs.enable = 0;
> +
> + err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_CBS, &cbs);
This can lead to NULL pointer deref if it is not set (tested for in
cbs_change and error there leads us here, which then explodes).
> + if (err < 0)
> + pr_warn("Couldn't reset queue %d to default values\n",
> + cbs.queue);
> +
> + qdisc_destroy(q->qdisc);
Same, q->qdisc was NULL when cbs_init() failed.
> +}
> +
> +static int cbs_dump(struct Qdisc *sch, struct sk_buff *skb)
> +{
> + struct cbs_sched_data *q = qdisc_priv(sch);
> + struct nlattr *nest;
> + struct tc_cbs_qopt opt;
> +
> + sch->qstats.backlog = q->qdisc->qstats.backlog;
> + nest = nla_nest_start(skb, TCA_OPTIONS);
> + if (!nest)
> + goto nla_put_failure;
> +
> + opt.hicredit = q->hicredit;
> + opt.locredit = q->locredit;
> + opt.sendslope = q->sendslope;
> + opt.idleslope = q->idleslope;
> +
> + if (nla_put(skb, TCA_CBS_PARMS, sizeof(opt), &opt))
> + goto nla_put_failure;
> +
> + return nla_nest_end(skb, nest);
> +
> +nla_put_failure:
> + nla_nest_cancel(skb, nest);
> + return -1;
> +}
> +
> +static int cbs_dump_class(struct Qdisc *sch, unsigned long cl,
> + struct sk_buff *skb, struct tcmsg *tcm)
> +{
> + struct cbs_sched_data *q = qdisc_priv(sch);
> +
> + tcm->tcm_handle |= TC_H_MIN(1);
> + tcm->tcm_info = q->qdisc->handle;
> +
> + return 0;
> +}
> +
> +static int cbs_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
> + struct Qdisc **old)
> +{
> + struct cbs_sched_data *q = qdisc_priv(sch);
> +
> + if (!new)
> + new = &noop_qdisc;
> +
> + *old = qdisc_replace(sch, new, &q->qdisc);
> + return 0;
> +}
> +
> +static struct Qdisc *cbs_leaf(struct Qdisc *sch, unsigned long arg)
> +{
> + struct cbs_sched_data *q = qdisc_priv(sch);
> +
> + return q->qdisc;
> +}
> +
> +static unsigned long cbs_find(struct Qdisc *sch, u32 classid)
> +{
> + return 1;
> +}
> +
> +static int cbs_delete(struct Qdisc *sch, unsigned long arg)
> +{
> + return 0;
> +}
> +
> +static void cbs_walk(struct Qdisc *sch, struct qdisc_walker *walker)
> +{
> + if (!walker->stop) {
> + if (walker->count >= walker->skip)
> + if (walker->fn(sch, 1, walker) < 0) {
> + walker->stop = 1;
> + return;
> + }
> + walker->count++;
> + }
> +}
> +
> +static const struct Qdisc_class_ops cbs_class_ops = {
> + .graft = cbs_graft,
> + .leaf = cbs_leaf,
> + .find = cbs_find,
> + .delete = cbs_delete,
> + .walk = cbs_walk,
> + .dump = cbs_dump_class,
> +};
> +
> +static struct Qdisc_ops cbs_qdisc_ops __read_mostly = {
> + .next = NULL,
> + .cl_ops = &cbs_class_ops,
> + .id = "cbs",
> + .priv_size = sizeof(struct cbs_sched_data),
> + .enqueue = cbs_enqueue,
> + .dequeue = cbs_dequeue,
> + .peek = qdisc_peek_dequeued,
> + .init = cbs_init,
> + .reset = cbs_reset,
> + .destroy = cbs_destroy,
> + .change = cbs_change,
> + .dump = cbs_dump,
> + .owner = THIS_MODULE,
> +};
> +
> +static int __init cbs_module_init(void)
> +{
> + return register_qdisc(&cbs_qdisc_ops);
> +}
> +
> +static void __exit cbs_module_exit(void)
> +{
> + unregister_qdisc(&cbs_qdisc_ops);
> +}
> +module_init(cbs_module_init)
> +module_exit(cbs_module_exit)
> +MODULE_LICENSE("GPL");
> --
> 2.14.1
>
--
Henrik Austad
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 2/3] net: ethernet: socionext: add AVE ethernet driver
From: Andrew Lunn @ 2017-09-08 13:50 UTC (permalink / raw)
To: Kunihiko Hayashi
Cc: netdev, David S. Miller, Florian Fainelli, Rob Herring,
Mark Rutland, linux-arm-kernel, linux-kernel, devicetree,
Masahiro Yamada, Masami Hiramatsu, Jassi Brar
In-Reply-To: <1504875731-3680-3-git-send-email-hayashi.kunihiko@socionext.com>
> +static int ave_mdio_busywait(struct net_device *ndev)
> +{
> + int ret = 1, loop = 100;
> + u32 mdiosr;
> +
> + /* wait until completion */
> + while (1) {
> + mdiosr = ave_r32(ndev, AVE_MDIOSR);
> + if (!(mdiosr & AVE_MDIOSR_STS))
> + break;
> +
> + usleep_range(10, 20);
> + if (!loop--) {
> + netdev_err(ndev,
> + "failed to read from MDIO (status:0x%08x)\n",
> + mdiosr);
> + ret = 0;
ETIMEDOUT would be better.
> + break;
> + }
> + }
> +
> + return ret;
and then return 0 on success. That is the normal convention for return
values. An error code, and 0.
> +static int ave_mdiobus_write(struct mii_bus *bus,
> + int phyid, int regnum, u16 val)
> +{
> + struct net_device *ndev = bus->priv;
> + u32 mdioctl;
> +
> + /* write address */
> + ave_w32(ndev, AVE_MDIOAR, (phyid << 8) | regnum);
> +
> + /* write data */
> + ave_w32(ndev, AVE_MDIOWDR, val);
> +
> + /* write request */
> + mdioctl = ave_r32(ndev, AVE_MDIOCTR);
> + ave_w32(ndev, AVE_MDIOCTR, mdioctl | AVE_MDIOCTR_WREQ);
> +
> + if (!ave_mdio_busywait(ndev)) {
> + netdev_err(ndev, "phy-%d reg-%x write failed\n",
> + phyid, regnum);
> + return -1;
If ave_mdio_busywait() returns ETIMEDOUT, you can just return
it. Returning -1 is not good.
> + }
> +
> + return 0;
> +}
> +
> +static irqreturn_t ave_interrupt(int irq, void *netdev)
> +{
> + struct net_device *ndev = (struct net_device *)netdev;
> + struct ave_private *priv = netdev_priv(ndev);
> + u32 gimr_val, gisr_val;
> +
> + gimr_val = ave_irq_disable_all(ndev);
> +
> + /* get interrupt status */
> + gisr_val = ave_r32(ndev, AVE_GISR);
> +
> + /* PHY */
> + if (gisr_val & AVE_GI_PHY) {
> + ave_w32(ndev, AVE_GISR, AVE_GI_PHY);
> + if (priv->internal_phy_interrupt)
> + phy_mac_interrupt(ndev->phydev, ndev->phydev->link);
Humm. I don't think this is correct. You are supposed to give it the
new link state, not the old.
What does a PHY interrupt mean here?
> +static void ave_adjust_link(struct net_device *ndev)
> +{
> + struct ave_private *priv = netdev_priv(ndev);
> + struct phy_device *phydev = ndev->phydev;
> + u32 val, txcr, rxcr, rxcr_org;
> +
> + /* set RGMII speed */
> + val = ave_r32(ndev, AVE_TXCR);
> + val &= ~(AVE_TXCR_TXSPD_100 | AVE_TXCR_TXSPD_1G);
> +
> + if (priv->phy_mode == PHY_INTERFACE_MODE_RGMII &&
> + phydev->speed == SPEED_1000)
phy_interface_mode_is_rgmii(), so that you handle all the RGMII modes.
> + val |= AVE_TXCR_TXSPD_1G;
> + else if (phydev->speed == SPEED_100)
> + val |= AVE_TXCR_TXSPD_100;
> +
> + ave_w32(ndev, AVE_TXCR, val);
> +
> + /* set RMII speed (100M/10M only) */
> + if (priv->phy_mode != PHY_INTERFACE_MODE_RGMII) {
Not so safe. It would be better to check for the modes you actually
support.
> + if (phydev->link)
> + netif_carrier_on(ndev);
> + else
> + netif_carrier_off(ndev);
I don't think you need this. The phylib should do it for you.
> +
> + phy_print_status(phydev);
> +}
> +
> +static int ave_init(struct net_device *ndev)
> +{
> + struct ave_private *priv = netdev_priv(ndev);
> + struct device *dev = ndev->dev.parent;
> + struct device_node *phy_node, *np = dev->of_node;
> + struct phy_device *phydev;
> + const void *mac_addr;
> + u32 supported;
> +
> + /* get mac address */
> + mac_addr = of_get_mac_address(np);
> + if (mac_addr)
> + ether_addr_copy(ndev->dev_addr, mac_addr);
> +
> + /* if the mac address is invalid, use random mac address */
> + if (!is_valid_ether_addr(ndev->dev_addr)) {
> + eth_hw_addr_random(ndev);
> + dev_warn(dev, "Using random MAC address: %pM\n",
> + ndev->dev_addr);
> + }
> +
> + /* attach PHY with MAC */
> + phy_node = of_get_next_available_child(np, NULL);
???
Should this not be looking for a phy-handle property?
Documentation/devicetree/binds/net/ethernet.txt:
- phy-handle: phandle, specifies a reference to a node representing a PHY
device; this property is described in the Devicetree Specification and so
preferred;
> + phydev = of_phy_connect(ndev, phy_node,
> + ave_adjust_link, 0, priv->phy_mode);
> + if (!phydev) {
> + dev_err(dev, "could not attach to PHY\n");
> + return -ENODEV;
> + }
> + of_node_put(phy_node);
> +
> + priv->phydev = phydev;
> + phydev->autoneg = AUTONEG_ENABLE;
> + phydev->speed = 0;
> + phydev->duplex = 0;
And this should not be needed.
> +
> + dev_info(dev, "connected to %s phy with id 0x%x\n",
> + phydev->drv->name, phydev->phy_id);
phy_attached_info()
> +
> + if (priv->phy_mode != PHY_INTERFACE_MODE_RGMII) {
Same comment as above.
> + supported = phydev->supported;
> + phydev->supported &= ~PHY_GBIT_FEATURES;
> + phydev->supported |= supported & PHY_BASIC_FEATURES;
> + }
> +
> + /* PHY interrupt stop instruction is needed because the interrupt
> + * continues to assert.
> + */
> + phy_stop_interrupts(phydev);
Could you explain this some more? It sounds like your interrupt
controller is broken.
> +
> + /* When PHY driver can't handle its interrupt directly,
> + * interrupt request always fails and polling method is used
> + * alternatively. In this case, the libphy says
> + * "libphy: uniphier-mdio: Can't get IRQ -1 (PHY)".
> + */
> + phy_start_interrupts(phydev);
-1 is PHY_POLL. So calling phy_start_interrupts() is wrong. In fact,
you should not be calling phy_start_interrupts() at all. No other
Ethernet driver does.
> +
> + phy_start_aneg(phydev);
> +
> + return 0;
> +}
> +
> +static void ave_uninit(struct net_device *ndev)
> +{
> + struct ave_private *priv = netdev_priv(ndev);
> +
> + phy_stop_interrupts(priv->phydev);
And no other Ethernet driver calls phy_stop_interrupts either.
Please take a look at this.
> + phy_disconnect(priv->phydev);
> +}
> +
Andrew
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: phy: realtek: add RTL8201F phy-id and functions
From: Andrew Lunn @ 2017-09-08 13:57 UTC (permalink / raw)
To: Kunihiko Hayashi
Cc: netdev, David S. Miller, Florian Fainelli, Rob Herring,
Mark Rutland, linux-arm-kernel, linux-kernel, devicetree,
Masahiro Yamada, Masami Hiramatsu, Jassi Brar, Jongsung Kim
In-Reply-To: <1504875731-3680-4-git-send-email-hayashi.kunihiko@socionext.com>
On Fri, Sep 08, 2017 at 10:02:11PM +0900, Kunihiko Hayashi wrote:
> From: Jassi Brar <jaswinder.singh@linaro.org>
>
> Add RTL8201F phy-id and the related functions to the driver.
>
> The original patch is as follows:
> https://patchwork.kernel.org/patch/2538341/
>
> Signed-off-by: Jongsung Kim <neidhard.kim@lge.com>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs
From: Andrew Lunn @ 2017-09-08 14:00 UTC (permalink / raw)
To: Corentin Labbe
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, peppe.cavallaro-qxv4g6HH51o,
alexandre.torgue-qxv4g6HH51o, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170908132632.GA3037@Red>
> > > +static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
> > > + void *data)
> > > +{
> > > + struct stmmac_priv *priv = data;
> > > + struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
> > > + u32 reg, val;
> > > + int ret = 0;
> > > + bool need_reset = false;
> > > +
> > > + if (current_child ^ desired_child) {
> > > + regmap_read(gmac->regmap, SYSCON_EMAC_REG, ®);
> > > + switch (desired_child) {
> > > + case DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID:
> > > + dev_info(priv->device, "Switch mux to internal PHY");
> > > + val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SELECT;
> > > + if (gmac->use_internal_phy)
> > > + need_reset = true;
> > > + break;
> >
> > This i don't get. Why do you need use_internal_phy? Isn't that
> > implicit from DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID? Is it even possible to
> > use an external PHY on the internal MDIO bus?
> >
>
> On my H3 box with external PHY, the MDIO mux library first select (for scan ?) the internal MDIO.
> Without use_internal_phy usage, this board will launch a reset to use the internal MDIO... and this reset timeout/fail.
Do you know why the reset times out/fails?
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH iproute2 1/2] lib/libnetlink: re malloc buff if size is not enough
From: Hangbin Liu @ 2017-09-08 14:01 UTC (permalink / raw)
To: Phil Sutter, netdev, Stephen Hemminger, Michal Kubecek
In-Reply-To: <20170908110247.GS2399@orbyte.nwl.cc>
Hi Phil,
Thanks for the comments, see replies bellow.
On Fri, Sep 08, 2017 at 01:02:47PM +0200, Phil Sutter wrote:
> Hi Hangbin,
>
> On Fri, Sep 08, 2017 at 06:14:56PM +0800, Hangbin Liu wrote:
> [...]
> > diff --git a/lib/libnetlink.c b/lib/libnetlink.c
> > index be7ac86..37cfb5a 100644
> > --- a/lib/libnetlink.c
> > +++ b/lib/libnetlink.c
> > @@ -402,6 +402,59 @@ static void rtnl_dump_error(const struct rtnl_handle *rth,
> > }
> > }
> >
> > +static int rtnl_recvmsg(int fd, struct msghdr *msg, char **buf)
> > +{
> > + struct iovec *iov;
> > + int len = -1, buf_len = 32768;
> > + char *buffer = *buf;
>
> Isn't it possible to make 'buffer' static instead of the two 'buf'
> variables in rtnl_dump_filter_l() and __rtnl_talk()? Then we would have
> only a single buffer which is shared between both functions instead of
> two which are independently allocated.
I was also thinking of this before. But in function ipaddrlabel_flush()
if (rtnl_dump_filter(&rth, flush_addrlabel, NULL) < 0)
It will cal rtnl_dump_filter_l() first via
rtnl_dump_filter() -> rtnl_dump_filter_nc() -> rtnl_dump_filter_l().
Then call rtnl_talk() later via call back
a->filter(&nladdr, h, a->arg1) -> flush_addrlabel() -> rtnl_talk()
So if we only use one static buffer in rtnl_recvmsg(). Then it will be written
at lease twice.
The path looks like bellow in function rtnl_dump_filter_l()
while (1) {
status = rtnl_recvmsg(rth->fd, &msg, &buf); <== write buf
for (a = arg; a->filter; a++) {
struct nlmsghdr *h = (struct nlmsghdr *)buf; <== assign buf to h
while (NLMSG_OK(h, msglen)) {
if (!rth->dump_fp) {
err = a->filter(&nladdr, h, a->arg1); <== buf changed via rtnl_talk()
}
h = NLMSG_NEXT(h, msglen); <== so h will also be changed
}
}
}
That's why I have to use two static buffers.
>
> > +
> > + int flag = MSG_PEEK | MSG_TRUNC;
> > +
> > + if (buffer == NULL)
> > +re_malloc:
> > + buffer = malloc(buf_len);
>
> I think using realloc() here is more appropriate since there is no need
> to free the buffer in beforehand and calling realloc(NULL, len) is
> equivalent to calling malloc(len). I think 'realloc' is also a better
> name for the goto label.
Good idea.
>
> > + if (buffer == NULL) {
> > + fprintf(stderr, "malloc error: no enough buffer\n");
>
> Minor typo here: s/no/not/
>
> > + return -1;
>
> Return -ENOMEM?
>
> > + }
> > +
> > + iov = msg->msg_iov;
> > + iov->iov_base = buffer;
> > + iov->iov_len = buf_len;
> > +
> > +re_recv:
>
> Just call this 'recv'? (Not really important though.)
>
> > + len = recvmsg(fd, msg, flag);
> > +
> > + if (len < 0) {
> > + if (errno == EINTR || errno == EAGAIN)
> > + return 0;
>
> Instead of returning 0 (which makes callers retry), goto re_recv?
Yes, will fix this.
>
> > + fprintf(stderr, "netlink receive error %s (%d)\n",
> > + strerror(errno), errno);
> > + return len;
> > + }
> > +
> > + if (len == 0) {
> > + fprintf(stderr, "EOF on netlink\n");
> > + return -1;
>
> Return -ENODATA here? (Initially I though about -EOF, but EOF is -1 so
> that would be incorrect).
>
> > + }
> > +
> > + if (len > buf_len) {
> > + free(buffer);
>
> If you use realloc() above, this can be dropped.
Yes.
>
> > + buf_len = len;
>
> For this to work you have to make buf_len static too, otherwise you will
> unnecessarily reallocate the buffer. Oh, and that also requires the
> single buffer (as pointed out above) because you will otherwise use a
> common buf_len for both static buffers passed to this function.
Since we have to use two static bufffers. So how about check like
if (len > strlen(buffer))
>
> > + flag = 0;
> > + goto re_malloc;
> > + }
> > +
> > + if (flag != 0) {
> > + flag = 0;
> > + goto re_recv;
> > + }
> > +
> > + *buf = buffer;
> > + return len;
> > +}
> > +
> > int rtnl_dump_filter_l(struct rtnl_handle *rth,
> > const struct rtnl_dump_filter_arg *arg)
> > {
> > @@ -413,31 +466,20 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
> > .msg_iov = &iov,
> > .msg_iovlen = 1,
> > };
> > - char buf[32768];
> > + static char *buf = NULL;
>
> If you keep the static buffer in rtnl_recvmsg(), there is no need to
> assign NULL here.
>
> > int dump_intr = 0;
> >
> > - iov.iov_base = buf;
> > while (1) {
> > int status;
> > const struct rtnl_dump_filter_arg *a;
> > int found_done = 0;
> > int msglen = 0;
> >
> > - iov.iov_len = sizeof(buf);
> > - status = recvmsg(rth->fd, &msg, 0);
> > -
> > - if (status < 0) {
> > - if (errno == EINTR || errno == EAGAIN)
> > - continue;
> > - fprintf(stderr, "netlink receive error %s (%d)\n",
> > - strerror(errno), errno);
> > - return -1;
> > - }
> > -
> > - if (status == 0) {
> > - fprintf(stderr, "EOF on netlink\n");
> > - return -1;
> > - }
> > + status = rtnl_recvmsg(rth->fd, &msg, &buf);
> > + if (status < 0)
> > + return status;
> > + else if (status == 0)
> > + continue;
>
> When retrying inside rtnl_recvmsg(), it won't return 0 anymore. I
> believe the whole 'while (1)' loop could go away then.
>
Like Michal said, there may have multi netlink packets?
Thanks
Hangbin
^ permalink raw reply
* Re: [PATCH net-next v2 01/10] net: dsa: add debugfs interface
From: Vivien Didelot @ 2017-09-08 13:58 UTC (permalink / raw)
To: Greg KH
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Egil Hjelmeland, John Crispin, Woojung Huh,
Sean Wang, Nikita Yushchenko, Chris Healy
In-Reply-To: <20170907193434.GA11006@kroah.com>
Hi Greg,
Greg KH <gregkh@linuxfoundation.org> writes:
> I agree you shouldn't be using debugfs for this, but in the future, if
> you do write debugfs code, please take the following review into
> account:
Humm sorry I may not have given enough details. This was really meant
for debug and dev only, because DSA makes it hard to query directly the
hardware (some switch ports are not exposed to userspace as well.)
This is not meant to be used for anything real at all, or even be
compiled-in in a production kernel. That's why I found it appropriate.
So I am still wondering why it doesn't fit here, can you tell me why?
> You should _never_ care about the return value of a debugfs call, and
> you should not need to ever propagate the error upward. The api was
> written to not need this.
>
> Just call the function, and return, that's it. If you need to save the
> return value (i.e. it's a dentry), you also don't care, just save it and
> pass it to some other debugfs call, and all will still be fine. Your
> code should never do anything different if a debugfs call succeeds or
> fails.
Thank for your interesting review! I'll cleanup my out-of-tree patches.
Vivien
^ permalink raw reply
* Re: [PATCH net-next 1/3] dt-bindings: net: add DT bindings for Socionext UniPhier AVE
From: Andrew Lunn @ 2017-09-08 14:03 UTC (permalink / raw)
To: Kunihiko Hayashi
Cc: netdev, David S. Miller, Florian Fainelli, Rob Herring,
Mark Rutland, linux-arm-kernel, linux-kernel, devicetree,
Masahiro Yamada, Masami Hiramatsu, Jassi Brar
In-Reply-To: <1504875731-3680-2-git-send-email-hayashi.kunihiko@socionext.com>
> + eth: ethernet@65000000 {
> + compatible = "socionext,uniphier-ave4";
> + reg = <0x65000000 0x8500>;
> + interrupts = <0 66 4>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_ether_rgmii>;
> + phy-mode = "rgmii";
> + socionext,desc-bits = <64>;
> + local-mac-address = [00 00 00 00 00 00];
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> + ethphy: ethphy@1 {
> + reg = <1>;
> + };
So you normally have an mdio node, and the phy as a children of that
node.
mdio {
ethphy: ethernet-phy@6 {
reg = <6>;
};
};
Andrew
^ 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