Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 2/2] net: sched: protect against stack overflow in TC act_mirred
From: John Hurley @ 2019-06-25  9:42 UTC (permalink / raw)
  To: Florian Westphal
  Cc: Eyal Birger, Linux Netdev List, David Miller, Jamal Hadi Salim,
	Simon Horman, Jakub Kicinski, oss-drivers, shmulik
In-Reply-To: <20190625091507.pwtingx6yk4ltmbo@breakpoint.cc>

On Tue, Jun 25, 2019 at 10:15 AM Florian Westphal <fw@strlen.de> wrote:
>
> John Hurley <john.hurley@netronome.com> wrote:
> > Hi Eyal,
> > The value of 4 is basically a revert to what it was on older kernels
> > when TC had a TTL value in the skb:
> > https://elixir.bootlin.com/linux/v3.19.8/source/include/uapi/linux/pkt_cls.h#L97
>
> IIRC this TTL value was not used ever.

It was used to carry out this looping check on ingress redirects:
https://elixir.bootlin.com/linux/v3.19.8/source/net/core/dev.c#L3468
It appears this was removed/unused after changes in 4.2


>
> > I also found with my testing that a value greater than 4 was sailing
> > close to the edge.
> > With a larger value (on my system anyway), I could still trigger a
> > stack overflow here.
> > I'm not sure on the history of why a value of 4 was selected here but
> > it seems to fall into line with my findings.
> > Is there a hard requirement for >4 recursive calls here?
>
> One alternative would be to (instead of dropping the skb), to
> decrement the ttl and use netif_rx() instead.

Yes, this seems like something worth investigating.
Thanks

^ permalink raw reply

* RE: [PATCH v5 2/5] net: macb: add support for sgmii MAC-PHY interface
From: Parshuram Raju Thombare @ 2019-06-25  9:38 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: andrew@lunn.ch, nicolas.ferre@microchip.com, davem@davemloft.net,
	f.fainelli@gmail.com, netdev@vger.kernel.org,
	hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
	Rafal Ciepiela, Anil Joy Varughese, Piotr Sroka
In-Reply-To: <20190625092930.ootk5nvbkqqvfbtd@shell.armlinux.org.uk>


>> >In which case, gem_phylink_validate() must clear the support mask when
>> >SGMII mode is requested to indicate that the interface mode is not
>> >supported.
>> >The same goes for _all_ other PHY link modes that the hardware does not
>> >actually support, such as PHY_INTERFACE_MODE_10GKR...
>> If interface is not supported by hardware probe returns with error, so we don't
>> net interface is not registered at all.
>That does not negate my comment above.
Sorry if I misunderstood your question, but hardware supports interfaces and based
on that link modes are supported. So if interface is not supported by hardware,
net device is not registered and there will be no phylink_validate call.
If hardware support particular interface, link modes supported by interface
are added to (not cleared from) supported mask, provided configs is not trying to limit 
data rate using GIGABIT_ENABLE* macro.

Regards,
Parshuram Thombare

^ permalink raw reply

* [PATCH iproute2] ip/iptoken: fix dump error when ipv6 disabled
From: Hangbin Liu @ 2019-06-25  9:35 UTC (permalink / raw)
  To: netdev
  Cc: Daniel Borkmann, Phil Sutter, Stephen Hemminger, David Ahern,
	Andrea Claudi, Hangbin Liu

When we disable IPv6 from the start up (ipv6.disable=1), there will be
no IPv6 route info in the dump message. If we return -1 when
ifi->ifi_family != AF_INET6, we will get error like

$ ip token list
Dump terminated

which will make user feel confused. There is no need to return -1 if the
dump message not match. Return 0 is enough.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 ip/iptoken.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/ip/iptoken.c b/ip/iptoken.c
index f1194c3e..dfd22734 100644
--- a/ip/iptoken.c
+++ b/ip/iptoken.c
@@ -59,13 +59,9 @@ static int print_token(struct nlmsghdr *n, void *arg)
 	if (len < 0)
 		return -1;
 
-	if (ifi->ifi_family != AF_INET6)
-		return -1;
-	if (ifi->ifi_index == 0)
-		return -1;
-	if (ifindex > 0 && ifi->ifi_index != ifindex)
-		return 0;
-	if (ifi->ifi_flags & (IFF_LOOPBACK | IFF_NOARP))
+	if (ifi->ifi_family != AF_INET6 || ifi->ifi_index == 0 ||
+	    (ifindex > 0 && ifi->ifi_index != ifindex) ||
+	    (ifi->ifi_flags & (IFF_LOOPBACK | IFF_NOARP)))
 		return 0;
 
 	parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len);
-- 
2.19.2


^ permalink raw reply related

* Re: Removing skb_orphan() from ip_rcv_core()
From: Daniel Borkmann @ 2019-06-25  9:35 UTC (permalink / raw)
  To: Eric Dumazet, Joe Stringer, Florian Westphal
  Cc: netdev, john fastabend, Lorenz Bauer, Jakub Sitnicki, Paolo Abeni,
	Flavio Leitner, ast
In-Reply-To: <b6baadcb-29af-82f1-bebe-56d5f45b12e6@gmail.com>

