* Re: [PATCH net-next v2] ipv4: fib: Replay events when registering FIB notifier
From: Jiri Pirko @ 2016-11-02 7:20 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Ido Schimmel, Eric Dumazet, netdev, davem, jiri, mlxsw, dsa,
nikolay, andy, vivien.didelot, andrew, f.fainelli,
alexander.h.duyck, kuznet, jmorris, yoshfuji, kaber, Ido Schimmel
In-Reply-To: <58194BD6.5040406@cumulusnetworks.com>
Wed, Nov 02, 2016 at 03:13:42AM CET, roopa@cumulusnetworks.com wrote:
>On 11/1/16, 10:03 AM, Ido Schimmel wrote:
>> Hi Roopa,
>>
>> On Tue, Nov 01, 2016 at 08:14:14AM -0700, Roopa Prabhu wrote:
>>>
>[snip]
>>> I have the same concern as Eric here.
>>>
>>> I understand why you need it, but can the driver request for an initial dump and that
>>> dump be made more efficient somehow ie not hold rtnl for the whole dump ?.
>>> instead of making the fib notifier registration code doing it.
>> We can do what we suggested in the last bi-weekly meeting, which is
>> still holding rtnl, but moving the hardware operation to delayed work.
>> This is possible because upper layers always assume operation was
>> successful and driver is responsible for invoking its abort mechanism in
>> case of failure.
>>
>>> these routing table sizes can be huge and an analogy for this in user-space:
>>> We do request a netlink dump of routing tables at initialization (on driver starts or resets)...
>>> but, existing netlink routing table dumps for that scale don't hold rtnl for the whole dump.
>>> The dump is split into multiple responses to the user and hence it does not starve other rtnl users.
>> In my reply to Eric I mentioned that when we register and unregister
>> from this chain the tables aren't really huge, but instead quite small.
>> I understand your concerns, but I don't wish to make things more
>> complicated than they should be only to address concerns that aren't
>> really realistic.
>
>I understand..but, if you are adding some core infrastructure for switchdev ..it cannot be
>based on the number of simple use-cases or data you have today.
>
>I won't be surprised if tomorrow other switch drivers have a case where they need to
>reset the hw routing table state and reprogram all routes again. Re-registering the notifier to just
>get the routing state of the kernel will not scale. For the long term, since the driver does not maintain a cache,
Driver (mlxsw, rocker) maintain a cache. So I'm not sure why you say
otherwise.
>a pull api with efficient use of rtnl will be useful for other such cases as well.
How do you imagine this "pull API" should look like?
>
>
>If you don't want to get to the complexity of a new api right away because of the
>simple case of management interface routes you have, Can your driver register the notifier early ?
>(I am sure you have probably already thought about this)
Register early? What it would resolve? I must be missing something. We
register as early as possible. But the thing is, we cannot register
in a past. And that is what this patch resolves.
>
>>
>> I believe current patch is quite simple and also consistent with other
>> notification chains in the kernel, such as the netdevice, where rtnl is
>> held during replay of events.
>> http://lxr.free-electrons.com/source/net/core/dev.c#L1535
>as you know, netdev and routing scale are not the same thing.
>Looking at the current code for netdevices (replay and rollback on failure),
>a pull api (equivalent to the netlink dump api) may end up being less complex...with an
>ability to batch in the future.
>
>
>
>
>
>
>
^ permalink raw reply
* [PATCH] igb/e1000: correct register comments
From: Cao jin @ 2016-11-02 7:20 UTC (permalink / raw)
To: linux-kernel, netdev; +Cc: intel-wired-lan, jeffrey.t.kirsher
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
drivers/net/ethernet/intel/igb/e1000_regs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igb/e1000_regs.h b/drivers/net/ethernet/intel/igb/e1000_regs.h
index d84afdd..58adbf2 100644
--- a/drivers/net/ethernet/intel/igb/e1000_regs.h
+++ b/drivers/net/ethernet/intel/igb/e1000_regs.h
@@ -320,7 +320,7 @@
#define E1000_VT_CTL 0x0581C /* VMDq Control - RW */
#define E1000_WUC 0x05800 /* Wakeup Control - RW */
#define E1000_WUFC 0x05808 /* Wakeup Filter Control - RW */
-#define E1000_WUS 0x05810 /* Wakeup Status - RO */
+#define E1000_WUS 0x05810 /* Wakeup Status - R/W1C */
#define E1000_MANC 0x05820 /* Management Control - RW */
#define E1000_IPAV 0x05838 /* IP Address Valid - RW */
#define E1000_WUPL 0x05900 /* Wakeup Packet Length - RW */
--
2.1.0
^ permalink raw reply related
* Re: [PATCH (net.git) 2/3] stmmac: fix PTP support for GMAC4
From: Rayagond Kokatanur @ 2016-11-02 7:04 UTC (permalink / raw)
To: Giuseppe CAVALLARO
Cc: netdev, alexandre.torgue, Richard Cochran, linux-kernel,
Seraphin Bonnaffe
In-Reply-To: <a0a8b469-2b5a-3a32-9896-e531ed281a6c@st.com>
On Wed, Nov 2, 2016 at 12:04 PM, Giuseppe CAVALLARO
<peppe.cavallaro@st.com> wrote:
> Hello Rayagond
>
> if patches are ok, can we consider you Acked-by ?
Yes.
>
> Thx
> Peppe
>
>
> On 10/27/2016 12:51 PM, Rayagond Kokatanur wrote:
>>
>> On Thu, Oct 27, 2016 at 4:02 PM, Giuseppe CAVALLARO
>> <peppe.cavallaro@st.com> wrote:
>>>
>>> Hello Rayagond !
>>>
>>> On 10/27/2016 12:25 PM, Rayagond Kokatanur wrote:
>>>>>
>>>>>
>>>>> +static int dwmac4_wrback_get_rx_timestamp_status(void *desc, u32 ats)
>>>>>>
>>>>>> {
>>>>>> struct dma_desc *p = (struct dma_desc *)desc;
>>>>>> + int ret = -EINVAL;
>>>>>> +
>>>>>> + /* Get the status from normal w/b descriptor */
>>>>>> + if (likely(p->des3 & TDES3_RS1V)) {
>>>>>> + if (likely(p->des1 & RDES1_TIMESTAMP_AVAILABLE)) {
>>>>>> + int i = 0;
>>>>>> +
>>>>>> + /* Check if timestamp is OK from context
>>>>>> descriptor */
>>>>>> + do {
>>>>>> + ret = dwmac4_rx_check_timestamp(desc);
>>>>
>>>>
>>>> Here, "desc" is not pointing to next descriptor (ie context
>>>> descriptor). Driver should check the context descriptor.
>>>
>>>
>>>
>>> you are right and this is done by the caller: stmmac_get_rx_hwtstamp
>>
>>
>> Yes.
>>
>>>
>>> Cheers
>>> peppe
>>>
>>
>>
>>
>
--
wwr
Rayagond
^ permalink raw reply
* Re: [PATCH (net.git) 0/3] stmmac: fix PTP support
From: Rayagond Kokatanur @ 2016-11-02 7:03 UTC (permalink / raw)
To: Richard Cochran
Cc: Giuseppe Cavallaro, netdev, alexandre.torgue, linux-kernel,
Seraphin Bonnaffe
In-Reply-To: <20161026085812.GD11974@localhost.localdomain>
On Wed, Oct 26, 2016 at 2:28 PM, Richard Cochran
<richardcochran@gmail.com> wrote:
> On Wed, Oct 26, 2016 at 08:56:01AM +0200, Giuseppe Cavallaro wrote:
>> This subset of patches aim to fix the PTP support
>> for the stmmac and especially for 4.x chip series.
>> While setting PTP on an ST box with 4.00a Ethernet
>> core, the kernel panics due to a broken settings
>> of the descriptors. The patches review the
>> register configuration, the algo used for configuring
>> the protocol, the way to get the timestamp inside
>> the RX/TX descriptors and, in the end, the statistics
>> displayed by ethtool.
>>
>> Giuseppe Cavallaro (3):
>> stmmac: update the PTP header file
>> stmmac: fix PTP support for GMAC4
>> stmmac: fix PTP type ethtool stats
>
> Acked-by: Richard Cochran <richardcochran@gmail.com>
Acked-by: Rayagond Kokatanur <rayagond@vayavyalabs.com>
--
wwr
Rayagond
^ permalink raw reply
* Re: [net-next PATCH 0/7] stmmac: dwmac-sti refactor+cleanup
From: Giuseppe CAVALLARO @ 2016-11-02 6:39 UTC (permalink / raw)
To: Joachim Eastwood, davem; +Cc: alexandre.torgue, netdev
In-Reply-To: <20161030200507.851-1-manabian@gmail.com>
Hello Joachim
thx for this series, I will review them and test the changes
on my board asap ... I let you known.
Peppe
On 10/30/2016 9:05 PM, Joachim Eastwood wrote:
> This patch set aims to remove the init/exit callbacks from the
> dwmac-sti driver and instead use standard PM callbacks. Doing this
> will also allow us to cleanup the driver.
>
> Eventually the init/exit callbacks will be deprecated and removed
> from all drivers dwmac-* except for dwmac-generic. Drivers will be
> refactored to use standard PM and remove callbacks.
>
> Note that this patch set has only been test compiled and no functional
> change is intended.
>
> Joachim Eastwood (7):
> stmmac: dwmac-sti: remove useless of_node check
> stmmac: dwmac-sti: remove clk NULL checks
> stmmac: dwmac-sti: add PM ops and resume function
> stmmac: dwmac-sti: move st,gmac_en parsing to sti_dwmac_parse_data
> stmmac: dwmac-sti: move clk_prepare_enable out of init and add error handling
> stmmac: dwmac-sti: clean up and rename sti_dwmac_init
> stmmac: dwmac-sti: remove unused priv dev member
>
> drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c | 86 ++++++++++++++++---------
> 1 file changed, 57 insertions(+), 29 deletions(-)
>
^ permalink raw reply
* Re: [PATCH (net.git) 2/3] stmmac: fix PTP support for GMAC4
From: Giuseppe CAVALLARO @ 2016-11-02 6:34 UTC (permalink / raw)
To: Rayagond Kokatanur
Cc: netdev, alexandre.torgue, Richard Cochran, linux-kernel,
seraphin.bonnaffe
In-Reply-To: <CAJ3bTp7YON-+EiXCY+eaBP6NhtGJTngh2pLaKXdp2YPP1i+C6w@mail.gmail.com>
Hello Rayagond
if patches are ok, can we consider you Acked-by ?
Thx
Peppe
On 10/27/2016 12:51 PM, Rayagond Kokatanur wrote:
> On Thu, Oct 27, 2016 at 4:02 PM, Giuseppe CAVALLARO
> <peppe.cavallaro@st.com> wrote:
>> Hello Rayagond !
>>
>> On 10/27/2016 12:25 PM, Rayagond Kokatanur wrote:
>>>>
>>>> +static int dwmac4_wrback_get_rx_timestamp_status(void *desc, u32 ats)
>>>>> {
>>>>> struct dma_desc *p = (struct dma_desc *)desc;
>>>>> + int ret = -EINVAL;
>>>>> +
>>>>> + /* Get the status from normal w/b descriptor */
>>>>> + if (likely(p->des3 & TDES3_RS1V)) {
>>>>> + if (likely(p->des1 & RDES1_TIMESTAMP_AVAILABLE)) {
>>>>> + int i = 0;
>>>>> +
>>>>> + /* Check if timestamp is OK from context
>>>>> descriptor */
>>>>> + do {
>>>>> + ret = dwmac4_rx_check_timestamp(desc);
>>>
>>> Here, "desc" is not pointing to next descriptor (ie context
>>> descriptor). Driver should check the context descriptor.
>>
>>
>> you are right and this is done by the caller: stmmac_get_rx_hwtstamp
>
> Yes.
>
>>
>> Cheers
>> peppe
>>
>
>
>
^ permalink raw reply
* [PATCH net] net: Check for fullsock in sock_i_uid()
From: Subash Abhinov Kasiviswanathan @ 2016-11-02 5:27 UTC (permalink / raw)
To: netdev; +Cc: Subash Abhinov Kasiviswanathan, Eric Dumazet
sock_i_uid() acquires the sk_callback_lock which does not exist
for sockets in TCP_NEW_SYN_RECV state. This results in errors
showing up as spinlock bad magic.
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Cc: Eric Dumazet <edumazet@google.com>
---
net/core/sock.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/core/sock.c b/net/core/sock.c
index c73e28f..af15ef0 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1727,7 +1727,10 @@ void sock_efree(struct sk_buff *skb)
kuid_t sock_i_uid(struct sock *sk)
{
- kuid_t uid;
+ kuid_t uid = GLOBAL_ROOT_UID;
+
+ if (!sk_fullsock(sk))
+ return uid;
read_lock_bh(&sk->sk_callback_lock);
uid = sk->sk_socket ? SOCK_INODE(sk->sk_socket)->i_uid : GLOBAL_ROOT_UID;
--
1.9.1
^ permalink raw reply related
* [PATCH net] cxgb4: correct device ID of T6 adapter
From: Hariprasad Shenai @ 2016-11-02 5:22 UTC (permalink / raw)
To: netdev; +Cc: davem, leedom, nirranjan, Hariprasad Shenai
Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/t4_pci_id_tbl.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_pci_id_tbl.h b/drivers/net/ethernet/chelsio/cxgb4/t4_pci_id_tbl.h
index 50812a1d67bd..df1573c4a659 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_pci_id_tbl.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_pci_id_tbl.h
@@ -178,9 +178,9 @@ CH_PCI_DEVICE_ID_TABLE_DEFINE_BEGIN
CH_PCI_ID_TABLE_FENTRY(0x6005),
CH_PCI_ID_TABLE_FENTRY(0x6006),
CH_PCI_ID_TABLE_FENTRY(0x6007),
+ CH_PCI_ID_TABLE_FENTRY(0x6008),
CH_PCI_ID_TABLE_FENTRY(0x6009),
CH_PCI_ID_TABLE_FENTRY(0x600d),
- CH_PCI_ID_TABLE_FENTRY(0x6010),
CH_PCI_ID_TABLE_FENTRY(0x6011),
CH_PCI_ID_TABLE_FENTRY(0x6014),
CH_PCI_ID_TABLE_FENTRY(0x6015),
--
2.3.4
^ permalink raw reply related
* Re: Let's do P4
From: Maciej Żenczykowski @ 2016-11-02 5:06 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Jiri Pirko, Alexei Starovoitov, Thomas Graf, John Fastabend,
Jakub Kicinski, Linux NetDev, David Miller, Jamal Hadi Salim,
roopa, simon.horman, ast, prem, Hannes Frederic Sowa, Jiri Benc,
Tom Herbert, mattyk, idosch, eladr, yotamg, nogahf, ogerlitz,
John W. Linville, Andy Gospodarek, Florian Fainelli
In-Reply-To: <58194F83.9020205@iogearbox.net>
> Sorry for jumping into the middle and the delay (plumbers this week). My
> question would be, if the main target is for p4 *offloading* anyway, who
> would use this sw fallback path? Mostly for testing purposes?
>
> I'm not sure about compilerB here and the complexity that needs to be
> pushed into the kernel along with it. I would assume this would result
> in slower code than what the existing P4 -> eBPF front ends for LLVM
> would generate since it could perform all kind of optimizations there,
> that might not be feasible for doing inside the kernel. Thus, if I'd want
> to do that in sw, I'd just use the existing LLVM facilities instead and
> go via cls_bpf in that case.
>
> What is your compilerA? Is that part of tc in user space? Maybe linked
> against LLVM lib, for example? If you really want some sw path, can't tc
> do this transparently from user space instead when it gets a netlink error
> that it cannot get offloaded (and thus switch internally to f_bpf's loader)?
Since we're jumping in the middle ;-)
Ideally we'd have an interface where some generic like program is
loaded into the kernel,
and the kernel core fetches some sort of generic description of the
hardware capabilities,
translates the program and fits as much of it as it can into the hardware,
possibly all of it, and emulates/executes the rest in software.
ie. if hardware can only match on 5 different 10 byte headers, but we
need to match on 7 different 12 byte headers,
we can still use the hardware to help us dispatch straight into 'check
the last 2 bytes, then the last 2 headers' software emulation code.
or maybe hardware can match, but can't count packets... so we need to
implement counting in sw.
or it can't do all types of encap/decap, so we need to sw encap in
certain cases...
Doing this via extracting such information out of a bpf program seems
pretty hard.
Or maybe I'm overestimating the true difficulty of taking a bpf
program and extracting it into a TCAM...
Maybe if the bpf program has a more 'standard' layout
(ie. a tree doing packet parsing/matching, with 'actions' in the
leaves) then it's not so hard?...
Obviously real hardware has significantly more capabilities then just
a tcam at the front of the pipeline...
I'm afraid I lack the knowledge of what the real capabilities of
current (and future...) hardware are...
But maybe we could come up with some sufficiently generic description
of *what* we want accomplished
instead of the precise specifics of how.
^ permalink raw reply
* Re: [PATCH net-next 3/3] tools lib bpf: Sync bpf_map_def with tc
From: Joe Stringer @ 2016-11-02 4:09 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev, wangnan0, ast
In-Reply-To: <581958D1.3010209@iogearbox.net>
On 1 November 2016 at 20:09, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 10/31/2016 07:39 PM, Joe Stringer wrote:
>>
>> TC uses a slightly different map layout in its ELFs. Update libbpf to
>> use the same definition so that ELFs may be built using libbpf and
>> loaded using tc.
>>
>> Signed-off-by: Joe Stringer <joe@ovn.org>
>> ---
>> tools/lib/bpf/libbpf.h | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
>> index dd7a513efb10..ea70c2744f8c 100644
>> --- a/tools/lib/bpf/libbpf.h
>> +++ b/tools/lib/bpf/libbpf.h
>> @@ -181,10 +181,13 @@ bool bpf_program__is_kprobe(struct bpf_program
>> *prog);
>> * and will be treated as an error due to -Werror.
>> */
>> struct bpf_map_def {
>> - unsigned int type;
>> - unsigned int key_size;
>> - unsigned int value_size;
>> - unsigned int max_entries;
>> + uint32_t type;
>> + uint32_t key_size;
>> + uint32_t value_size;
>> + uint32_t max_entries;
>> + uint32_t flags;
>> + uint32_t id;
>> + uint32_t pinning;
>> };
>>
>> /*
>
>
> I think the problem is that this would break existing obj files that have
> been compiled with the current struct bpf_map_def (besides libbpf not having
> a use for the last two members right now).
Right, this is a problem. I wasn't sure whether libbpf was yet at a
stage where it tries to retain compatibility with binaries compiled
against older kernels.
> For tc, we have a refactoring of the tc_bpf.c bits that generalizes them so
> we can move these bits into iproute2 lib part and add new BPF types really
> easily. What I did along with that is to implement a map compat mode, where
> it detects the size of struct bpf_elf_map (or however you want to name it)
> from the obj file and fixes up the missing members with some reasonable
> default,
> so these programs can still be loaded. Thus, the sample code using the
> current
> struct bpf_map_def will then work with tc as well. (I'll post the iproute2
> patch early next week.)
Are you encoding the number of maps into the start of the maps section
in the ELF then using that to divide out and determine the size?
I look forward to your patches. Maybe if TC is more tolerant of other
map definition sizes then this patch is less relevant.
^ permalink raw reply
* Re: [PATCH net-next 2/3] tools lib bpf: Sync with samples/bpf/libbpf
From: Joe Stringer @ 2016-11-02 3:50 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev, wangnan0, ast
In-Reply-To: <58195506.8040205@iogearbox.net>
On 1 November 2016 at 19:52, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 10/31/2016 07:39 PM, Joe Stringer wrote:
>>
>> Extend the tools/ version of libbpf to include all of the functionality
>> provided in the samples/bpf version.
>>
>> Signed-off-by: Joe Stringer <joe@ovn.org>
>> ---
>> tools/lib/bpf/bpf.c | 139 +++++++++++++++++++++++++++------
>> tools/lib/bpf/bpf.h | 208
>> +++++++++++++++++++++++++++++++++++++++++++++++--
>> tools/lib/bpf/libbpf.c | 3 +-
>> 3 files changed, 317 insertions(+), 33 deletions(-)
>>
> [...]
>
>> +int open_raw_sock(const char *name)
>> +{
>> + struct sockaddr_ll sll;
>> + int sock;
>> +
>> + sock = socket(PF_PACKET, SOCK_RAW | SOCK_NONBLOCK | SOCK_CLOEXEC,
>> + htons(ETH_P_ALL));
>> + if (sock < 0) {
>> + printf("cannot create raw socket\n");
>> + return -1;
>> + }
>> +
>> + memset(&sll, 0, sizeof(sll));
>> + sll.sll_family = AF_PACKET;
>> + sll.sll_ifindex = if_nametoindex(name);
>> + sll.sll_protocol = htons(ETH_P_ALL);
>> + if (bind(sock, (struct sockaddr *)&sll, sizeof(sll)) < 0) {
>> + printf("bind to %s: %s\n", name, strerror(errno));
>> + close(sock);
>> + return -1;
>> + }
>> +
>> + return sock;
>> +}
>> +
>> +int perf_event_open(struct perf_event_attr *attr, int pid, int cpu,
>> + int group_fd, unsigned long flags)
>> +{
>> + return syscall(__NR_perf_event_open, attr, pid, cpu,
>> + group_fd, flags);
>> }
>
>
> I'm actually wondering, above bits are not really libbpf related. Maybe
> these should go elsewhere into some other misc header file just for the
> samples to use?
True, I started this patch by just copying the whole file over to
tools/lib/bpf so this came with it. I can bring it back to the samples
directory in v2.
>> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
>> index e8ba54087497..227edb23c022 100644
>> --- a/tools/lib/bpf/bpf.h
>> +++ b/tools/lib/bpf/bpf.h
>> @@ -23,16 +23,208 @@
>>
>> #include <linux/bpf.h>
>>
>> +struct bpf_insn;
>> +
>
>
> Isn't that already defined in the above uapi bpf.h anyway?
Yup, I'll drop this bit.
^ permalink raw reply
* Re: [PATCH net-next 3/3] tools lib bpf: Sync bpf_map_def with tc
From: Daniel Borkmann @ 2016-11-02 3:09 UTC (permalink / raw)
To: Joe Stringer, netdev; +Cc: wangnan0, ast
In-Reply-To: <20161031183917.9938-4-joe@ovn.org>
On 10/31/2016 07:39 PM, Joe Stringer wrote:
> TC uses a slightly different map layout in its ELFs. Update libbpf to
> use the same definition so that ELFs may be built using libbpf and
> loaded using tc.
>
> Signed-off-by: Joe Stringer <joe@ovn.org>
> ---
> tools/lib/bpf/libbpf.h | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index dd7a513efb10..ea70c2744f8c 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -181,10 +181,13 @@ bool bpf_program__is_kprobe(struct bpf_program *prog);
> * and will be treated as an error due to -Werror.
> */
> struct bpf_map_def {
> - unsigned int type;
> - unsigned int key_size;
> - unsigned int value_size;
> - unsigned int max_entries;
> + uint32_t type;
> + uint32_t key_size;
> + uint32_t value_size;
> + uint32_t max_entries;
> + uint32_t flags;
> + uint32_t id;
> + uint32_t pinning;
> };
>
> /*
I think the problem is that this would break existing obj files that have
been compiled with the current struct bpf_map_def (besides libbpf not having
a use for the last two members right now).
For tc, we have a refactoring of the tc_bpf.c bits that generalizes them so
we can move these bits into iproute2 lib part and add new BPF types really
easily. What I did along with that is to implement a map compat mode, where
it detects the size of struct bpf_elf_map (or however you want to name it)
from the obj file and fixes up the missing members with some reasonable default,
so these programs can still be loaded. Thus, the sample code using the current
struct bpf_map_def will then work with tc as well. (I'll post the iproute2
patch early next week.)
^ permalink raw reply
* Re: [PATCH net-next 2/3] tools lib bpf: Sync with samples/bpf/libbpf
From: Daniel Borkmann @ 2016-11-02 2:52 UTC (permalink / raw)
To: Joe Stringer, netdev; +Cc: wangnan0, ast
In-Reply-To: <20161031183917.9938-3-joe@ovn.org>
On 10/31/2016 07:39 PM, Joe Stringer wrote:
> Extend the tools/ version of libbpf to include all of the functionality
> provided in the samples/bpf version.
>
> Signed-off-by: Joe Stringer <joe@ovn.org>
> ---
> tools/lib/bpf/bpf.c | 139 +++++++++++++++++++++++++++------
> tools/lib/bpf/bpf.h | 208 +++++++++++++++++++++++++++++++++++++++++++++++--
> tools/lib/bpf/libbpf.c | 3 +-
> 3 files changed, 317 insertions(+), 33 deletions(-)
>
[...]
> +int open_raw_sock(const char *name)
> +{
> + struct sockaddr_ll sll;
> + int sock;
> +
> + sock = socket(PF_PACKET, SOCK_RAW | SOCK_NONBLOCK | SOCK_CLOEXEC,
> + htons(ETH_P_ALL));
> + if (sock < 0) {
> + printf("cannot create raw socket\n");
> + return -1;
> + }
> +
> + memset(&sll, 0, sizeof(sll));
> + sll.sll_family = AF_PACKET;
> + sll.sll_ifindex = if_nametoindex(name);
> + sll.sll_protocol = htons(ETH_P_ALL);
> + if (bind(sock, (struct sockaddr *)&sll, sizeof(sll)) < 0) {
> + printf("bind to %s: %s\n", name, strerror(errno));
> + close(sock);
> + return -1;
> + }
> +
> + return sock;
> +}
> +
> +int perf_event_open(struct perf_event_attr *attr, int pid, int cpu,
> + int group_fd, unsigned long flags)
> +{
> + return syscall(__NR_perf_event_open, attr, pid, cpu,
> + group_fd, flags);
> }
I'm actually wondering, above bits are not really libbpf related. Maybe
these should go elsewhere into some other misc header file just for the
samples to use?
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index e8ba54087497..227edb23c022 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -23,16 +23,208 @@
>
> #include <linux/bpf.h>
>
> +struct bpf_insn;
> +
Isn't that already defined in the above uapi bpf.h anyway?
> int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size,
> - int max_entries);
> + int max_entries, int map_flags);
> +int bpf_update_elem(int fd, void *key, void *value, unsigned long long flags);
> +int bpf_lookup_elem(int fd, void *key, void *value);
> +int bpf_delete_elem(int fd, void *key);
> +int bpf_get_next_key(int fd, void *key, void *next_key);
> +
> +int bpf_load_program(enum bpf_prog_type prog_type,
> + const struct bpf_insn *insns, int insn_len,
> + const char *license, int kern_version,
> + char *log_buf, size_t log_buf_sz);
> +
> +int bpf_obj_pin(int fd, const char *pathname);
> +int bpf_obj_get(const char *pathname);
^ permalink raw reply
* Re: [PATCH net-next 0/3] tools lib bpf: Synchronize implementations
From: Daniel Borkmann @ 2016-11-02 2:46 UTC (permalink / raw)
To: Joe Stringer, David Ahern; +Cc: netdev, wangnan0, ast
In-Reply-To: <CAPWQB7HtmX7+v37q9rH_3PD+QfSVg_uRiP_=5ZoKnwsF8gpyKg@mail.gmail.com>
On 11/01/2016 11:17 PM, Joe Stringer wrote:
> On 1 November 2016 at 13:51, David Ahern <dsa@cumulusnetworks.com> wrote:
>> On 10/31/16 12:39 PM, Joe Stringer wrote:
>>> Update tools/lib/bpf to provide more functionality and improve interoperation
>>> with other tools that generate and use eBPF code.
>>>
>>> The kernel uapi headers are a bit newer than the version in the tools/
>>> directory; synchronize those.
>>>
>>> samples/bpf/libbpf* has a bit more functionality than tools/lib/bpf, so extend
>>> tools/lib/bpf/bpf* with these functions to bring them into parity.
>>>
>>> tools/lib/bpf cannot read ELFs that tc can read, and vice versa. Update the
>>> map definition to be the same as in tc so the ELFs may be interchangeable
>>> (at least for now; I don't have a long-term plan in mind to ensure this always
>>> works).
>>
>> can samples/bpf be converted to use tools/lib/bpf/libbpf.a?
>
> I have a few other patches sitting around that need this series,
> including an attempt at this.
That sounds great, looking forward!
^ permalink raw reply
* Re: Let's do P4
From: Daniel Borkmann @ 2016-11-02 2:29 UTC (permalink / raw)
To: Jiri Pirko, Alexei Starovoitov
Cc: Thomas Graf, John Fastabend, Jakub Kicinski, netdev, davem, jhs,
roopa, simon.horman, ast, prem, hannes, jbenc, tom, mattyk,
idosch, eladr, yotamg, nogahf, ogerlitz, linville, andy,
f.fainelli, dsa, vivien.didelot, andrew, ivecera,
Maciej Żenczykowski
In-Reply-To: <20161031093922.GA2895@nanopsycho.orion>
On 10/31/2016 10:39 AM, Jiri Pirko wrote:
> Sun, Oct 30, 2016 at 11:39:05PM CET, alexei.starovoitov@gmail.com wrote:
>> On Sun, Oct 30, 2016 at 05:38:36PM +0100, Jiri Pirko wrote:
>>> Sun, Oct 30, 2016 at 11:26:49AM CET, tgraf@suug.ch wrote:
>>>> On 10/30/16 at 08:44am, Jiri Pirko wrote:
>>>>> Sat, Oct 29, 2016 at 06:46:21PM CEST, john.fastabend@gmail.com wrote:
>>>>>> On 16-10-29 07:49 AM, Jakub Kicinski wrote:
>>>>>>> On Sat, 29 Oct 2016 09:53:28 +0200, Jiri Pirko wrote:
>>>>>>>> Hi all.
>>
>> sorry for delay. travelling to KS, so probably missed something in
>> this thread and comments can be totally off...
>>
>> the subject "let's do P4" is imo misleading, since it reads like
>> we don't do P4 at the moment, whereas the opposite is true.
>> Several p4->bpf compilers is a proof.
>
> We don't do p4 in kernel now, we don't do p4 offloading now. That is
> the reason I started this discussion.
>
>>> The network world is divided into 2 general types of hw:
>>> 1) network ASICs - network specific silicon, containing things like TCAM
>>> These ASICs are suitable to be programmed by P4.
>>
>> i think the opposite is the case in case of P4.
>> when hw asic has tcam it's still far far away from being usable with P4
>> which requires fully programmable protocol parser, arbitrary tables and so on.
>> P4 doesn't even define TCAM as a table type. The p4 program can declare
>> a desired algorithm of search in the table and compiler has to figure out
>> what HW resources to use to satisfy such p4 program.
>>
>>> 2) network processors - basically a general purpose CPUs
>>> These processors are suitable to be programmed by eBPF.
>>
>> I think this statement is also misleading, since it positions
>> p4 and bpf as competitors whereas that's not the case.
>> p4 is the language. bpf is an instruction set.
>
> I wanted to say that we are having 2 approaches in silicon, 2 different
> paradigms. Sure you can do p4>bpf. But hard to do it the opposite way.
>
>>> Exactly. Following drawing shows p4 pipeline setup for SW and Hw:
>>>
>>> |
>>> | +--> ebpf engine
>>> | |
>>> | |
>>> | compilerB
>>> | ^
>>> | |
>>> p4src --> compilerA --> p4ast --TCNL--> cls_p4 --+-> driver -> compilerC -> HW
>>> |
>>> userspace | kernel
>>> |
Sorry for jumping into the middle and the delay (plumbers this week). My
question would be, if the main target is for p4 *offloading* anyway, who
would use this sw fallback path? Mostly for testing purposes?
I'm not sure about compilerB here and the complexity that needs to be
pushed into the kernel along with it. I would assume this would result
in slower code than what the existing P4 -> eBPF front ends for LLVM
would generate since it could perform all kind of optimizations there,
that might not be feasible for doing inside the kernel. Thus, if I'd want
to do that in sw, I'd just use the existing LLVM facilities instead and
go via cls_bpf in that case.
What is your compilerA? Is that part of tc in user space? Maybe linked
against LLVM lib, for example? If you really want some sw path, can't tc
do this transparently from user space instead when it gets a netlink error
that it cannot get offloaded (and thus switch internally to f_bpf's loader)?
^ permalink raw reply
* Re: [PATCH net-next v2] ipv4: fib: Replay events when registering FIB notifier
From: Roopa Prabhu @ 2016-11-02 2:13 UTC (permalink / raw)
To: Ido Schimmel
Cc: Eric Dumazet, netdev, davem, jiri, mlxsw, dsa, nikolay, andy,
vivien.didelot, andrew, f.fainelli, alexander.h.duyck, kuznet,
jmorris, yoshfuji, kaber, Ido Schimmel
In-Reply-To: <20161101170345.pq2ewecw35mrurkp@splinter>
On 11/1/16, 10:03 AM, Ido Schimmel wrote:
> Hi Roopa,
>
> On Tue, Nov 01, 2016 at 08:14:14AM -0700, Roopa Prabhu wrote:
>>
[snip]
>> I have the same concern as Eric here.
>>
>> I understand why you need it, but can the driver request for an initial dump and that
>> dump be made more efficient somehow ie not hold rtnl for the whole dump ?.
>> instead of making the fib notifier registration code doing it.
> We can do what we suggested in the last bi-weekly meeting, which is
> still holding rtnl, but moving the hardware operation to delayed work.
> This is possible because upper layers always assume operation was
> successful and driver is responsible for invoking its abort mechanism in
> case of failure.
>
>> these routing table sizes can be huge and an analogy for this in user-space:
>> We do request a netlink dump of routing tables at initialization (on driver starts or resets)...
>> but, existing netlink routing table dumps for that scale don't hold rtnl for the whole dump.
>> The dump is split into multiple responses to the user and hence it does not starve other rtnl users.
> In my reply to Eric I mentioned that when we register and unregister
> from this chain the tables aren't really huge, but instead quite small.
> I understand your concerns, but I don't wish to make things more
> complicated than they should be only to address concerns that aren't
> really realistic.
I understand..but, if you are adding some core infrastructure for switchdev ..it cannot be
based on the number of simple use-cases or data you have today.
I won't be surprised if tomorrow other switch drivers have a case where they need to
reset the hw routing table state and reprogram all routes again. Re-registering the notifier to just
get the routing state of the kernel will not scale. For the long term, since the driver does not maintain a cache,
a pull api with efficient use of rtnl will be useful for other such cases as well.
If you don't want to get to the complexity of a new api right away because of the
simple case of management interface routes you have, Can your driver register the notifier early ?
(I am sure you have probably already thought about this)
>
> I believe current patch is quite simple and also consistent with other
> notification chains in the kernel, such as the netdevice, where rtnl is
> held during replay of events.
> http://lxr.free-electrons.com/source/net/core/dev.c#L1535
as you know, netdev and routing scale are not the same thing.
Looking at the current code for netdevices (replay and rollback on failure),
a pull api (equivalent to the netlink dump api) may end up being less complex...with an
ability to batch in the future.
^ permalink raw reply
* Re: [Patch net] inet: fix sleeping inside inet_wait_for_connect()
From: Eric Dumazet @ 2016-11-02 1:54 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, Andrey Konovalov, Peter Zijlstra
In-Reply-To: <1478041476-5568-1-git-send-email-xiyou.wangcong@gmail.com>
On Tue, 2016-11-01 at 16:04 -0700, Cong Wang wrote:
> Andrey reported this kernel warning:
> Unlike commit 26cabd31259ba43f68026ce3f62b78094124333f
> ("sched, net: Clean up sk_wait_event() vs. might_sleep()"), the
> sleeping function is called before schedule_timeout(), this is indeed
> a bug. Fix this by moving the wait logic to the new API, it is similar
> to commit ff960a731788a7408b6f66ec4fd772ff18833211
> ("netdev, sched/wait: Fix sleeping inside wait event").
>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Cc: Andrey Konovalov <andreyknvl@google.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
Excellent.
I guess we could also define sk_wait_event_woken()
and use it instead of sk_wait_event(), and also in
inet_wait_for_connect()
+#define sk_wait_event_woken(__sk, __timeo, __condition, __wait) \
+ ({ int __rc; \
+ release_sock(__sk); \
+ __rc = __condition; \
+ if (!__rc) { \
+ *(__timeo) = wait_woken(__wait, TASK_INTERRUPTIBLE, \
+ *(__timeo)); \
+ } \
+ lock_sock(__sk); \
+ __rc = __condition; \
+ __rc; \
+ })
sk_wait_data() would need :
@@ -2078,14 +2080,14 @@ void __sk_flush_backlog(struct sock *sk)
*/
int sk_wait_data(struct sock *sk, long *timeo, const struct sk_buff *skb)
{
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
int rc;
- DEFINE_WAIT(wait);
- prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
+ add_wait_queue(sk_sleep(sk), &wait);
sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
- rc = sk_wait_event(sk, timeo, skb_peek_tail(&sk->sk_receive_queue) != skb);
+ rc = sk_wait_event_woken(sk, timeo, skb_peek_tail(&sk->sk_receive_queue) != skb, &wait);
sk_clear_bit(SOCKWQ_ASYNC_WAITDATA, sk);
- finish_wait(sk_sleep(sk), &wait);
+ remove_wait_queue(sk_sleep(sk), &wait);
return rc;
}
EXPORT_SYMBOL(sk_wait_data);
^ permalink raw reply
* Re: [PATCH v7 5/6] net: ipv4, ipv6: run cgroup eBPF egress programs
From: Daniel Borkmann @ 2016-11-02 1:17 UTC (permalink / raw)
To: David Miller, daniel-cYrQPVfZoowdnm+yROfE0A
Cc: htejun-b10kYP2dOMg, ast-b10kYP2dOMg, kafai-b10kYP2dOMg,
fw-HFFVJYpyMKqzQB+pC5nmwQ, pablo-Cap9r6Oaw4JrovVCs/uTlw,
harald-H+wXaHxf7aLQT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
sargun-GaZTRHToo+CzQB+pC5nmwQ, cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161031.124003.1361406552151798940.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On 10/31/2016 05:40 PM, David Miller wrote:
> From: Daniel Mack <daniel-cYrQPVfZoowdnm+yROfE0A@public.gmane.org>
> Date: Tue, 25 Oct 2016 12:14:13 +0200
>
>> @@ -312,6 +314,13 @@ int ip_mc_output(struct net *net, struct sock *sk, struct sk_buff *skb)
>> skb->dev = dev;
>> skb->protocol = htons(ETH_P_IP);
>>
>> + ret = cgroup_bpf_run_filter(sk_to_full_sk(sk), skb,
>> + BPF_CGROUP_INET_EGRESS);
>> + if (ret) {
>> + kfree_skb(skb);
>> + return ret;
>> + }
>> +
>> /*
>> * Multicasts are looped back for other local users
>> */
>> @@ -364,12 +373,20 @@ int ip_mc_output(struct net *net, struct sock *sk, struct sk_buff *skb)
>> int ip_output(struct net *net, struct sock *sk, struct sk_buff *skb)
>> {
>> struct net_device *dev = skb_dst(skb)->dev;
>> + int ret;
>>
>> IP_UPD_PO_STATS(net, IPSTATS_MIB_OUT, skb->len);
>>
>> skb->dev = dev;
>> skb->protocol = htons(ETH_P_IP);
>>
>> + ret = cgroup_bpf_run_filter(sk_to_full_sk(sk), skb,
>> + BPF_CGROUP_INET_EGRESS);
>> + if (ret) {
>> + kfree_skb(skb);
>> + return ret;
>> + }
>> +
>> return NF_HOOK_COND(NFPROTO_IPV4, NF_INET_POST_ROUTING,
>> net, sk, skb, NULL, dev,
>> ip_finish_output,
>
> The "sk" here is not necessarily the application socket. It could be
> a UDP tunnel socket or similar encapsulation object.
>
> "skb->sk" is always the application socket, so is probably what you
> need to pass down into the cgroup bpf run filter hook.
Wouldn't that mean however, when you go through stacked devices that
you'd run the same eBPF cgroup program for skb->sk multiple times?
(Except for things like crossing netns due to the skb_orphan() there.)
For example, when you're doing accounting through this facility, then
TX part is counted X times instead of ideally just once, and for RX
afaik, we seem to short-cut sk_filter_trim_cap() on the encaps anyway.
I guess one could work around this by marking the skb with a verdict
once it was accounted for the first time, either from within eBPF prog
or from stack side. For eBPF we'd need to implement our own .is_valid_access()
callback for the verifier to allow writing into skb meta data like skb->mark
(sk filter can only write into cb[] for passing data between tail calls)
plus making sure in either case that no other user is mangling that flag
further on; hmm, probably rather unintuitive and fragile.
Unless I'm missing something obvious, I would currently think that just
passing "sk" might be more correct.
^ permalink raw reply
* Re: [PATCH] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC
From: John Heenan @ 2016-11-02 1:13 UTC (permalink / raw)
To: Jes Sorensen, Barry Day, Rafał Miłecki
Cc: Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <CAAye0QNjYY5gBzSdc+0_rhy1OHk7iQ-CVp0Svph1DR7+mgVWGg@mail.gmail.com>
Barry Day has submitted real world reports for the 8192eu and 8192cu.
This needs to be acknowledged. I have submitted real world reports for
the 8723bu.
When it comes down to it, it looks like the kernel code changes are
really going to be very trivial to fix this problem and we need to
take the focus off dramatic outbursts over style issues to a strategy
for getting usable results from real world testing.
Addressing style issues in a dramatic manner to me looks like a mean
sport for maintainers who line up to easy target first time
contributors. This mean attitude comes from the top with a well known
comment about "publicly making fun of people". The polite comments
over style from Joe Perches and Rafał Miłecki are welcomed.
An effective strategy would be to insert some printk statements to
trace what init steps vendor derived drivers do each time
wpa_supplicant is called and ask real world testers to report their
results. This is a lot more productive and less error prone than
laboriously pouring over vendor source code. Alternative drivers that
use vendor code from Realtek is enormously complicated and a huge pain
to make sense of.
Joe Sorensen's driver code is far easier to make sense of and it is a
shame Realtek don't come to the party. Joe Sorensens's code take takes
advantage of the excellent work of kernel contributors to the mac80211
driver.
Previous comments I made about enable_rf, rtl8xxxu_start,
rtl8xxxu_init_device etc should be clarified. I will leave it for the
moment as it currently serves no direct useful purpose.
John Heenan
On 1 November 2016 at 17:24, John Heenan <john@zgus.com> wrote:
> I have a prepared another patch that is not USB VID/PID dependent for
> rtl8723bu devices. It is more elegant. I will send it after this
> email.
>
> If I have more patches is it preferable I just put them on github only
> and notify a link address until there might be some resolution?
>
> What I meant below about not finding a matching function is that I
> cannot find matching actions to take on any function calls in
> rtl8xxxu_stop as for rtl8xxxu_start.
>
> The function that is called later and potentially more than once for
> rtl8723bu devices is rtl8xxxu_init_device. There is no corresponding
> rtl8xxxu_deinit_device function.
>
> enable_rf is called in rtl8xxxu_start, not in the delayed call to
> rtl8xxxu_init_device. The corresponding disable_rf is called in
> rtl8xxxu_stop. So no matching issue here. However please see below for
> another potential issue.
>
> power_on is called in rtl8xxxu_init_device. power_off is called in
> rtl8xxxu_disconnect. Does not appear to be an issue if calling
> power_on has no real effect if already on.
>
> The following should be looked at though. For all devices enable_rf is
> only called once. For the proposed patch the first call to
> rtl8xxxu_init_device is called after the single call to enable_rf for
> rtl8723bu devices. Without the patch enable_rf is called after
> rtl8xxxu_init_device for all devices
>
> Perhaps enable_rf just configures RF modes to start up when RF is
> powered on or if called after power_on then enters requested RF modes.
>
> So I cannot see appropriate additional matching action to take.
>
> Below is some background for anyone interested
>
> rtl8xxxu_start and rtl8xxxu_stop are assigned to rtl8xxxu_ops.
>
> rtl8xxxu_probe assigns rtl8xxxu_ops to its driver layer with
> ieee80211_register_hw.
>
> rtl8xxxu_disconnect unassigns rtl8xxxu_ops from its driver layer with
> ieee80211_unregister_hw.
>
> rtl8xxxu_probe and rtl8xxxu_disconnect are USB driver functions
>
> John Heenan
>
>
> On 1 November 2016 at 08:15, John Heenan <john@zgus.com> wrote:
>> On 1 November 2016 at 07:25, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
>>> John Heenan <john@zgus.com> writes:
>>
>>>
>>>> @@ -5776,9 +5781,17 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
>>>> struct rtl8xxxu_tx_urb *tx_urb;
>>>> unsigned long flags;
>>>> int ret, i;
>>>> + struct usb_device_descriptor *udesc = &priv->udev->descriptor;
>>>>
>>>> ret = 0;
>>>>
>>>> + if(udesc->idVendor == USB_VENDOR_ID_REALTEK
>>>> + && udesc->idProduct == USB_PRODUCT_ID_RTL8723BU) {
>>>> + ret = rtl8xxxu_init_device(hw);
>>>> + if (ret)
>>>> + goto error_out;
>>>> + }
>>>> +
>>>
>>> As mentioned previously, if this is to be changed here, it has to be
>>> matched in the _stop section too.
>>
>> I looked at this and could not find a matching function. I will have a
>> look again.
>>
^ permalink raw reply
* Re: [PATCH net-next 08/11] net: dsa: mv88e6xxx: add port duplex setter
From: Vivien Didelot @ 2016-11-02 1:12 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <20161101094440.GC2802@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
>> int (*port_set_link)(struct mv88e6xxx_chip *chip, int port, int link);
>> +
>> + /* Port's MAC duplex mode
>> + *
>> + * Use DUPLEX_HALF or DUPLEX_FULL to force half or full duplex, or
>> + * DUPLEX_UNKNOWN for normal duplex detection.
>> + */
>> + int (*port_set_duplex)(struct mv88e6xxx_chip *chip, int port, int dup);
>
> Again, i think DUPLEX_UNFORCED, or DUPLEX_AUTO would be better.
DUPLEX_UNKNOWN and SPEED_UNKNOWN are existing values used by the
phy/ethtool interface for the phy_device. I prefered to use them when
possible instead of creating new ones.
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCH net-next 07/11] net: dsa: mv88e6xxx: add port link setter
From: Vivien Didelot @ 2016-11-02 1:07 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <20161101094313.GB2802@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
>> +#define LINK_UNKNOWN -1
>> +
>> + /* Port's MAC link state
>> + * LINK_UNKNOWN for normal link detection, 0 to force link down,
>> + * otherwise force link up.
>> + */
>> + int (*port_set_link)(struct mv88e6xxx_chip *chip, int port, int link);
>
> Maybe LINK_AUTO would be better than UNKNOWN? Or LINK_UNFORCED.
I used LINK_UNKNOWN to be consistent with the supported SPEED_UNKNOWN
and DUPLEX_UNKNOWN values of PHY devices.
Thanks,
Vivien
^ permalink raw reply
* [PATCH 1/1] xen-netfront: cast grant table reference first to type int
From: Dongli Zhang @ 2016-11-02 1:04 UTC (permalink / raw)
To: netdev, linux-kernel, xen-devel; +Cc: davem
IS_ERR_VALUE() in commit 87557efc27f6a50140fb20df06a917f368ce3c66
("xen-netfront: do not cast grant table reference to signed short") would
not return true for error code unless we cast ref first to type int.
Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
---
drivers/net/xen-netfront.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 189a28d..bf2744e 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -304,7 +304,7 @@ static void xennet_alloc_rx_buffers(struct netfront_queue *queue)
queue->rx_skbs[id] = skb;
ref = gnttab_claim_grant_reference(&queue->gref_rx_head);
- WARN_ON_ONCE(IS_ERR_VALUE((unsigned long)ref));
+ WARN_ON_ONCE(IS_ERR_VALUE((unsigned long)(int)ref));
queue->grant_rx_ref[id] = ref;
page = skb_frag_page(&skb_shinfo(skb)->frags[0]);
@@ -428,7 +428,7 @@ static void xennet_tx_setup_grant(unsigned long gfn, unsigned int offset,
id = get_id_from_freelist(&queue->tx_skb_freelist, queue->tx_skbs);
tx = RING_GET_REQUEST(&queue->tx, queue->tx.req_prod_pvt++);
ref = gnttab_claim_grant_reference(&queue->gref_tx_head);
- WARN_ON_ONCE(IS_ERR_VALUE((unsigned long)ref));
+ WARN_ON_ONCE(IS_ERR_VALUE((unsigned long)(int)ref));
gnttab_grant_foreign_access_ref(ref, queue->info->xbdev->otherend_id,
gfn, GNTMAP_readonly);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 04/11] net: dsa: mv88e6xxx: add port FID accessors
From: Vivien Didelot @ 2016-11-02 1:01 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <20161101093819.GA2802@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
>> --- a/drivers/net/dsa/mv88e6xxx/port.c
>> +++ b/drivers/net/dsa/mv88e6xxx/port.c
>> @@ -61,6 +61,8 @@ int mv88e6xxx_port_set_state(struct mv88e6xxx_chip *chip, int port, u8 state)
>> return 0;
>> }
>>
>> +/* Offset 0x05: Port Control 1 */
>> +
>> /* Offset 0x06: Port Based VLAN Map */
>>
>> int mv88e6xxx_port_set_vlan_map(struct mv88e6xxx_chip *chip, int port, u16 map)
>
> This comment seems in an odd place.
It isn't. Port's default FID uses register 0x05 and I document them
above the related functions.
Thanks,
Vivien
^ permalink raw reply
* [PATCH net-next] enic: set skb->hash type properly
From: Govindarajulu Varadarajan @ 2016-11-02 0:58 UTC (permalink / raw)
To: davem, benve, netdev; +Cc: Govindarajulu Varadarajan
From: Govindarajulu Varadarajan <_govind@gmx.com>
Driver sets the skb l4/l3 hash based on NIC_CFG_RSS_HASH_TYPE_*,
which is bit mask. This is wrong. Hw actually provides us enum.
Use CQ_ENET_RQ_DESC_RSS_TYPE_* to set l3 and l4 hash type.
Fixes: bf751ba802fe ("driver/net: enic: record q_number and rss_hash for skb")
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
drivers/net/ethernet/cisco/enic/enic.h | 2 +-
drivers/net/ethernet/cisco/enic/enic_main.c | 18 ++++++++++++------
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/cisco/enic/enic.h b/drivers/net/ethernet/cisco/enic/enic.h
index 130f910..9023c85 100644
--- a/drivers/net/ethernet/cisco/enic/enic.h
+++ b/drivers/net/ethernet/cisco/enic/enic.h
@@ -33,7 +33,7 @@
#define DRV_NAME "enic"
#define DRV_DESCRIPTION "Cisco VIC Ethernet NIC Driver"
-#define DRV_VERSION "2.3.0.20"
+#define DRV_VERSION "2.3.0.31"
#define DRV_COPYRIGHT "Copyright 2008-2013 Cisco Systems, Inc"
#define ENIC_BARS_MAX 6
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index f514faf..cdd7a1a 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -1166,12 +1166,18 @@ static void enic_rq_indicate_buf(struct vnic_rq *rq,
skb->protocol = eth_type_trans(skb, netdev);
skb_record_rx_queue(skb, q_number);
if (netdev->features & NETIF_F_RXHASH) {
- skb_set_hash(skb, rss_hash,
- (rss_type &
- (NIC_CFG_RSS_HASH_TYPE_TCP_IPV6_EX |
- NIC_CFG_RSS_HASH_TYPE_TCP_IPV6 |
- NIC_CFG_RSS_HASH_TYPE_TCP_IPV4)) ?
- PKT_HASH_TYPE_L4 : PKT_HASH_TYPE_L3);
+ switch (rss_type) {
+ case CQ_ENET_RQ_DESC_RSS_TYPE_TCP_IPv4:
+ case CQ_ENET_RQ_DESC_RSS_TYPE_TCP_IPv6:
+ case CQ_ENET_RQ_DESC_RSS_TYPE_TCP_IPv6_EX:
+ skb_set_hash(skb, rss_hash, PKT_HASH_TYPE_L4);
+ break;
+ case CQ_ENET_RQ_DESC_RSS_TYPE_IPv4:
+ case CQ_ENET_RQ_DESC_RSS_TYPE_IPv6:
+ case CQ_ENET_RQ_DESC_RSS_TYPE_IPv6_EX:
+ skb_set_hash(skb, rss_hash, PKT_HASH_TYPE_L3);
+ break;
+ }
}
/* Hardware does not provide whole packet checksum. It only
--
2.10.2
^ permalink raw reply related
* Re: [PATCH] net: 3com: typhoon: use new api ethtool_{get|set}_link_ksettings
From: David Dillow @ 2016-11-01 23:31 UTC (permalink / raw)
To: Philippe Reynes; +Cc: davem, netdev, linux-kernel
In-Reply-To: <1478041911-22188-1-git-send-email-tremyfr@gmail.com>
On Wed, 2016-11-02 at 00:11 +0100, Philippe Reynes wrote:
> The ethtool api {get|set}_settings is deprecated.
> We move this driver to new api {get|set}_link_ksettings.
>
> Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
LGTM.
Reviewed-by: David Dillow <dave@thedillows.org>
^ 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