* Re: [PATCH v3 net-next 0/5] ulp: Generalize ULP infrastructure
From: John Fastabend @ 2017-08-08 19:30 UTC (permalink / raw)
To: Tom Herbert
Cc: Tom Herbert, Linux Kernel Network Developers, Rohit Seth,
Dave Watson
In-Reply-To: <CALx6S37HW6VF1ed5RQx7HCNMGM8pRpb54R1OSFHN8mDNpsc1uQ@mail.gmail.com>
On 08/08/2017 10:04 AM, Tom Herbert wrote:
> On Tue, Aug 8, 2017 at 8:31 AM, John Fastabend <john.fastabend@gmail.com> wrote:
>> On 08/07/2017 10:28 AM, Tom Herbert wrote:
>>> Generalize the ULP infrastructure that was recently introduced to
>>> support kTLS. This adds a SO_ULP socket option and creates new fields in
>>> sock structure for ULP ops and ULP data. Also, the interface allows
>>> additional per ULP parameters to be set so that a ULP can be pushed
>>> and operations started in one shot.
>>>
>>> In this patch set:
>>> - Minor dependency fix in inet_common.h
>>> - Implement ULP infrastructure as a socket mechanism
>>> - Fixes TCP and TLS to use the new method (maintaining backwards
>>> API compatibility)
>>> - Adds a ulp.txt document
>>>
>>> Tested: Ran simple ULP. Dave Watson verified kTLS works.
>>>
>>> -v2: Fix compilation errors when CONFIG_ULP_SOCK not set.
>>> -v3: Fix one more build issue, check that sk_protocol is IPPROTO_TCP
>>> in tsl_init. Also, fix a couple of minor issues related to
>>> introducing locked versions of sendmsg, send page. Thanks to
>>> Dave Watson, John Fastabend, and Mat Martineau for testing and
>>> providing fixes.
>>>
>>
>>
>> Hi Tom, Dave,
>>
>> I'm concerned about the performance impact of walking a list and
>> doing string compares on every socket we create with kTLS. Dave
>> do you have any request/response tests for kTLS that would put pressure
>> on the create/destroy time of this infrastructure? We should do some
>> tests with dummy entries in the ULP list to understand the impact of
>> this list walk.
>>
>> I like the underlying TCP generalized hooks, but do we really expect a
>> lot of these hooks to exist? If we only have two on the roadmap
>> (kTLS and socktap) it seems a bit overkill. Further, if we really expect
>> many ULP objects then the list walk and compare will become more expensive
>> perhaps becoming noticeable in request per second metrics.
>>
>> Why not just create another socktap socketopt? That will be better from
>> complexity and likely performance sides.
>>
> IMO, given that there is at most two even proposed at this point I
> don't there's much point addressing performance. When ULP feature
> catches on and we start see a whole bunch of them then it's
> straightforward to use a hash table or some more efficient mechanism.
>
OTOH these optimizations are usually easiest to do at the beginning. And
building an enum of ULP types would allow removing string comparisons and
to do simpler unsigned comparisons. I wont complain too much here though
because this series didn't introduce the lists.
> Tom
>
>> Thanks,
>> .John
>>
^ permalink raw reply
* Re: [PATCH net] net: sched: fix NULL pointer dereference when action calls some targets
From: Cong Wang @ 2017-08-08 19:33 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, David Miller, netfilter-devel, Jamal Hadi Salim
In-Reply-To: <CADvbK_fuuBrh+eOuhQU+SYs9uazH2TRP+KUmhxU=hNZ=EkOT=Q@mail.gmail.com>
On Mon, Aug 7, 2017 at 7:33 PM, Xin Long <lucien.xin@gmail.com> wrote:
> On Tue, Aug 8, 2017 at 9:15 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> This looks like a completely API burden?
> netfilter xt targets are not really compatible with netsched action.
> I've got to say, the patch is just a way to make checkentry return
> false and avoid panic. like [1] said
I don't doubt you fix a crash, I am thinking if we can
"fix" the API instead of fixing the caller.
I am not familiar with this API, so just my 2 cents...
^ permalink raw reply
* Re: [PATCH v3 net-next 4/5] tcp: Adjust TCP ULP to defer to sockets ULP
From: John Fastabend @ 2017-08-08 19:37 UTC (permalink / raw)
To: Tom Herbert, netdev; +Cc: rohit, davejwatson
In-Reply-To: <20170807172818.31855-5-tom@quantonium.net>
On 08/07/2017 10:28 AM, Tom Herbert wrote:
> Fix TCP and TLS to use the newer ULP infrastructure in sockets.
>
> Signed-off-by: Tom Herbert <tom@quantonium.net>
> ---
Looks OK to me.
Acked-by: John Fastabend <john.fastabend@gmail.com>
^ permalink raw reply
* Re: Re:Re: Re: [PATCH net] ppp: Fix a scheduling-while-atomic bug in del_chan
From: Cong Wang @ 2017-08-08 19:45 UTC (permalink / raw)
To: Gao Feng; +Cc: xeb, David Miller, Linux Kernel Network Developers
In-Reply-To: <16ae6009.7a67.15dbf64b398.Coremail.gfree.wind@vip.163.com>
On Mon, Aug 7, 2017 at 6:10 PM, Gao Feng <gfree.wind@vip.163.com> wrote:
>
> Sorry, I don't get you clearly. Why the sock_hold() isn't helpful?
I already told you, the dereference happends before sock_hold().
sock = rcu_dereference(callid_sock[call_id]);
if (sock) {
opt = &sock->proto.pptp;
if (opt->dst_addr.sin_addr.s_addr != s_addr) <=== HERE
sock = NULL;
else
sock_hold(sk_pppox(sock));
}
If we don't wait for readers properly, sock could be freed at the
same time when deference it.
> The pptp_release invokes synchronize_rcu after del_chan, it could make sure the others has increased the sock refcnt if necessary
> and the lookup is over.
> There is no one could get the sock after synchronize_rcu in pptp_release.
If this were true, then this code in pptp_sock_destruct()
would be unneeded:
if (!(sk->sk_state & PPPOX_DEAD)) {
del_chan(pppox_sk(sk));
pppox_unbind_sock(sk);
}
>
>
> But I think about another problem.
> It seems the pptp_sock_destruct should not invoke del_chan and pppox_unbind_sock.
> Because when the sock refcnt is 0, the pptp_release must have be invoked already.
>
I don't know. Looks like sock_orphan() is only called
in pptp_release(), but I am not sure if there is a case
we call sock destructor before release.
Also note, this socket is very special, it doesn't support
poll(), sendmsg() or recvmsg()..
^ permalink raw reply
* Re: [PATCH v3 net-next 0/5] ulp: Generalize ULP infrastructure
From: Tom Herbert @ 2017-08-08 19:50 UTC (permalink / raw)
To: John Fastabend
Cc: Tom Herbert, Linux Kernel Network Developers, Rohit Seth,
Dave Watson
In-Reply-To: <598A1146.9090205@gmail.com>
On Tue, Aug 8, 2017 at 12:30 PM, John Fastabend
<john.fastabend@gmail.com> wrote:
> On 08/08/2017 10:04 AM, Tom Herbert wrote:
>> On Tue, Aug 8, 2017 at 8:31 AM, John Fastabend <john.fastabend@gmail.com> wrote:
>>> On 08/07/2017 10:28 AM, Tom Herbert wrote:
>>>> Generalize the ULP infrastructure that was recently introduced to
>>>> support kTLS. This adds a SO_ULP socket option and creates new fields in
>>>> sock structure for ULP ops and ULP data. Also, the interface allows
>>>> additional per ULP parameters to be set so that a ULP can be pushed
>>>> and operations started in one shot.
>>>>
>>>> In this patch set:
>>>> - Minor dependency fix in inet_common.h
>>>> - Implement ULP infrastructure as a socket mechanism
>>>> - Fixes TCP and TLS to use the new method (maintaining backwards
>>>> API compatibility)
>>>> - Adds a ulp.txt document
>>>>
>>>> Tested: Ran simple ULP. Dave Watson verified kTLS works.
>>>>
>>>> -v2: Fix compilation errors when CONFIG_ULP_SOCK not set.
>>>> -v3: Fix one more build issue, check that sk_protocol is IPPROTO_TCP
>>>> in tsl_init. Also, fix a couple of minor issues related to
>>>> introducing locked versions of sendmsg, send page. Thanks to
>>>> Dave Watson, John Fastabend, and Mat Martineau for testing and
>>>> providing fixes.
>>>>
>>>
>>>
>>> Hi Tom, Dave,
>>>
>>> I'm concerned about the performance impact of walking a list and
>>> doing string compares on every socket we create with kTLS. Dave
>>> do you have any request/response tests for kTLS that would put pressure
>>> on the create/destroy time of this infrastructure? We should do some
>>> tests with dummy entries in the ULP list to understand the impact of
>>> this list walk.
>>>
>>> I like the underlying TCP generalized hooks, but do we really expect a
>>> lot of these hooks to exist? If we only have two on the roadmap
>>> (kTLS and socktap) it seems a bit overkill. Further, if we really expect
>>> many ULP objects then the list walk and compare will become more expensive
>>> perhaps becoming noticeable in request per second metrics.
>>>
>>> Why not just create another socktap socketopt? That will be better from
>>> complexity and likely performance sides.
>>>
>> IMO, given that there is at most two even proposed at this point I
>> don't there's much point addressing performance. When ULP feature
>> catches on and we start see a whole bunch of them then it's
>> straightforward to use a hash table or some more efficient mechanism.
>>
>
> OTOH these optimizations are usually easiest to do at the beginning. And
> building an enum of ULP types would allow removing string comparisons and
> to do simpler unsigned comparisons. I wont complain too much here though
> because this series didn't introduce the lists.
>
Hi John,
It's a tradeoff. The nice thing about using strings is that we don't
need maintain a universal enum.
A related problem is how to combine different ULPs on the same socket.
For instance, I might want to do filtering on the application layer
messages being sent over TLS (stap+kTLS ULPs). So far I don't see an
obvious way to do that. The buffering requirement for crypto seems to
convolute this some.
Tom
^ permalink raw reply
* Re: [PATCH v3 net-next 0/5] ulp: Generalize ULP infrastructure
From: Edward Cree @ 2017-08-08 20:23 UTC (permalink / raw)
To: Tom Herbert, John Fastabend
Cc: Tom Herbert, Linux Kernel Network Developers, Rohit Seth,
Dave Watson
In-Reply-To: <CAPDqMer5fh9xKTcA5fuycoOcHaOcFVRnnfEBJYj_Jv1S=R0-xQ@mail.gmail.com>
On 08/08/17 20:50, Tom Herbert wrote:
> It's a tradeoff. The nice thing about using strings is that we don't
> need maintain a universal enum.
Hmm, that makes it sound as though you're intending for random out-of-tree
modules to add these things; since if they're in-tree it's easy for them
to get enum values assigned when they're added. Do we really want to
encourage sticking random module code into the network stack like this?
In any case, if you go with the enum approach and later it _does_ prove
necessary to have more flexibility, you can have enum values dynamically
assigned (like genetlink manages to do); and programs using the existing
fixed IDs will continue to work. It's much harder to go the other way...
-Ed
^ permalink raw reply
* [net 1/1] tipc: remove premature ESTABLISH FSM event at link synchronization
From: Jon Maloy @ 2017-08-08 20:23 UTC (permalink / raw)
To: davem, netdev; +Cc: parthasarathy.bhuvaragan, ying.xue, tipc-discussion
When a link between two nodes come up, both endpoints will initially
send out a STATE message to the peer, to increase the probability that
the peer endpoint also is up when the first traffic message arrives.
Thereafter, if the establishing link is the second link between two
nodes, this first "traffic" message is a TUNNEL_PROTOCOL/SYNCH message,
helping the peer to perform initial synchronization between the two
links.
However, the initial STATE message may be lost, in which case the SYNCH
message will be the first one arriving at the peer. This should also
work, as the SYNCH message itself will be used to take up the link
endpoint before initializing synchronization.
Unfortunately the code for this case is broken. Currently, the link is
brought up through a tipc_link_fsm_evt(ESTABLISHED) when a SYNCH
arrives, whereupon __tipc_node_link_up() is called to distribute the
link slots and take the link into traffic. But, __tipc_node_link_up() is
itself starting with a test for whether the link is up, and if true,
returns without action. Clearly, the tipc_link_fsm_evt(ESTABLISHED) call
is unnecessary, since tipc_node_link_up() is itself issuing such an
event, but also harmful, since it inhibits tipc_node_link_up() to
perform the test of its tasks, and the link endpoint in question hence
is never taken into traffic.
This problem has been exposed when we set up dual links between pre-
and post-4.4 kernels, because the former ones don't send out the
initial STATE message described above.
We fix this by removing the unnecessary event call.
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/node.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/tipc/node.c b/net/tipc/node.c
index aeef801..9b4dcb6 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -1455,10 +1455,8 @@ static bool tipc_node_check_state(struct tipc_node *n, struct sk_buff *skb,
/* Initiate synch mode if applicable */
if ((usr == TUNNEL_PROTOCOL) && (mtyp == SYNCH_MSG) && (oseqno == 1)) {
syncpt = iseqno + exp_pkts - 1;
- if (!tipc_link_is_up(l)) {
- tipc_link_fsm_evt(l, LINK_ESTABLISH_EVT);
+ if (!tipc_link_is_up(l))
__tipc_node_link_up(n, bearer_id, xmitq);
- }
if (n->state == SELF_UP_PEER_UP) {
n->sync_point = syncpt;
tipc_link_fsm_evt(l, LINK_SYNCH_BEGIN_EVT);
--
2.1.4
^ permalink raw reply related
* [PATCH ericsson v2 1/1] tipc: remove premature ESTABLISH FSM event at link synchronization
From: Jon Maloy @ 2017-08-08 20:18 UTC (permalink / raw)
To: davem, netdev; +Cc: parthasarathy.bhuvaragan, ying.xue, tipc-discussion
When a link between two nodes come up, both endpoints will initially
send out a STATE message to the peer, to increase the probability that
the peer endpoint also is up when the first traffic message arrives.
Thereafter, if the establishing link is the second link between two
nodes, this first "traffic" message is a TUNNEL_PROTOCOL/SYNCH message,
helping the peer to perform initial synchronization between the two
links.
However, the initial STATE message may be lost, in which case the SYNCH
message will be the first one arriving at the peer. This should also
work, as the SYNCH message itself will be used to take up the link
endpoint before initializing synchronization.
Unfortunately the code for this case is broken. Currently, the link is
brought up through a tipc_link_fsm_evt(ESTABLISHED) when a SYNCH
arrives, whereupon __tipc_node_link_up() is called to distribute the
link slots and take the link into traffic. But, __tipc_node_link_up() is
itself starting with a test for whether the link is up, and if true,
returns without action. Clearly, the tipc_link_fsm_evt(ESTABLISHED) call
is unnecessary, since tipc_node_link_up() is itself issuing such an
event, but also harmful, since it inhibits tipc_node_link_up() to
perform the test of its tasks, and the link endpoint in question hence
is never taken into traffic.
This problem has been exposed when we set up dual links between pre-
and post-4.4 kernels, because the former ones don't send out the
initial STATE message described above.
We fix this by removing the unnecessary event call.
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/node.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/tipc/node.c b/net/tipc/node.c
index aeef801..9b4dcb6 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -1455,10 +1455,8 @@ static bool tipc_node_check_state(struct tipc_node *n, struct sk_buff *skb,
/* Initiate synch mode if applicable */
if ((usr == TUNNEL_PROTOCOL) && (mtyp == SYNCH_MSG) && (oseqno == 1)) {
syncpt = iseqno + exp_pkts - 1;
- if (!tipc_link_is_up(l)) {
- tipc_link_fsm_evt(l, LINK_ESTABLISH_EVT);
+ if (!tipc_link_is_up(l))
__tipc_node_link_up(n, bearer_id, xmitq);
- }
if (n->state == SELF_UP_PEER_UP) {
n->sync_point = syncpt;
tipc_link_fsm_evt(l, LINK_SYNCH_BEGIN_EVT);
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v3 net-next 0/5] ulp: Generalize ULP infrastructure
From: Tom Herbert @ 2017-08-08 21:08 UTC (permalink / raw)
To: Edward Cree
Cc: John Fastabend, Tom Herbert, Linux Kernel Network Developers,
Rohit Seth, Dave Watson
In-Reply-To: <6bee85e2-5fb3-135a-ad7c-ae4a350f7148@solarflare.com>
On Tue, Aug 8, 2017 at 1:23 PM, Edward Cree <ecree@solarflare.com> wrote:
> On 08/08/17 20:50, Tom Herbert wrote:
>> It's a tradeoff. The nice thing about using strings is that we don't
>> need maintain a universal enum.
> Hmm, that makes it sound as though you're intending for random out-of-tree
> modules to add these things; since if they're in-tree it's easy for them
> to get enum values assigned when they're added. Do we really want to
> encourage sticking random module code into the network stack like this?
>
> In any case, if you go with the enum approach and later it _does_ prove
> necessary to have more flexibility, you can have enum values dynamically
> assigned (like genetlink manages to do); and programs using the existing
> fixed IDs will continue to work. It's much harder to go the other way...
>
There is history and precedence. The string mechanism for ulp_ops a
direct port of the original ULP infrastructure done for kTLS. That
code based the mechanism on TCP congestion ops and that was introduced
into the kernel twelve years ago. This method doesn't seem to have
been viewed as a problem before now...
Tom
> -Ed
^ permalink raw reply
* Re: [RFC PATCH 2/2] bpf: Initialise mod[] in bpf_trace_printk
From: James Hogan @ 2017-08-08 21:20 UTC (permalink / raw)
To: David Miller, daniel; +Cc: ast, linux-kernel, rostedt, mingo, netdev
In-Reply-To: <20170808.094857.245786887664041622.davem@davemloft.net>
On 8 August 2017 17:48:57 BST, David Miller <davem@davemloft.net> wrote:
>From: Daniel Borkmann <daniel@iogearbox.net>
>Date: Tue, 08 Aug 2017 10:46:52 +0200
>
>> On 08/08/2017 12:25 AM, James Hogan wrote:
>>> In bpf_trace_printk(), the elements in mod[] are left uninitialised,
>>> but
>>> they are then incremented to track the width of the formats. Zero
>>> initialise the array just in case the memory contains non-zero
>values
>>> on
>>> entry.
>>>
>>> Fixes: 9c959c863f82 ("tracing: Allow BPF programs to call
>>> bpf_trace_printk()")
>>> Signed-off-by: James Hogan <james.hogan@imgtec.com>
>>> Cc: Alexei Starovoitov <ast@kernel.org>
>>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>>> Cc: Steven Rostedt <rostedt@goodmis.org>
>>> Cc: Ingo Molnar <mingo@redhat.com>
>>> Cc: netdev@vger.kernel.org
>>> ---
>>> When I checked (on MIPS32), the elements tended to have the value
>zero
>>> anyway (does BPF zero the stack or something clever?), so this is a
>>> purely theoretical fix.
>>> ---
>>> kernel/trace/bpf_trace.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
>>> index 32dcbe1b48f2..86a52857d941 100644
>>> --- a/kernel/trace/bpf_trace.c
>>> +++ b/kernel/trace/bpf_trace.c
>>> @@ -129,7 +129,7 @@ BPF_CALL_5(bpf_trace_printk, char *, fmt, u32,
>>> fmt_size, u64, arg1,
>>> u64, arg2, u64, arg3)
>>> {
>>> bool str_seen = false;
>>> - int mod[3] = {};
>>> + int mod[3] = { 0, 0, 0 };
>>
>> I'm probably missing something, but is the behavior of gcc wrt
>> above initializers different on mips (it zeroes just fine on x86
>> at least)? If yes, we'd probably need a cocci script to also check
>> rest of the kernel given this is used in a number of places. Hm,
>> could you elaborate?
>
>This change is not necessary at all.
>
>An empty initializer must clear the whole object to zero.
>
>"theoretical" fix indeed... :-(
cool, i hadn't realised unmentioned elements in an initialiser are always zeroed, even when non-global/static, so had interpreted the whole array as uninitialised. learn something new every day :-) sorry for the noise.
cheers
James
^ permalink raw reply
* [PATCH] net: systemport: Fix software statistics for SYSTEMPORT Lite
From: Florian Fainelli @ 2017-08-08 21:39 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
With SYSTEMPORT Lite we have holes in our statistics layout that make us
skip over the hardware MIB counters, bcm_sysport_get_stats() was not
taking that into account, resulting in reporting 0 for all SW-maintained
statistics, fix this by skipping accordingly.
Fixes: 44a4524c54af ("net: systemport: Add support for SYSTEMPORT Lite")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 5333601f855f..abf175372719 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -449,6 +449,10 @@ static void bcm_sysport_get_stats(struct net_device *dev,
p = (char *)&dev->stats;
else
p = (char *)priv;
+
+ if (priv->is_lite && !bcm_sysport_lite_stat_valid(s->type))
+ continue;
+
p += s->stat_offset;
data[j] = *(unsigned long *)p;
j++;
@@ -608,8 +612,8 @@ static struct sk_buff *bcm_sysport_rx_refill(struct bcm_sysport_priv *priv,
/* Allocate a new SKB for a new packet */
skb = netdev_alloc_skb(priv->netdev, RX_BUF_LENGTH);
+ priv->mib.alloc_rx_buff_failed++;
if (!skb) {
- priv->mib.alloc_rx_buff_failed++;
netif_err(priv, rx_err, ndev, "SKB alloc failed\n");
return NULL;
}
--
2.9.3
^ permalink raw reply related
* Re: [PATCH] net: systemport: Fix software statistics for SYSTEMPORT Lite
From: Florian Fainelli @ 2017-08-08 21:44 UTC (permalink / raw)
To: netdev; +Cc: davem
In-Reply-To: <20170808213925.26126-1-f.fainelli@gmail.com>
On 08/08/2017 02:39 PM, Florian Fainelli wrote:
> With SYSTEMPORT Lite we have holes in our statistics layout that make us
> skip over the hardware MIB counters, bcm_sysport_get_stats() was not
> taking that into account, resulting in reporting 0 for all SW-maintained
> statistics, fix this by skipping accordingly.
>
> Fixes: 44a4524c54af ("net: systemport: Add support for SYSTEMPORT Lite")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
David, please ignore this version, I accidentally sent it with the
debugging left.
--
Florian
^ permalink raw reply
* [PATCH net v2] net: systemport: Fix software statistics for SYSTEMPORT Lite
From: Florian Fainelli @ 2017-08-08 21:45 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
With SYSTEMPORT Lite we have holes in our statistics layout that make us
skip over the hardware MIB counters, bcm_sysport_get_stats() was not
taking that into account resulting in reporting 0 for all SW-maintained
statistics, fix this by skipping accordingly.
Fixes: 44a4524c54af ("net: systemport: Add support for SYSTEMPORT Lite")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
- no debugging
drivers/net/ethernet/broadcom/bcmsysport.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 5333601f855f..dc3052751bc1 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -449,6 +449,10 @@ static void bcm_sysport_get_stats(struct net_device *dev,
p = (char *)&dev->stats;
else
p = (char *)priv;
+
+ if (priv->is_lite && !bcm_sysport_lite_stat_valid(s->type))
+ continue;
+
p += s->stat_offset;
data[j] = *(unsigned long *)p;
j++;
--
2.9.3
^ permalink raw reply related
* [PATCH net-next] net: ipv6: lower ndisc notifier priority below addrconf
From: David Ahern @ 2017-08-08 21:51 UTC (permalink / raw)
To: netdev; +Cc: David Ahern
ndisc_notify is used to send unsolicited neighbor advertisements
(e.g., on a link up). Currently, the ndisc notifier is run before the
addrconf notifer which means NA's are not sent for link-local addresses
which are added by the addrconf notifier.
Fix by lowering the priority of the ndisc notifier. Setting the priority
to ADDRCONF_NOTIFY_PRIORITY - 5 means it runs after addrconf and before
the route notifier which is ADDRCONF_NOTIFY_PRIORITY - 10.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv6/ndisc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 0327c1f2e6fc..5e338eb89509 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1779,6 +1779,7 @@ static int ndisc_netdev_event(struct notifier_block *this, unsigned long event,
static struct notifier_block ndisc_netdev_notifier = {
.notifier_call = ndisc_netdev_event,
+ .priority = ADDRCONF_NOTIFY_PRIORITY - 5,
};
#ifdef CONFIG_SYSCTL
--
2.9.3
^ permalink raw reply related
* Re: [RFC PATCH 2/2] bpf: Initialise mod[] in bpf_trace_printk
From: David Miller @ 2017-08-08 21:54 UTC (permalink / raw)
To: james.hogan; +Cc: daniel, ast, linux-kernel, rostedt, mingo, netdev
In-Reply-To: <650EB3B0-1101-4624-905A-E68D2EC5920F@imgtec.com>
From: James Hogan <james.hogan@imgtec.com>
Date: Tue, 08 Aug 2017 22:20:05 +0100
> cool, i hadn't realised unmentioned elements in an initialiser are
> always zeroed, even when non-global/static, so had interpreted the
> whole array as uninitialised. learn something new every day :-)
> sorry for the noise.
You didn't have to know in the first place, you could have simply
compiled the code into assembler by running:
make kernel/trace/bpf_trace.s
and seen for yourself before putting all of this time and effort into
this patch and discussion.
If you don't know what the compiler does, simply look!
^ permalink raw reply
* Re: [PATCH] net: systemport: Fix software statistics for SYSTEMPORT Lite
From: David Miller @ 2017-08-08 21:54 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev
In-Reply-To: <f7f79e92-952d-59c6-a0f5-0d9f961a2d8f@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 8 Aug 2017 14:44:40 -0700
> On 08/08/2017 02:39 PM, Florian Fainelli wrote:
>> With SYSTEMPORT Lite we have holes in our statistics layout that make us
>> skip over the hardware MIB counters, bcm_sysport_get_stats() was not
>> taking that into account, resulting in reporting 0 for all SW-maintained
>> statistics, fix this by skipping accordingly.
>>
>> Fixes: 44a4524c54af ("net: systemport: Add support for SYSTEMPORT Lite")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>
> David, please ignore this version, I accidentally sent it with the
> debugging left.
Ok.
^ permalink raw reply
* skb allocation from interrupt handler?
From: Murali Karicheri @ 2017-08-08 22:17 UTC (permalink / raw)
To: open list:TI NETCP ETHERNET DRIVER
Is there an skb_alloc function that can be used from interrupt handler? Looks like netdev_alloc_skb()
can't be used since I see following trace with kernel hack debug options enabled.
[ 652.481713] [<c021007c>] (unwind_backtrace) from [<c020bdcc>] (show_stack+0x10/0x14)
[ 652.481725] [<c020bdcc>] (show_stack) from [<c0517780>] (dump_stack+0x98/0xc4)
[ 652.481736] [<c0517780>] (dump_stack) from [<c0256a70>] (___might_sleep+0x1b8/0x2a4)
[ 652.481746] [<c0256a70>] (___might_sleep) from [<c0939e80>] (rt_spin_lock+0x24/0x5c)
[ 652.481755] [<c0939e80>] (rt_spin_lock) from [<c07d827c>] (__netdev_alloc_skb+0xd0/0x254)
[ 652.481774] [<c07d827c>] (__netdev_alloc_skb) from [<bf23a544>] (emac_rx_hardirq+0x374/0x554 [prueth])
[ 652.481793] [<bf23a544>] (emac_rx_hardirq [prueth]) from [<c02925dc>] (__handle_irq_event_percpu+0x9c/0x128)
This is running under RT kernel off 4.9.y
--
Murali Karicheri
Linux Kernel, Keystone
^ permalink raw reply
* Re: skb allocation from interrupt handler?
From: Matteo Croce @ 2017-08-08 22:29 UTC (permalink / raw)
To: Murali Karicheri, open list:TI NETCP ETHERNET DRIVER
In-Reply-To: <598A3890.80705@ti.com>
Il giorno mar, 08/08/2017 alle 18.17 -0400, Murali Karicheri ha
scritto:
> Is there an skb_alloc function that can be used from interrupt
> handler? Looks like netdev_alloc_skb()
> can't be used since I see following trace with kernel hack debug
> options enabled.
>
> [ 652.481713] [<c021007c>] (unwind_backtrace) from [<c020bdcc>]
> (show_stack+0x10/0x14)
> [ 652.481725] [<c020bdcc>] (show_stack) from [<c0517780>]
> (dump_stack+0x98/0xc4)
> [ 652.481736] [<c0517780>] (dump_stack) from [<c0256a70>]
> (___might_sleep+0x1b8/0x2a4)
> [ 652.481746] [<c0256a70>] (___might_sleep) from [<c0939e80>]
> (rt_spin_lock+0x24/0x5c)
> [ 652.481755] [<c0939e80>] (rt_spin_lock) from [<c07d827c>]
> (__netdev_alloc_skb+0xd0/0x254)
> [ 652.481774] [<c07d827c>] (__netdev_alloc_skb) from [<bf23a544>]
> (emac_rx_hardirq+0x374/0x554 [prueth])
> [ 652.481793] [<bf23a544>] (emac_rx_hardirq [prueth]) from
> [<c02925dc>] (__handle_irq_event_percpu+0x9c/0x128)
>
> This is running under RT kernel off 4.9.y
>
netdev_alloc_skb() passes GFP_ATOMIC to alloc_skb() so it should work
in an interrupt handler too.
--
Matteo Croce
per aspera ad upstream
^ permalink raw reply
* Re: [PATCH 0/6] In-kernel QMI handling
From: Bjorn Andersson @ 2017-08-08 22:42 UTC (permalink / raw)
To: Bj?rn Mork
Cc: David S. Miller, Andy Gross, David Brown, linux-arm-msm,
linux-soc, netdev, linux-kernel
In-Reply-To: <8737921fw2.fsf@miraculix.mork.no>
On Tue 08 Aug 04:02 PDT 2017, Bj?rn Mork wrote:
> Bjorn Andersson <bjorn.andersson@linaro.org> writes:
>
> > This series starts by moving the common definitions of the QMUX protocol to the
> > uapi header, as they are shared with clients - both in kernel and userspace.
> >
> > This series then introduces in-kernel helper functions for aiding the handling
> > of QMI encoded messages in the kernel. QMI encoding is a wire-format used in
> > exchanging messages between the majority of QRTR clients and services.
>
> Interesting! I tried to add some QMI handling in the kernel a few years
> ago, but was thankfully voted down. See
> https://www.spinics.net/lists/netdev/msg183101.html and the following
> discussion. I am convinced that was the right decision, for the client
> side at least. The protocol is just too extensive and ever-growing to be
> implemented in the kernel. We would be catching up forever.
>
> Note that I had very limited knowledge of the protocol at the time I
> wrote that driver. Still have, in fact :-)
>
Thanks for the pointer, I definitely think there's more work to be done
here to figure out the proper way to interact with these devices.
But I think that Dan's reply shows a huge source of confusion here; the
acronym "QMI" covers a large amount of different things - and means
different things for different people.
In the modem world QMI seems to mean a defined set of logical endpoints
that accepts TLV-encoded messages to do modem-related things. But the
TLV-encoding is used for non-modem related services and the only common
denominator of everything called QMI is the TLV-encoding.
Due to my limited exposure to the USB attached "QMI thingies" I haven't
previously looked into the exact differences. The proposed patches aimed
to support implementing a few non-modem-related clients using
QMI-encoded messages over ipcrouter.
Looking at your patch above, and oPhono, seems to highlight a few
important differences that will take some thinking to overcome.
= Transport
The transport header in the USB case is your struct qmux, which contains
the type of message (in "flags") and the transaction id. The "service"
in the QMUX header matches the service id being communicated with. But
in order to communicate with a service it seems like one requests a
client-id from the control service.
In the smartphone world (with shared memory communication) the transport
is ipcrouter - with a header very similar to UDP - and there's no
information about the payload, it provides only the means of delivering
messages from one address/port to another address/port. A typical
smartphone has 3-4 nodes (modem, sensors, audio etc) and ports are
dynamically allocated. The control messages in the QMUX protocol (not
the same QMUX protocol as in the USB case!) are used for clients to find
the mapping from service id to a port on the given address. The source
port is dynamically allocated in this case.
= QMI-encoded messages
The list of TLV-entries have a "QMI header" prepended in both cases, but
in the QMUX case the header consists only of "msgid" and length.
In the ipcrouter case the transport doesn't carry any information
regarding the payload, so the header prepended the TLV entries includes
"type", transaction id, "msg_id" and length.
It looks as if once past the differences in the transport and QMI
message header the messages (TLV-encoded data) are the same. But I'm not
yet sure about how we can hide the transport differences.
Regards,
Bjorn
^ permalink raw reply
* Re: skb allocation from interrupt handler?
From: David Miller @ 2017-08-08 23:00 UTC (permalink / raw)
To: m-karicheri2; +Cc: netdev
In-Reply-To: <598A3890.80705@ti.com>
From: Murali Karicheri <m-karicheri2@ti.com>
Date: Tue, 8 Aug 2017 18:17:52 -0400
> Is there an skb_alloc function that can be used from interrupt handler? Looks like netdev_alloc_skb()
> can't be used since I see following trace with kernel hack debug options enabled.
>
> [ 652.481713] [<c021007c>] (unwind_backtrace) from [<c020bdcc>] (show_stack+0x10/0x14)
> [ 652.481725] [<c020bdcc>] (show_stack) from [<c0517780>] (dump_stack+0x98/0xc4)
> [ 652.481736] [<c0517780>] (dump_stack) from [<c0256a70>] (___might_sleep+0x1b8/0x2a4)
> [ 652.481746] [<c0256a70>] (___might_sleep) from [<c0939e80>] (rt_spin_lock+0x24/0x5c)
> [ 652.481755] [<c0939e80>] (rt_spin_lock) from [<c07d827c>] (__netdev_alloc_skb+0xd0/0x254)
> [ 652.481774] [<c07d827c>] (__netdev_alloc_skb) from [<bf23a544>] (emac_rx_hardirq+0x374/0x554 [prueth])
> [ 652.481793] [<bf23a544>] (emac_rx_hardirq [prueth]) from [<c02925dc>] (__handle_irq_event_percpu+0x9c/0x128)
>
> This is running under RT kernel off 4.9.y
Your receive handler should be running from a NAPI poll, which is in
software interrupt. You should not be doing packet processing in
hardware interrupt context as hardware interrupts should be as short
as possible, and with NAPI polling packet input processing can be
properly distributed amongst several devices, and if the system is
overloaded such processing can be deferred to a kernel thread.
NAPI polling has a large number of other advantages as well, more
streamlined GRO support, automatic support for busypolling... the
list goes on and on and on.
I could show you how to do an SKB allocation in a hardware interrupt,
but instead I'd rather teach you how to fish properly, and encourage
you to convert your driver to NAPI polling instead.
Thanks.
^ permalink raw reply
* Re: [PATCH v06 15/36] uapi linux/socket.h: include sys/socket.h in user space
From: Dmitry V. Levin @ 2017-08-08 23:00 UTC (permalink / raw)
To: Mikko Rapeli
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170806164428.2273-16-mikko.rapeli-X3B1VOXEql0@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1420 bytes --]
On Sun, Aug 06, 2017 at 06:44:06PM +0200, Mikko Rapeli wrote:
> This libc header has sockaddr definition in user space.
>
> Fixes user space compilation errors like these from kernel headers including
> only linux/socket.h:
>
> error: field ‘ifru_addr’ has incomplete type
> struct sockaddr ifru_addr;
> error: field ‘_sockaddr’ has incomplete type
> struct sockaddr _sockaddr;
> error: invalid application of ‘sizeof’ to incomplete type ‘struct sockaddr’
>
> With this following uapi headers now compile in user space:
>
> rdma/rdma_user_rxe.h
> linux/vm_sockets.h
> linux/ncp_fs.h
> linux/nfc.h
> linux/phonet.h
>
> Signed-off-by: Mikko Rapeli <mikko.rapeli-X3B1VOXEql0@public.gmane.org>
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Dmitry V. Levin <ldv-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
> ---
> include/uapi/linux/socket.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/uapi/linux/socket.h b/include/uapi/linux/socket.h
> index 76ab0c68561e..8a81197cc08b 100644
> --- a/include/uapi/linux/socket.h
> +++ b/include/uapi/linux/socket.h
> @@ -1,6 +1,10 @@
> #ifndef _UAPI_LINUX_SOCKET_H
> #define _UAPI_LINUX_SOCKET_H
>
> +#ifndef __KERNEL__
> +#include <sys/socket.h>
> +#endif
This is scary because of infamous libc vs uapi interoperability issues.
Couldn't we fix affected headers instead?
--
ldv
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v2] wan: dscc4: add checks for dma mapping errors
From: Francois Romieu @ 2017-08-08 23:21 UTC (permalink / raw)
To: Alexey Khoroshilov; +Cc: David S . Miller, netdev, linux-kernel, ldv-project
In-Reply-To: <1502220499-18351-1-git-send-email-khoroshilov@ispras.ru>
Alexey Khoroshilov <khoroshilov@ispras.ru> :
[...]
> diff --git a/drivers/net/wan/dscc4.c b/drivers/net/wan/dscc4.c
> index 799830f..6a9ffac 100644
> --- a/drivers/net/wan/dscc4.c
> +++ b/drivers/net/wan/dscc4.c
> @@ -518,23 +518,31 @@ static void dscc4_release_ring(struct dscc4_dev_priv *dpriv)
> static inline int try_get_rx_skb(struct dscc4_dev_priv *dpriv,
> struct net_device *dev)
> {
> + struct pci_dev *pdev = dpriv->pci_priv->pdev;
> unsigned int dirty = dpriv->rx_dirty%RX_RING_SIZE;
> struct RxFD *rx_fd = dpriv->rx_fd + dirty;
> const int len = RX_MAX(HDLC_MAX_MRU);
For the edification of the masses, you may follow a strict inverted
xmas tree style (aka longer lines first as long as it does not bug).
[...]
> @@ -1147,14 +1155,22 @@ static netdev_tx_t dscc4_start_xmit(struct sk_buff *skb,
> struct dscc4_dev_priv *dpriv = dscc4_priv(dev);
> struct dscc4_pci_priv *ppriv = dpriv->pci_priv;
> struct TxFD *tx_fd;
> + dma_addr_t addr;
> int next;
>
> + addr = pci_map_single(ppriv->pdev, skb->data, skb->len,
> + PCI_DMA_TODEVICE);
Use a local struct pci_dev *pdev and it fits on a single line.
At some point it will probably be converted to plain dma api and use a 'd' dev.
[...]
> @@ -1887,16 +1903,22 @@ static struct sk_buff *dscc4_init_dummy_skb(struct dscc4_dev_priv *dpriv)
>
> skb = dev_alloc_skb(DUMMY_SKB_SIZE);
> if (skb) {
> + struct pci_dev *pdev = dpriv->pci_priv->pdev;
> int last = dpriv->tx_dirty%TX_RING_SIZE;
> struct TxFD *tx_fd = dpriv->tx_fd + last;
> + dma_addr_t addr;
>
> skb->len = DUMMY_SKB_SIZE;
> skb_copy_to_linear_data(skb, version,
> strlen(version) % DUMMY_SKB_SIZE);
> tx_fd->state = FrameEnd | TO_STATE_TX(DUMMY_SKB_SIZE);
> - tx_fd->data = cpu_to_le32(pci_map_single(dpriv->pci_priv->pdev,
> - skb->data, DUMMY_SKB_SIZE,
> - PCI_DMA_TODEVICE));
> + addr = pci_map_single(pdev, skb->data, DUMMY_SKB_SIZE,
> + PCI_DMA_TODEVICE);
> + if (pci_dma_mapping_error(pdev, addr)) {
> + dev_kfree_skb_any(skb);
> + return NULL;
> + }
> + tx_fd->data = cpu_to_le32(addr);
> dpriv->tx_skbuff[last] = skb;
> }
> return skb;
It isn't technically wrong but please don't update tx_fd before the mapping
succeeds. It will look marginally better.
Thanks.
--
Ueimor
^ permalink raw reply
* Re: [PATCH v9 1/4] PCI: Add new PCIe Fabric End Node flag, PCI_DEV_FLAGS_NO_RELAXED_ORDERING
From: Bjorn Helgaas @ 2017-08-08 23:22 UTC (permalink / raw)
To: Ding Tianhong
Cc: leedom, ashok.raj, bhelgaas, werner, ganeshgr, asit.k.mallick,
patrick.j.cramer, Suravee.Suthikulpanit, Bob.Shaw, l.stach, amira,
gabriele.paoloni, David.Laight, jeffrey.t.kirsher,
catalin.marinas, will.deacon, mark.rutland, robin.murphy, davem,
alexander.duyck, linux-arm-kernel, netdev, linux-pci,
linux-kernel, linuxarm
In-Reply-To: <1501917313-9812-2-git-send-email-dingtianhong@huawei.com>
On Sat, Aug 05, 2017 at 03:15:10PM +0800, Ding Tianhong wrote:
> From: Casey Leedom <leedom@chelsio.com>
>
> The patch adds a new flag PCI_DEV_FLAGS_NO_RELAXED_ORDERING to indicate that
> Relaxed Ordering (RO) attribute should not be used for Transaction Layer
> Packets (TLP) targetted towards these affected root complexes. Current list
> of affected parts include some Intel Xeon processors root complex which suffers from
> flow control credits that result in performance issues. On these affected
> parts RO can still be used for peer-2-peer traffic. AMD A1100 ARM ("SEATTLE")
> Root complexes don't obey PCIe 3.0 ordering rules, hence could lead to
> data-corruption.
This needs to include a link to the Intel spec
(https://software.intel.com/sites/default/files/managed/9e/bc/64-ia-32-architectures-optimization-manual.pdf,
sec 3.9.1).
It should also include a pointer to the AMD erratum, if available, or
at least some reference to how we know it doesn't obey the rules.
Ashok, thanks for chiming in. Now that you have, I have a few more
questions for you:
- Is the above doc the one you mentioned as being now public?
- Is this considered a hardware erratum?
- If so, is there a pointer to that as well?
- If this is not considered an erratum, can you provide any guidance
about how an OS should determine when it should use RO?
Relying on a list of device IDs in an optimization manual is OK for an
erratum, but if it's *not* an erratum, it seems like a hole in the
specs because as far as I know there's no generic way for the OS to
discover whether to use RO.
Bjorn
^ permalink raw reply
* Re: [PATCH v06 35/36] uapi linux/tls.h: don't include <net/tcp.h> in user space
From: Dmitry V. Levin @ 2017-08-08 23:25 UTC (permalink / raw)
To: Mikko Rapeli
Cc: linux-kernel, linux-api, Dave Watson, Ilya Lesokhin,
Aviad Yehezkel, netdev
In-Reply-To: <20170806164428.2273-36-mikko.rapeli@iki.fi>
[-- Attachment #1: Type: text/plain, Size: 939 bytes --]
On Sun, Aug 06, 2017 at 06:44:26PM +0200, Mikko Rapeli wrote:
> It is not needed and not part of uapi headers, but causes
> user space compilation error:
>
> fatal error: net/tcp.h: No such file or directory
> #include <net/tcp.h>
> ^
>
> Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
> Cc: Dave Watson <davejwatson@fb.com>
> Cc: Ilya Lesokhin <ilyal@mellanox.com>
> Cc: Aviad Yehezkel <aviadye@mellanox.com>
> ---
> include/uapi/linux/tls.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/uapi/linux/tls.h b/include/uapi/linux/tls.h
> index cc1d21db35d8..d87c698623f2 100644
> --- a/include/uapi/linux/tls.h
> +++ b/include/uapi/linux/tls.h
> @@ -37,7 +37,9 @@
> #include <asm/byteorder.h>
> #include <linux/socket.h>
> #include <linux/tcp.h>
> +#ifdef __KERNEL__
> #include <net/tcp.h>
> +#endif
Let's move it to include/net/tls.h instead.
--
ldv
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH] netfilter: nf_nat_h323: fix logical-not-parentheses warning
From: Nick Desaulniers @ 2017-08-08 23:28 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Matthias Kaehlcke, Lorenzo Colitti, Pablo Neira Ayuso,
Jozsef Kadlecsik, Florian Westphal, David S. Miller,
Alexey Kuznetsov, Hideaki YOSHIFUJI, netfilter-devel, coreteam,
netdev, linux-kernel
In-Reply-To: <20170731183949.73763-1-ndesaulniers@google.com>
bumping for review
On Mon, Jul 31, 2017 at 11:39 AM, Nick Desaulniers
<ndesaulniers@google.com> wrote:
> Clang produces the following warning:
>
> net/ipv4/netfilter/nf_nat_h323.c:553:6: error:
> logical not is only applied to the left hand side of this comparison
> [-Werror,-Wlogical-not-parentheses]
> if (!set_h225_addr(skb, protoff, data, dataoff, taddr,
> ^
> add parentheses after the '!' to evaluate the comparison first
> add parentheses around left hand side expression to silence this warning
>
> There's not necessarily a bug here, but it's cleaner to use the form:
>
> if (x != 0)
>
> rather than:
>
> if (!x == 0)
>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Also, it's even cleaner to use the form:
>
> if (x)
>
> but then if the return codes change from treating 0 as success (unlikely),
> then all call sites must be updated.
>
> I'm happy to send v2 that changes to that form, and updates the other call
> sites to be:
>
> if (set_h225_addr())
> handle_failures()
> else
> handle_success()
>
> net/ipv4/netfilter/nf_nat_h323.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/netfilter/nf_nat_h323.c b/net/ipv4/netfilter/nf_nat_h323.c
> index 574f7ebba0b6..d8fb251fa6e3 100644
> --- a/net/ipv4/netfilter/nf_nat_h323.c
> +++ b/net/ipv4/netfilter/nf_nat_h323.c
> @@ -550,9 +550,9 @@ static int nat_callforwarding(struct sk_buff *skb, struct nf_conn *ct,
> }
>
> /* Modify signal */
> - if (!set_h225_addr(skb, protoff, data, dataoff, taddr,
> - &ct->tuplehash[!dir].tuple.dst.u3,
> - htons(nated_port)) == 0) {
> + if (set_h225_addr(skb, protoff, data, dataoff, taddr,
> + &ct->tuplehash[!dir].tuple.dst.u3,
> + htons(nated_port)) != 0) {
> nf_ct_unexpect_related(exp);
> return -1;
> }
> --
> 2.14.0.rc0.400.g1c36432dff-goog
>
--
Thanks,
~Nick Desaulniers
^ 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