On 06/25/2019 08:37 AM, Eric Dumazet wrote:
> On 6/24/19 8:17 PM, Joe Stringer wrote:
>> On Fri, Jun 21, 2019 at 1:59 PM Florian Westphal <fw@strlen.de> wrote:
>>>
>>> Joe Stringer <joe@wand.net.nz> wrote:
>>>> As discussed during LSFMM, I've been looking at adding something like
>>>> an `skb_sk_assign()` helper to BPF so that logic similar to TPROXY can
>>>> be implemented with integration into other BPF logic, however
>>>> currently any attempts to do so are blocked by the skb_orphan() call
>>>> in ip_rcv_core() (which will effectively ignore any socket assign
>>>> decision made by the TC BPF program).
>>>>
>>>> Recently I was attempting to remove the skb_orphan() call, and I've
>>>> been trying different things but there seems to be some context I'm
>>>> missing. Here's the core of the patch:
>>>>
>>>> diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
>>>> index ed97724c5e33..16aea980318a 100644
>>>> --- a/net/ipv4/ip_input.c
>>>> +++ b/net/ipv4/ip_input.c
>>>> @@ -500,8 +500,6 @@ static struct sk_buff *ip_rcv_core(struct sk_buff
>>>> *skb, struct net *net)
>>>>        memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));
>>>>        IPCB(skb)->iif = skb->skb_iif;
>>>>
>>>> -       /* Must drop socket now because of tproxy. */
>>>> -       skb_orphan(skb);
>>>>
>>>>        return skb;
>>>>
>>>> The statement that the socket must be dropped because of tproxy
>>>> doesn't make sense to me, because the PRE_ROUTING hook is hit after
>>>> this, which will call into the tproxy logic and eventually
>>>> nf_tproxy_assign_sock() which already does the skb_orphan() itself.
>>>
>>> in comment: s/tproxy/skb_steal_sock/
>>
>> For reference, I was following the path like this:
>>
>> ip_rcv()
>> ( -> ip_rcv_core() for skb_orphan)
>> -> NF_INET_PRE_ROUTING hook
>> (... invoke iptables hooks)
>> -> iptable_mangle_hook()
>> -> ipt_do_table()
>> ... -> tproxy_tg4()
>> ... -> nf_tproxy_assign_sock()
>> -> skb_orphan()
>> (... finish iptables processing)
>> ( -> ip_rcv_finish())
>> ( ... -> ip_rcv_finish_core() for early demux / route lookup )
>> (... -> dst_input())
>> (... -> tcp_v4_rcv())
>> ( -> __inet_lookup_skb())
>> ( -> skb_steal_sock() )
>>
>>> at least thats what I concluded a few years ago when I looked into
>>> the skb_oprhan() need.
>>>
>>> IIRC some device drivers use skb->sk for backpressure, so without this
>>> non-tcp socket would be stolen by skb_steal_sock.
>>
>> Do you happen to recall which device drivers? Or have some idea of a
>> list I could try to go through? Are you referring to virtual drivers
>> like veth or something else?
>>
>>> We also recently removed skb orphan when crossing netns:
>>>
>>> commit 9c4c325252c54b34d53b3d0ffd535182b744e03d
>>> Author: Flavio Leitner <fbl@redhat.com>
>>> skbuff: preserve sock reference when scrubbing the skb.
>>>
>>> So thats another case where this orphan is needed.
>>
>> Presumably the orphan is only needed in this case if the packet
>> crosses a namespace and then is subsequently passed back into the
>> stack?
> 
> Yes, I understand we do not want the skb_orphan() when 'srubing' the skb.
> 
> But we want the skb_orphan() right before the packet is reinjected in ingress path. 
> 
>>> What could be done is adding some way to delay/defer the orphaning
>>> further, but we would need at the very least some annotation for
>>> skb_steal_sock to know when the skb->sk is really from TPROXY or
>>> if it has to orphan.
>>
>> Eric mentions in another response to this thread that skb_orphan()
>> should be called from any ndo_start_xmit() which sends traffic back
>> into the stack. With that, presumably we would be pushing the
>> orphaning earlier such that the only way that the skb->sk ref can be
>> non-NULL around this point in receive would be because it was
>> specifically set by some kind of tproxy logic?
>>
>>> Same for the safety check in the forwarding path.
>>> Netfilter modules need o be audited as well, they might make assumptions
>>> wrt. skb->sk being inet sockets (set by local stack or early demux).
>>>
>>>> However, if I drop these lines then I end up causing sockets to
>>>> release references too many times. Seems like if we don't orphan the
>>>> skb here, then later logic assumes that we have one more reference
>>>> than we actually have, and decrements the count when it shouldn't
>>>> (perhaps the skb_steal_sock() call in __inet_lookup_skb() which seems
>>>> to assume we always have a reference to the socket?)
>>>
>>> We might be calling the wrong destructor (i.e., the one set by tcp
>>> receive instead of the one set at tx time)?
>>
>> Hmm, interesting thought. Sure enough, with a bit of bpftrace
>> debugging we find it's tcp_wfree():
>>
>> $ cat ip_rcv.bt
>> #include <linux/skbuff.h>
>>
>> kprobe:ip_rcv {
>>        $sk = ((struct sk_buff *)arg0)->sk;
>>        $des = ((struct sk_buff *)arg0)->destructor;
>>        if ($sk) {
>>                if ($des) {
>>                        printf("received %s on %s with sk destructor %s
>> set\n", str(arg0), str(arg1), ksym($des));
>>                        @ip4_stacks[kstack] = count();
>>                }
>>        }
>> }
>> $ sudo bpftrace ip_rcv.bt
>> Attaching 1 probe...
>> received  on eth0 with sk destructor tcp_wfree set
>> ^C
>>
>> @ip4_stacks[
>>    ip_rcv+1
>>    __netif_receive_skb+24
>>    process_backlog+179
>>    net_rx_action+304
>>    __do_softirq+220
>>    do_softirq_own_stack+42
>>    do_softirq.part.17+70
>>    __local_bh_enable_ip+101
>>    ip_finish_output2+421
>>    __ip_finish_output+187
>>    ip_finish_output+44
>>    ip_output+109
>>    ip_local_out+59
>>    __ip_queue_xmit+368
>>    ip_queue_xmit+16
>>    __tcp_transmit_skb+1303
>>    tcp_connect+2758
>>    tcp_v4_connect+1135
>>    __inet_stream_connect+214
>>    inet_stream_connect+59
>>    __sys_connect+237
>>    __x64_sys_connect+26
>>    do_syscall_64+90
>>    entry_SYSCALL_64_after_hwframe+68
>> ]: 1
>>
>> Is there a solution here where we call the destructor if it's not
>> sock_efree()? When the socket is later stolen, it will only return the
>> reference via a call to sock_put(), so presumably at that point in the
>> stack we already assume that the skb->destructor is not one of these
>> other destructors (otherwise we wouldn't release the resources
>> correctly).
> 
> What was the driver here ? In any case, the following patch should help.
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index eeacebd7debbe6a55daedb92f00afd48051ebaf8..5075b4b267af7057f69fcb935226fce097a920e2 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3699,6 +3699,7 @@ static __always_inline int ____dev_forward_skb(struct net_device *dev,
>                 return NET_RX_DROP;
>         }
>  
> +       skb_orphan(skb);
>         skb_scrub_packet(skb, true);
>         skb->priority = 0;
>         return 0;
> 

But wasn't the whole point of 9c4c325252c5 ("skbuff: preserve sock reference when
scrubbing the skb.") to defer orphaning to as late as possible? If I'm not missing
anything, then above would reintroduce the issues that 9c4c325252c5 was trying to
solve wrt TSQ/XPS/etc when skb was sent via veth based data path to cross netns and
then forwarded to phys dev for transmission; meaning, skb->sk is lost at the point
of dev_queue_xmit() for the latter. A side-effect this would also have is that this
changes behavior again for tc egress programs sitting on phys dev (e.g. querying
sock cookie or other related features).

Thanks,
Daniel

^ permalink raw reply

* Re: [PATCH RFC net-next 4/5] dt-bindings: net: dsa: mt7530: Add mediatek,ephy-handle to isolate ext. phy
From: René van Dorst @ 2019-06-25  9:30 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: sean.wang, linux, davem, matthias.bgg, andrew, vivien.didelot,
	frank-w, netdev, linux-mediatek, linux-mips
In-Reply-To: <e6175753-eb99-63e5-767e-f82becbf8d1a@gmail.com>

Quoting Florian Fainelli <f.fainelli@gmail.com>:

Hi Florian

> On 6/24/19 7:52 AM, René van Dorst wrote:
>> On some platforum the external phy can only interface to the port 5 of the
>> switch because the RGMII TX and RX lines are swapped. But it still can be
>> useful to use the internal phy of the switch to act as a WAN port which
>> connectes to the 2nd GMAC. This gives WAN port dedicated bandwidth to
>> the SOC. This increases the LAN and WAN routing.
>>
>> By adding the optional property mediatek,ephy-handle, the external phy
>> is put in isolation mode when internal phy is connected to 2nd GMAC via
>> phy-handle property.
>
> Most platforms we have seen so far implement this logic with a mdio-mux,
> can you see if that is possible here? stmmac has plenty of examples like
> those.

May I don't understand it correctly, but all the devices are on the same MDIO
bus.
I tried to make a ASCII diagram to make it a bit more clear.

  +-------------------+                     
+----------------------------------------------+
  | SOC MT7621/3      |                    | MT7530 SWITCH              
                    |
  |   +-------------+ |(T)RGMII BUS        |  +-------------+    
INTERNAL                  |
  |   |  1st GMAC   <------------------------->  PORT 6     |   MII  
BUS  +----------+     |
  |   +-------------+ |                    |  +-------------+      
+------>  GMAC5   |     |
  |                   |                    |                      |     
   +----------+     |
  |   +-------------+ | RGMII BUS          |  +-------------+     |     
                    |
  |   |  2nd GMAC   <------------------+------>  PORT 5     +-----+     
   +----------+     |
  |   +-------------+ |                |   |  +-------------+      
+------>  PHY P0  +<--+ |
  |                   |                |   |                      |     
   +----------+   | |
  |   +-------------+ |  MDIO BUS      |   |                      |     
                  | |
  |   |  MDIO       +--------+         |   |                      |     
   +----------+   | |
  |   +-------------+ |      |         |   |                       
+------>  PHY P4  +<--+ |
  |                   |      |         |   |                            
   +----------+   | |
  +-------------------+      |         |    
+--------------------------------------------|-+
                             |         |       +-------------+          
                  |
                             |         |       |  EXTERNAL   |          
                  |
                             |         +------->  PHY AT8033 | SGMII  
BUS +----------+   |
                             |                 |              
+-----------> SFP CAGE |   |
                             +---------------->+             |          
   +----------+   |
                             |                 |             |          
                  |
                             |                 +-------------+          
                  |
                              
+----------------------------------------------------------+

I don't see why I need a MDIO mux. Devices; 2nd GMAC, external phy and port 5
share the same RGMII bus. Depending on the port 5 mux settings, port 5  
is acting
as a GMAC of PHY of port 0/4. As long as the unused devices doesn't drive the
RGMII bus we are good.

2nd GMAC RGMII control is currently set with a pinctrl "RGMII2".
Port 5 and external phy are done in mt7530_port5_setup() depending on the
device-tree.

Greats,

René

> --
> Florian




^ permalink raw reply

* Re: [PATCH v5 2/5] net: macb: add support for sgmii MAC-PHY interface
From: Russell King - ARM Linux admin @ 2019-06-25  9:29 UTC (permalink / raw)
  To: Parshuram Raju Thombare
  Cc: andrew@lunn.ch, nicolas.ferre@microchip.com, davem@davemloft.net,
	f.fainelli@gmail.com, netdev@vger.kernel.org,
	hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
	Rafal Ciepiela, Anil Joy Varughese, Piotr Sroka
In-Reply-To: <SN2PR07MB2480B53AFE512F46986A1CAFC1E30@SN2PR07MB2480.namprd07.prod.outlook.com>

On Tue, Jun 25, 2019 at 09:26:29AM +0000, Parshuram Raju Thombare wrote:
> >In which case, gem_phylink_validate() must clear the support mask when
> >SGMII mode is requested to indicate that the interface mode is not
> >supported.
> >The same goes for _all_ other PHY link modes that the hardware does not
> >actually support, such as PHY_INTERFACE_MODE_10GKR...
> If interface is not supported by hardware probe returns with error, so we don't 
> net interface is not registered at all.

That does not negate my comment above.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* RE: [PATCH v5 4/5] net: macb: add support for high speed interface
From: Parshuram Raju Thombare @ 2019-06-25  9:29 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: andrew@lunn.ch, nicolas.ferre@microchip.com, davem@davemloft.net,
	f.fainelli@gmail.com, netdev@vger.kernel.org,
	hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
	Rafal Ciepiela, Anil Joy Varughese, Piotr Sroka
In-Reply-To: <20190625090324.c6tq2neksatfwljw@shell.armlinux.org.uk>

>> >>  	switch (state->interface) {
>> >>  	case PHY_INTERFACE_MODE_NA:
>> >> +	case PHY_INTERFACE_MODE_USXGMII:
>> >> +	case PHY_INTERFACE_MODE_10GKR:
>> >> +		if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE) {
>> >> +			phylink_set(mask, 10000baseCR_Full);
>> >> +			phylink_set(mask, 10000baseER_Full);
>> >> +			phylink_set(mask, 10000baseKR_Full);
>> >> +			phylink_set(mask, 10000baseLR_Full);
>> >> +			phylink_set(mask, 10000baseLRM_Full);
>> >> +			phylink_set(mask, 10000baseSR_Full);
>> >> +			phylink_set(mask, 10000baseT_Full);
>> >> +			phylink_set(mask, 5000baseT_Full);
>> >> +			phylink_set(mask, 2500baseX_Full);
>> >> +			phylink_set(mask, 1000baseX_Full);
>> >> +		}
>> >If MACB_CAPS_GIGABIT_MODE_AVAILABLE is not set, are these interface
>> >modes supported by the hardware?  If the PHY interface mode is not
>> >supported, then the returned support mask must be cleared.[]
>> There are some configs which uses this macro to limit data rate to 100M
>> even if hardware support higher rates.
>I'm sorry, this response does not address my statement, maybe I wasn't
>clear enough.  I am asking about the *PHY* interface modes, in
>other words (e.g.) PHY_INTERFACE_MODE_USXGMII.

If interface is not supported by hardware probe returns with error, so net 
device is not registered at all.

Regards,
Parshuram Thombare

^ permalink raw reply

* RE: [PATCH v5 2/5] net: macb: add support for sgmii MAC-PHY interface
From: Parshuram Raju Thombare @ 2019-06-25  9:26 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: andrew@lunn.ch, nicolas.ferre@microchip.com, davem@davemloft.net,
	f.fainelli@gmail.com, netdev@vger.kernel.org,
	hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
	Rafal Ciepiela, Anil Joy Varughese, Piotr Sroka
In-Reply-To: <20190624134233.suowuortj5dcbxdg@shell.armlinux.org.uk>

>> +	if (change_interface) {
>> +		bp->phy_interface = state->interface;
>> +		gem_writel(bp, NCR, ~GEM_BIT(TWO_PT_FIVE_GIG) &
>> +			   gem_readl(bp, NCR));
>This could do with a comment, such as the one I gave in my example.
Sure. I will add a comment here.

>> @@ -493,6 +516,7 @@ static void gem_mac_config(struct phylink_config
>*pl_config, unsigned int mode,
>>  		reg &= ~(MACB_BIT(SPD) | MACB_BIT(FD));
>>  		if (macb_is_gem(bp))
>>  			reg &= ~GEM_BIT(GBE);
>> +
>Useless change.
Ok, I will remove this empty line.


>> +		if (phy_mode == PHY_INTERFACE_MODE_SGMII) {
>> +			if (!(bp->caps & MACB_CAPS_PCS))
>> +				interface_supported = 0;
>
>So if bp->caps does not have MACB_CAPS_PCS set, then SGMII mode is not
>supported?
Yes

>In which case, gem_phylink_validate() must clear the support mask when
>SGMII mode is requested to indicate that the interface mode is not
>supported.
>The same goes for _all_ other PHY link modes that the hardware does not
>actually support, such as PHY_INTERFACE_MODE_10GKR...
If interface is not supported by hardware probe returns with error, so we don't 
net interface is not registered at all.
I think what is missing is setting appropriate err value (-ENOTSUPP ?), right now it is returning
default err.



Regards,
Parshuram Thombare

^ permalink raw reply

* Re: [PATCH v12 5/5] can: m_can: Fix checkpatch issues on existing code
From: Faiz Abbas @ 2019-06-25  9:24 UTC (permalink / raw)
  To: Dan Murphy, wg, mkl, davem; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <20190509161109.10499-5-dmurphy@ti.com>

Hi,

On 09/05/19 9:41 PM, Dan Murphy wrote:
> Fix checkpatch issues found during the m_can framework creation.
> The code the issues were in, was in untouched code and these
> changes should be done separately as to not be confused with the
> framework changes.
> 
> Fix these 3 check issues:
> CHECK: Unnecessary parentheses around 'cdev->can.state != CAN_STATE_ERROR_WARNING'
> 	if (psr & PSR_EW &&
> 	    (cdev->can.state != CAN_STATE_ERROR_WARNING)) {
> 
> CHECK: Unnecessary parentheses around 'cdev->can.state != CAN_STATE_ERROR_PASSIVE'
> 	if ((psr & PSR_EP) &&
> 	    (cdev->can.state != CAN_STATE_ERROR_PASSIVE)) {
> 
> CHECK: Unnecessary parentheses around 'cdev->can.state != CAN_STATE_BUS_OFF'
> 	if ((psr & PSR_BO) &&
> 	    (cdev->can.state != CAN_STATE_BUS_OFF)) {
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

Acked-by: Faiz Abbas <faiz_abbas@ti.com>

Thanks,
Faiz

^ permalink raw reply

* Re: [PATCH v12 2/5] can: m_can: Rename m_can_priv to m_can_classdev
From: Faiz Abbas @ 2019-06-25  9:23 UTC (permalink / raw)
  To: Dan Murphy, wg, mkl, davem; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <20190509161109.10499-2-dmurphy@ti.com>

Hi,

On 09/05/19 9:41 PM, Dan Murphy wrote:
> Rename the common m_can_priv class structure to
> m_can_classdev as this is more descriptive.
> 
> Acked-by: Wolfgang Grandegger <wg@grandegger.com>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

Acked-by: Faiz Abbas <faiz_abbas@ti.com>

Thanks,
Faiz

^ permalink raw reply

* Re: [PATCH nf-next v2 1/2] netfilter: nft_meta: add NFT_META_BRI_PVID support
From: wenxu @ 2019-06-25  9:23 UTC (permalink / raw)
  To: pablo, fw; +Cc: netfilter-devel, netdev
In-Reply-To: <1560993460-25569-1-git-send-email-wenxu@ucloud.cn>

Hi pablo,


Any idea about these two patches?


BR

wenxu

On 6/20/2019 9:17 AM, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
>
> nft add table bridge firewall
> nft add chain bridge firewall zones { type filter hook prerouting priority - 300 \; }
> nft add rule bridge firewall zones counter ct zone set vlan id map { 100 : 1, 200 : 2 }
>
> As above set the bridge port with pvid, the received packet don't contain
> the vlan tag which means the packet should belong to vlan 200 through pvid.
> With this pacth user can get the pvid of bridge ports.
>
> So add the following rule for as the first rule in the chain of zones.
>
> nft add rule bridge firewall zones counter meta brvlan set meta brpvid
>
> Signed-off-by: wenxu <wenxu@ucloud.cn>
> ---
>  include/uapi/linux/netfilter/nf_tables.h |  2 ++
>  net/netfilter/nft_meta.c                 | 13 +++++++++++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
> index 31a6b8f..4a16124 100644
> --- a/include/uapi/linux/netfilter/nf_tables.h
> +++ b/include/uapi/linux/netfilter/nf_tables.h
> @@ -793,6 +793,7 @@ enum nft_exthdr_attributes {
>   * @NFT_META_SECPATH: boolean, secpath_exists (!!skb->sp)
>   * @NFT_META_IIFKIND: packet input interface kind name (dev->rtnl_link_ops->kind)
>   * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
> + * @NFT_META_BRI_PVID: packet input bridge port pvid
>   */
>  enum nft_meta_keys {
>  	NFT_META_LEN,
> @@ -823,6 +824,7 @@ enum nft_meta_keys {
>  	NFT_META_SECPATH,
>  	NFT_META_IIFKIND,
>  	NFT_META_OIFKIND,
> +	NFT_META_BRI_PVID,
>  };
>  
>  /**
> diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
> index 987d2d6..cb877e01 100644
> --- a/net/netfilter/nft_meta.c
> +++ b/net/netfilter/nft_meta.c
> @@ -243,6 +243,14 @@ void nft_meta_get_eval(const struct nft_expr *expr,
>  			goto err;
>  		strncpy((char *)dest, p->br->dev->name, IFNAMSIZ);
>  		return;
> +	case NFT_META_BRI_PVID:
> +		if (in == NULL || (p = br_port_get_rtnl_rcu(in)) == NULL)
> +			goto err;
> +		if (br_opt_get(p->br, BROPT_VLAN_ENABLED)) {
> +			nft_reg_store16(dest, br_get_pvid(nbp_vlan_group_rcu(p)));
> +			return;
> +		}
> +		goto err;
>  #endif
>  	case NFT_META_IIFKIND:
>  		if (in == NULL || in->rtnl_link_ops == NULL)
> @@ -370,6 +378,11 @@ static int nft_meta_get_init(const struct nft_ctx *ctx,
>  			return -EOPNOTSUPP;
>  		len = IFNAMSIZ;
>  		break;
> +	case NFT_META_BRI_PVID:
> +		if (ctx->family != NFPROTO_BRIDGE)
> +			return -EOPNOTSUPP;
> +		len = sizeof(u16);
> +		break;
>  #endif
>  	default:
>  		return -EOPNOTSUPP;

^ permalink raw reply

* Re: [PATCH v12 1/5] can: m_can: Create a m_can platform framework
From: Faiz Abbas @ 2019-06-25  9:22 UTC (permalink / raw)
  To: Dan Murphy, wg, mkl, davem; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <20190509161109.10499-1-dmurphy@ti.com>

Hi,

On 09/05/19 9:41 PM, Dan Murphy wrote:
> Create a m_can platform framework that peripheral
> devices can register to and use common code and register sets.
> The peripheral devices may provide read/write and configuration
> support of the IP.
> 
> Acked-by: Wolfgang Grandegger <wg@grandegger.com>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

Acked-by: Faiz Abbas <faiz_abbas@ti.com>

Thanks,
Faiz

^ permalink raw reply

* Re: [PATCH net-next] xdp: Make __mem_id_disconnect static
From: Jesper Dangaard Brouer @ 2019-06-25  9:21 UTC (permalink / raw)
  To: YueHaibing
  Cc: davem, ast, daniel, jakub.kicinski, john.fastabend, linux-kernel,
	netdev, xdp-newbies, bpf, brouer
In-Reply-To: <20190625023137.29272-1-yuehaibing@huawei.com>

On Tue, 25 Jun 2019 10:31:37 +0800
YueHaibing <yuehaibing@huawei.com> wrote:

> Fix sparse warning:
> 
> net/core/xdp.c:88:6: warning:
>  symbol '__mem_id_disconnect' was not declared. Should it be static?

I didn't declare it static as I didn't want it to get inlined.  As
during development I was using kprobes to inspect this function.  In
the end I added a tracepoint in this function as kprobes was not enough
to capture the state needed.

So, I guess we can declare it static.

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>

> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  net/core/xdp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index b29d7b5..829377c 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -85,7 +85,7 @@ static void __xdp_mem_allocator_rcu_free(struct rcu_head *rcu)
>  	kfree(xa);
>  }
>  
> -bool __mem_id_disconnect(int id, bool force)
> +static bool __mem_id_disconnect(int id, bool force)
>  {
>  	struct xdp_mem_allocator *xa;
>  	bool safe_to_remove = true;


-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer
From: Florian Westphal @ 2019-06-25  9:19 UTC (permalink / raw)
  To: Ran Rozenstein
  Cc: Tariq Toukan, Florian Westphal, netdev@vger.kernel.org,
	Maor Gottlieb, edumazet@google.com
In-Reply-To: <AM4PR0501MB276924D7AD83B349AA2A6A0BC5E30@AM4PR0501MB2769.eurprd05.prod.outlook.com>

Ran Rozenstein <ranro@mellanox.com> wrote:
> > On 6/17/2019 5:02 PM, Florian Westphal wrote:
> > > Tariq reported a soft lockup on net-next that Mellanox was able to
> > > bisect to 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list").
> > >
> > > While reviewing above patch I found a regression when addresses have a
> > > lifetime specified.
> > >
> > > Second patch extends rtnetlink.sh to trigger crash (without first
> > > patch applied).
> > >
> > 
> > Thanks Florian.
> > 
> > Ran, can you please test?
> 
> Tested, still reproduce.

Can you be a little more specific? Is there any reproducer?

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: sched: protect against stack overflow in TC act_mirred
From: Florian Westphal @ 2019-06-25  9:15 UTC (permalink / raw)
  To: John Hurley
  Cc: Eyal Birger, Linux Netdev List, David Miller, Florian Westphal,
	Jamal Hadi Salim, Simon Horman, Jakub Kicinski, oss-drivers,
	shmulik
In-Reply-To: <CAK+XE=mOjtp16tdz83RZ-x_jEp3nPRY3smxbG=OfCmGi9_DnXg@mail.gmail.com>

John Hurley <john.hurley@netronome.com> wrote:
> Hi Eyal,
> The value of 4 is basically a revert to what it was on older kernels
> when TC had a TTL value in the skb:
> https://elixir.bootlin.com/linux/v3.19.8/source/include/uapi/linux/pkt_cls.h#L97

IIRC this TTL value was not used ever.

> I also found with my testing that a value greater than 4 was sailing
> close to the edge.
> With a larger value (on my system anyway), I could still trigger a
> stack overflow here.
> I'm not sure on the history of why a value of 4 was selected here but
> it seems to fall into line with my findings.
> Is there a hard requirement for >4 recursive calls here?

One alternative would be to (instead of dropping the skb), to
decrement the ttl and use netif_rx() instead.

^ permalink raw reply

* Re: [PATCH v2] samples: bpf: make the use of xdp samples consistent
From: Toke Høiland-Jørgensen @ 2019-06-25  9:08 UTC (permalink / raw)
  To: Daniel T. Lee, Daniel Borkmann, Alexei Starovoitov; +Cc: bpf, netdev
In-Reply-To: <20190625005536.2516-1-danieltimlee@gmail.com>

"Daniel T. Lee" <danieltimlee@gmail.com> writes:

> Currently, each xdp samples are inconsistent in the use.
> Most of the samples fetch the interface with it's name.
> (ex. xdp1, xdp2skb, xdp_redirect_cpu, xdp_sample_pkts, etc.)
>
> But some of the xdp samples are fetching the interface with
> ifindex by command argument.
>
> This commit enables xdp samples to fetch interface with it's name
> without changing the original index interface fetching.
> (<ifname|ifindex> fetching in the same way as xdp_sample_pkts_user.c does.)
>
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> ---
> Changes in v2:
>   - added xdp_redirect_user.c, xdp_redirect_map_user.c

Great, thanks!

Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: sched: protect against stack overflow in TC act_mirred
From: John Hurley @ 2019-06-25  9:06 UTC (permalink / raw)
  To: Eyal Birger
  Cc: Linux Netdev List, David Miller, Florian Westphal,
	Jamal Hadi Salim, Simon Horman, Jakub Kicinski, oss-drivers,
	shmulik
In-Reply-To: <20190625113010.7da5dbcb@jimi>

On Tue, Jun 25, 2019 at 9:30 AM Eyal Birger <eyal.birger@gmail.com> wrote:
>
> Hi John,
>
> On Mon, 24 Jun 2019 23:13:36 +0100
> John Hurley <john.hurley@netronome.com> wrote:
>
> > TC hooks allow the application of filters and actions to packets at
> > both ingress and egress of the network stack. It is possible, with
> > poor configuration, that this can produce loops whereby an ingress
> > hook calls a mirred egress action that has an egress hook that
> > redirects back to the first ingress etc. The TC core classifier
> > protects against loops when doing reclassifies but there is no
> > protection against a packet looping between multiple hooks and
> > recursively calling act_mirred. This can lead to stack overflow
> > panics.
> >
> > Add a per CPU counter to act_mirred that is incremented for each
> > recursive call of the action function when processing a packet. If a
> > limit is passed then the packet is dropped and CPU counter reset.
> >
> > Note that this patch does not protect against loops in TC datapaths.
> > Its aim is to prevent stack overflow kernel panics that can be a
> > consequence of such loops.
> >
> > Signed-off-by: John Hurley <john.hurley@netronome.com>
> > Reviewed-by: Simon Horman <simon.horman@netronome.com>
> > ---
> >  net/sched/act_mirred.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
> > index 8c1d736..c3fce36 100644
> > --- a/net/sched/act_mirred.c
> > +++ b/net/sched/act_mirred.c
> > @@ -27,6 +27,9 @@
> >  static LIST_HEAD(mirred_list);
> >  static DEFINE_SPINLOCK(mirred_list_lock);
> >
> > +#define MIRRED_RECURSION_LIMIT    4
>
> Could you increase the limit to maybe 6 or 8? I am aware of cases where
> mirred ingress is used for cascading several layers of logical network
> interfaces and 4 seems a little limiting.
>
> Thanks,
> Eyal.

Hi Eyal,
The value of 4 is basically a revert to what it was on older kernels
when TC had a TTL value in the skb:
https://elixir.bootlin.com/linux/v3.19.8/source/include/uapi/linux/pkt_cls.h#L97

I also found with my testing that a value greater than 4 was sailing
close to the edge.
With a larger value (on my system anyway), I could still trigger a
stack overflow here.
I'm not sure on the history of why a value of 4 was selected here but
it seems to fall into line with my findings.
Is there a hard requirement for >4 recursive calls here?

John

^ permalink raw reply

* RE: [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer
From: Ran Rozenstein @ 2019-06-25  9:04 UTC (permalink / raw)
  To: Tariq Toukan, Florian Westphal, netdev@vger.kernel.org
  Cc: Maor Gottlieb, edumazet@google.com
In-Reply-To: <08e102a0-8051-e582-56c8-d721bfc9e8b9@mellanox.com>



> -----Original Message-----
> From: Tariq Toukan
> Sent: Monday, June 17, 2019 19:16
> To: Florian Westphal <fw@strlen.de>; netdev@vger.kernel.org
> Cc: Ran Rozenstein <ranro@mellanox.com>; Maor Gottlieb
> <maorg@mellanox.com>; edumazet@google.com
> Subject: Re: [PATCH net-next 0/2] net: ipv4: remove erroneous
> advancement of list pointer
> 
> 
> 
> On 6/17/2019 5:02 PM, Florian Westphal wrote:
> > Tariq reported a soft lockup on net-next that Mellanox was able to
> > bisect to 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list").
> >
> > While reviewing above patch I found a regression when addresses have a
> > lifetime specified.
> >
> > Second patch extends rtnetlink.sh to trigger crash (without first
> > patch applied).
> >
> 
> Thanks Florian.
> 
> Ran, can you please test?

Tested, still reproduce.

^ permalink raw reply

* Re: [PATCH v5 4/5] net: macb: add support for high speed interface
From: Russell King - ARM Linux admin @ 2019-06-25  9:03 UTC (permalink / raw)
  To: Parshuram Raju Thombare
  Cc: andrew@lunn.ch, nicolas.ferre@microchip.com, davem@davemloft.net,
	f.fainelli@gmail.com, netdev@vger.kernel.org,
	hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
	Rafal Ciepiela, Anil Joy Varughese, Piotr Sroka
In-Reply-To: <SN2PR07MB2480CF15E11D54DA8C3B7319C1E30@SN2PR07MB2480.namprd07.prod.outlook.com>

On Tue, Jun 25, 2019 at 08:49:33AM +0000, Parshuram Raju Thombare wrote:
> >>  	switch (state->interface) {
> >>  	case PHY_INTERFACE_MODE_NA:
> >> +	case PHY_INTERFACE_MODE_USXGMII:
> >> +	case PHY_INTERFACE_MODE_10GKR:
> >> +		if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE) {
> >> +			phylink_set(mask, 10000baseCR_Full);
> >> +			phylink_set(mask, 10000baseER_Full);
> >> +			phylink_set(mask, 10000baseKR_Full);
> >> +			phylink_set(mask, 10000baseLR_Full);
> >> +			phylink_set(mask, 10000baseLRM_Full);
> >> +			phylink_set(mask, 10000baseSR_Full);
> >> +			phylink_set(mask, 10000baseT_Full);
> >> +			phylink_set(mask, 5000baseT_Full);
> >> +			phylink_set(mask, 2500baseX_Full);
> >> +			phylink_set(mask, 1000baseX_Full);
> >> +		}
> >If MACB_CAPS_GIGABIT_MODE_AVAILABLE is not set, are these interface
> >modes supported by the hardware?  If the PHY interface mode is not
> >supported, then the returned support mask must be cleared.[] 
> There are some configs which uses this macro to limit data rate to 100M 
> even if hardware support higher rates.

I'm sorry, this response does not address my statement, maybe I wasn't
clear enough.  I am asking about the *PHY* interface modes, in
other words (e.g.) PHY_INTERFACE_MODE_USXGMII.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* RE: [PATCH v5 4/5] net: macb: add support for high speed interface
From: Parshuram Raju Thombare @ 2019-06-25  8:55 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: andrew@lunn.ch, nicolas.ferre@microchip.com, davem@davemloft.net,
	f.fainelli@gmail.com, netdev@vger.kernel.org,
	hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
	Rafal Ciepiela, Anil Joy Varughese, Piotr Sroka
In-Reply-To: <20190624134755.u3oq3xr6uergnfs5@shell.armlinux.org.uk>

>> +static inline void gem_mac_configure(struct macb *bp, int speed)
>> +	switch (speed) {
>> +	case SPEED_1000:
>> +		gem_writel(bp, NCFGR, GEM_BIT(GBE) |
>> +			   gem_readl(bp, NCFGR));
>> +		break;
>> +	case SPEED_100:
>> +		macb_writel(bp, NCFGR, MACB_BIT(SPD) |
>> +			    macb_readl(bp, NCFGR));
>What happens to the NCFGR register if we call mac_config() first for
>a 1G speed, then 100M and finally 10M - what value does the NCFGR
>register end up with?
>
>I suspect it ends up with both the GBE and SPD bits set, and that is
>probably not what you want.

No, In gem_mac_config GBE and SPD bits are always cleared
before setting appropriate bits as per requested speed, duplex mode.


Regards,
Parshuram Thombare

^ permalink raw reply

* Re: [PATCH v2 2/2] net: macb: Kconfig: Rename Atmel to Cadence
From: Nicolas.Ferre @ 2019-06-25  8:54 UTC (permalink / raw)
  To: palmer, harinik; +Cc: davem, netdev, linux-kernel
In-Reply-To: <20190625084828.540-3-palmer@sifive.com>

On 25/06/2019 at 10:48, Palmer Dabbelt wrote:
> The help text makes it look like NET_VENDOR_CADENCE enables support for
> Atmel devices, when in reality it's a driver written by Atmel that
> supports Cadence devices.  This may confuse users that have this device
> on a non-Atmel SoC.
> 
> The fix is just s/Atmel/Cadence/, but I did go and re-wrap the Kconfig
> help text as that change caused it to go over 80 characters.
> 
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>

Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

> ---
>   drivers/net/ethernet/cadence/Kconfig | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
> index 64d8d6ee7739..f4b3bd85dfe3 100644
> --- a/drivers/net/ethernet/cadence/Kconfig
> +++ b/drivers/net/ethernet/cadence/Kconfig
> @@ -1,6 +1,6 @@
>   # SPDX-License-Identifier: GPL-2.0-only
>   #
> -# Atmel device configuration
> +# Cadence device configuration
>   #
>   
>   config NET_VENDOR_CADENCE
> @@ -13,8 +13,8 @@ config NET_VENDOR_CADENCE
>   	  If unsure, say Y.
>   
>   	  Note that the answer to this question doesn't directly affect the
> -	  kernel: saying N will just cause the configurator to skip all
> -	  the remaining Atmel network card questions. If you say Y, you will be
> +	  kernel: saying N will just cause the configurator to skip all the
> +	  remaining Cadence network card questions. If you say Y, you will be
>   	  asked for your specific card in the following questions.
>   
>   if NET_VENDOR_CADENCE
> 


-- 
Nicolas Ferre

^ permalink raw reply

* Re: [PATCH v2 1/2] net: macb: Kconfig: Make MACB depend on COMMON_CLK
From: Nicolas.Ferre @ 2019-06-25  8:54 UTC (permalink / raw)
  To: palmer, harinik; +Cc: davem, netdev, linux-kernel
In-Reply-To: <20190625084828.540-2-palmer@sifive.com>

On 25/06/2019 at 10:48, Palmer Dabbelt wrote:
> commit c218ad559020 ("macb: Add support for SiFive FU540-C000") added a
> dependency on the common clock framework to the macb driver, but didn't
> express that dependency in Kconfig.  As a result macb now fails to
> compile on systems without COMMON_CLK, which specifically causes a build
> failure on powerpc allyesconfig.
> 
> This patch adds the dependency, which results in the macb driver no
> longer being selectable on systems without the common clock framework.
> All known systems that have this device already support the common clock
> framework, so this should not cause trouble for any uses.  Supporting
> both the FU540-C000 and systems without COMMON_CLK is quite ugly.
> 
> I've build tested this on powerpc allyesconfig and RISC-V defconfig
> (which selects MACB), but I have not even booted the resulting kernels.
> 
> Fixes: c218ad559020 ("macb: Add support for SiFive FU540-C000")
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>

Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Thanks!

> ---
>   drivers/net/ethernet/cadence/Kconfig | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
> index 1766697c9c5a..64d8d6ee7739 100644
> --- a/drivers/net/ethernet/cadence/Kconfig
> +++ b/drivers/net/ethernet/cadence/Kconfig
> @@ -21,7 +21,7 @@ if NET_VENDOR_CADENCE
>   
>   config MACB
>   	tristate "Cadence MACB/GEM support"
> -	depends on HAS_DMA
> +	depends on HAS_DMA && COMMON_CLK
>   	select PHYLIB
>   	---help---
>   	  The Cadence MACB ethernet interface is found on many Atmel AT32 and
> @@ -42,7 +42,7 @@ config MACB_USE_HWSTAMP
>   
>   config MACB_PCI
>   	tristate "Cadence PCI MACB/GEM support"
> -	depends on MACB && PCI && COMMON_CLK
> +	depends on MACB && PCI
>   	---help---
>   	  This is PCI wrapper for MACB driver.
>   
> 


-- 
Nicolas Ferre

^ permalink raw reply

* Re: [PATCH v3] net: netfilter: Fix rpfilter dropping vrf packets by mistake
From: linmiaohe @ 2019-06-25  8:51 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: kadlec@blackhole.kfki.hu, fw@strlen.de, davem@davemloft.net,
	kuznet@ms2.inr.ac.ru, yoshfuji@linux-ipv6.org,
	netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	dsahern@gmail.com, Mingfangsen


On Wed, Jun 19, 2019 at 09:49:04AM +0000, linmiaohe wrote:
> 
> On 2019/6/18 23:58, Pablo Neira Ayuso wrote:
> > On Thu, Apr 25, 2019 at 09:43:53PM +0800, linmiaohe wrote:
> >> From: Miaohe Lin <linmiaohe@huawei.com>
> >>
> >> When firewalld is enabled with ipv4/ipv6 rpfilter, vrf
> >> ipv4/ipv6 packets will be dropped because in device is vrf but out 
> >> device is an enslaved device. So failed with the check of the 
> >> rpfilter.
> >>
> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> >> ---
> >> --- a/net/ipv4/netfilter/ipt_rpfilter.c
> >> +++ b/net/ipv4/netfilter/ipt_rpfilter.c
> >> @@ -81,6 +81,7 @@ static bool rpfilter_mt(const struct sk_buff *skb, struct xt_action_param *par)
> >>  	flow.flowi4_mark = info->flags & XT_RPFILTER_VALID_MARK ? skb->mark : 0;
> >>  	flow.flowi4_tos = RT_TOS(iph->tos);
> >>  	flow.flowi4_scope = RT_SCOPE_UNIVERSE;
> >> +	flow.flowi4_oif = l3mdev_master_ifindex_rcu(xt_in(par));
> >>
> >>  	return rpfilter_lookup_reverse(xt_net(par), &flow, xt_in(par),
> >> --- a/net/ipv6/netfilter/ip6t_rpfilter.c
> >> +++ b/net/ipv6/netfilter/ip6t_rpfilter.c
> >> @@ -58,7 +58,9 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
> >>  	if (rpfilter_addr_linklocal(&iph->saddr)) {
> >>  		lookup_flags |= RT6_LOOKUP_F_IFACE;
> >>  		fl6.flowi6_oif = dev->ifindex;
> >> -	} else if ((flags & XT_RPFILTER_LOOSE) == 0)
> >> +	} else if (((flags & XT_RPFILTER_LOOSE) == 0) ||
> >> +		   (netif_is_l3_master(dev)) ||
> >> +		   (netif_is_l3_slave(dev)))
> >>  		fl6.flowi6_oif = dev->ifindex;
> >>
> >>  	rt = (void *)ip6_route_lookup(net, &fl6, skb, lookup_flags); @@
> >> -73,6 +75,12 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
> >>  		goto out;
> >>  	}
> >>
> >> +	if (netif_is_l3_master(dev)) {
> >> +		dev = dev_get_by_index_rcu(dev_net(dev), IP6CB(skb)->iif);
> >> +		if (!dev)
> >> +			goto out;
> >> +	}
> > 
> > So, for the l3 device cases this makes:
> > 
> > #1 ip6_route_lookup() to fetch the route, using the device in xt_in()
> >    (the _LOOSE flag is ignored for the l3 device case).
> > 
> > #2 If this is a l3dev master, then you make a global lookup for the
> >    device using IP6CB(skb)->iif.
> > 
> > #3 You check if route matches with the device, using the new device
> >    from the lookup:
> > 
> >    if (rt->rt6i_idev->dev == dev ...
> > 
> > If there is no other way to fix this, OK, that's fair enough.
> > 
> > Still this fix looks a bit tricky to me.
> > 
> > And this assymmetric between the IPv4 and IPv6 codebase looks rare.
> > 
> > Probably someone can explain me this in more detail? I'd appreciate.
> > 
> > Thanks!
> > 
> Thanks for explaining.
>
> Something must be wrong in all these helper function logic because this new code logic is hard to follow for the IPv6 chunk...
>
> Can you explore a more readable fix?
>
> So I'm not inclined to quickly take this patch.
>
> Thanks.

Thanks, I will try a more readable fix.

^ permalink raw reply

* RE: [PATCH v5 4/5] net: macb: add support for high speed interface
From: Parshuram Raju Thombare @ 2019-06-25  8:49 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: andrew@lunn.ch, nicolas.ferre@microchip.com, davem@davemloft.net,
	f.fainelli@gmail.com, netdev@vger.kernel.org,
	hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
	Rafal Ciepiela, Anil Joy Varughese, Piotr Sroka
In-Reply-To: <20190624134755.u3oq3xr6uergnfs5@shell.armlinux.org.uk>

>>  	switch (state->interface) {
>>  	case PHY_INTERFACE_MODE_NA:
>> +	case PHY_INTERFACE_MODE_USXGMII:
>> +	case PHY_INTERFACE_MODE_10GKR:
>> +		if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE) {
>> +			phylink_set(mask, 10000baseCR_Full);
>> +			phylink_set(mask, 10000baseER_Full);
>> +			phylink_set(mask, 10000baseKR_Full);
>> +			phylink_set(mask, 10000baseLR_Full);
>> +			phylink_set(mask, 10000baseLRM_Full);
>> +			phylink_set(mask, 10000baseSR_Full);
>> +			phylink_set(mask, 10000baseT_Full);
>> +			phylink_set(mask, 5000baseT_Full);
>> +			phylink_set(mask, 2500baseX_Full);
>> +			phylink_set(mask, 1000baseX_Full);
>> +		}
>If MACB_CAPS_GIGABIT_MODE_AVAILABLE is not set, are these interface
>modes supported by the hardware?  If the PHY interface mode is not
>supported, then the returned support mask must be cleared.[] 
There are some configs which uses this macro to limit data rate to 100M 
even if hardware support higher rates.
Empty link mode mask is initialized at the beginning and supported link 
modes are added to it and at the end of this function this mask is AND'ed 
with supported mask.

>> +static inline void gem_mac_configure(struct macb *bp, int speed)
>> +{
>> +	if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII)
>> +		gem_writel(bp, NCFGR, GEM_BIT(SGMIIEN) |
>> +			   GEM_BIT(PCSSEL) |
>> +			   gem_readl(bp, NCFGR));
>Is this still necessary?
Sorry, missed this. I will remove in next patch set.

Regards,
Parshuram Thombare

^ permalink raw reply

* Re: [PATCH net-next 1/1] tc-testing: Restore original behaviour for namespaces in tdc
From: Nicolas Dichtel @ 2019-06-25  8:49 UTC (permalink / raw)
  To: Lucas Bates, davem
  Cc: netdev, jhs, xiyou.wangcong, jiri, mleitner, vladbu, dcaratti,
	kernel
In-Reply-To: <1561424427-9949-1-git-send-email-lucasb@mojatatu.com>

Le 25/06/2019 à 03:00, Lucas Bates a écrit :
> This patch restores the original behaviour for tdc prior to the
> introduction of the plugin system, where the network namespace
> functionality was split from the main script.
> 
> It introduces the concept of required plugins for testcases,
> and will automatically load any plugin that isn't already
> enabled when said plugin is required by even one testcase.
> 
> Additionally, the -n option for the nsPlugin is deprecated
> so the default action is to make use of the namespaces.
> Instead, we introduce -N to not use them, but still create
> the veth pair.
> 
> buildebpfPlugin's -B option is also deprecated.
> 
> If a test cases requires the features of a specific plugin
> in order to pass, it should instead include a new key/value
> pair describing plugin interactions:
> 
>         "plugins": {
>                 "requires": "buildebpfPlugin"
>         },
> 
> A test case can have more than one required plugin: a list
> can be inserted as the value for 'requires'.
> 
> Signed-off-by: Lucas Bates <lucasb@mojatatu.com>

Thank you for the follow up!

Tested-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

[snip]

> @@ -550,6 +614,7 @@ def filter_tests_by_category(args, testlist):
>  
>      return answer
>  
> +
>  def get_test_cases(args):
>      """
>      If a test case file is specified, retrieve tests from that file.
nit: this new line is probably a leftover of a previous version ;-)

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox