1. 16 2月, 2021 1 次提交
    • V
      net: mscc: ocelot: avoid type promotion when calling ocelot_ifh_set_dest · 1f778d50
      Vladimir Oltean 提交于
      Smatch is confused by the fact that a 32-bit BIT(port) macro is passed
      as argument to the ocelot_ifh_set_dest function and warns:
      
      ocelot_xmit() warn: should '(((1))) << (dp->index)' be a 64 bit type?
      seville_xmit() warn: should '(((1))) << (dp->index)' be a 64 bit type?
      
      The destination port mask is copied into a 12-bit field of the packet,
      starting at bit offset 67 and ending at 56.
      
      So this DSA tagging protocol supports at most 12 bits, which is clearly
      less than 32. Attempting to send to a port number > 12 will cause the
      packing() call to truncate way before there will be 32-bit truncation
      due to type promotion of the BIT(port) argument towards u64.
      
      Therefore, smatch's fears that BIT(port) will do the wrong thing and
      cause unexpected truncation for "port" values >= 32 are unfounded.
      Nonetheless, let's silence the warning by explicitly passing an u64
      value to ocelot_ifh_set_dest, such that the compiler does not need to do
      a questionable type promotion.
      Reported-by: Nkernel test robot <lkp@intel.com>
      Reported-by: NDan Carpenter <dan.carpenter@oracle.com>
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      1f778d50
  2. 15 2月, 2021 4 次提交
    • V
      net: dsa: tag_ocelot_8021q: add support for PTP timestamping · 0a6f17c6
      Vladimir Oltean 提交于
      For TX timestamping, we use the felix_txtstamp method which is common
      with the regular (non-8021q) ocelot tagger. This method says that skb
      deferral is needed, prepares a timestamp request ID, and puts a clone of
      the skb in a queue waiting for the timestamp IRQ.
      
      felix_txtstamp is called by dsa_skb_tx_timestamp() just before the
      tagger's xmit method. In the tagger xmit, we divert the packets
      classified by dsa_skb_tx_timestamp() as PTP towards the MMIO-based
      injection registers, and we declare them as dead towards dsa_slave_xmit.
      If not PTP, we proceed with normal tag_8021q stuff.
      
      Then the timestamp IRQ fires, the clone queued up from felix_txtstamp is
      matched to the TX timestamp retrieved from the switch's FIFO based on
      the timestamp request ID, and the clone is delivered to the stack.
      
      On RX, thanks to the VCAP IS2 rule that redirects the frames with an
      EtherType for 1588 towards two destinations:
      - the CPU port module (for MMIO based extraction) and
      - if the "no XTR IRQ" workaround is in place, the dsa_8021q CPU port
      the relevant data path processing starts in the ptp_classify_raw BPF
      classifier installed by DSA in the RX data path (post tagger, which is
      completely unaware that it saw a PTP packet).
      
      This time we can't reuse the same implementation of .port_rxtstamp that
      also works with the default ocelot tagger. That is because felix_rxtstamp
      is given an skb with a freshly stripped DSA header, and it says "I don't
      need deferral for its RX timestamp, it's right in it, let me show you";
      and it just points to the header right behind skb->data, from where it
      unpacks the timestamp and annotates the skb with it.
      
      The same thing cannot happen with tag_ocelot_8021q, because for one
      thing, the skb did not have an extraction frame header in the first
      place, but a VLAN tag with no timestamp information. So the code paths
      in felix_rxtstamp for the regular and 8021q tagger are completely
      independent. With tag_8021q, the timestamp must come from the packet's
      duplicate delivered to the CPU port module, but there is potentially
      complex logic to be handled [ and prone to reordering ] if we were to
      just start reading packets from the CPU port module, and try to match
      them to the one we received over Ethernet and which needs an RX
      timestamp. So we do something simple: we tell DSA "give me some time to
      think" (we request skb deferral by returning false from .port_rxtstamp)
      and we just drop the frame we got over Ethernet with no attempt to match
      it to anything - we just treat it as a notification that there's data to
      be processed from the CPU port module's queues. Then we proceed to read
      the packets from those, one by one, which we deliver up the stack,
      timestamped, using netif_rx - the same function that any driver would
      use anyway if it needed RX timestamp deferral. So the assumption is that
      we'll come across the PTP packet that triggered the CPU extraction
      notification eventually, but we don't know when exactly. Thanks to the
      VCAP IS2 trap/redirect rule and the exclusion of the CPU port module
      from the flooding replicators, only PTP frames should be present in the
      CPU port module's RX queues anyway.
      
      There is just one conflict between the VCAP IS2 trapping rule and the
      semantics of the BPF classifier. Namely, ptp_classify_raw() deems
      general messages as non-timestampable, but still, those are trapped to
      the CPU port module since they have an EtherType of ETH_P_1588. So, if
      the "no XTR IRQ" workaround is in place, we need to run another BPF
      classifier on the frames extracted over MMIO, to avoid duplicates being
      sent to the stack (once over Ethernet, once over MMIO). It doesn't look
      like it's possible to install VCAP IS2 rules based on keys extracted
      from the 1588 frame headers.
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: NFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      0a6f17c6
    • V
      net: mscc: ocelot: refactor ocelot_xtr_irq_handler into ocelot_xtr_poll · 924ee317
      Vladimir Oltean 提交于
      Since the felix DSA driver will need to poll the CPU port module for
      extracted frames as well, let's create some common functions that read
      an Extraction Frame Header, and then an skb, from a CPU extraction
      group.
      
      We abuse the struct ocelot_ops :: port_to_netdev function a little bit,
      in order to retrieve the DSA port net_device or the ocelot switchdev
      net_device based on the source port information from the Extraction
      Frame Header, but it's all in the benefit of code simplification -
      netdev_alloc_skb needs it. Originally, the port_to_netdev method was
      intended for parsing act->dev from tc flower offload code.
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: NFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      924ee317
    • V
      net: mscc: ocelot: use common tag parsing code with DSA · 40d3f295
      Vladimir Oltean 提交于
      The Injection Frame Header and Extraction Frame Header that the switch
      prepends to frames over the NPI port is also prepended to frames
      delivered over the CPU port module's queues.
      
      Let's unify the handling of the frame headers by making the ocelot
      driver call some helpers exported by the DSA tagger. Among other things,
      this allows us to get rid of the strange cpu_to_be32 when transmitting
      the Injection Frame Header on ocelot, since the packing API uses
      network byte order natively (when "quirks" is 0).
      
      The comments above ocelot_gen_ifh talk about setting pop_cnt to 3, and
      the cpu extraction queue mask to something, but the code doesn't do it,
      so we don't do it either.
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: NFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      40d3f295
    • V
      net: mscc: ocelot: refactor ocelot_port_inject_frame out of ocelot_port_xmit · 137ffbc4
      Vladimir Oltean 提交于
      The felix DSA driver will inject some frames through register MMIO, same
      as ocelot switchdev currently does. So we need to be able to reuse the
      common code.
      
      Also create some shim definitions, since the DSA tagger can be compiled
      without support for the switch driver.
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: NFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      137ffbc4
  3. 13 2月, 2021 2 次提交
  4. 10 2月, 2021 1 次提交
    • V
      net: dsa: felix: implement port flushing on .phylink_mac_link_down · eb4733d7
      Vladimir Oltean 提交于
      There are several issues which may be seen when the link goes down while
      forwarding traffic, all of which can be attributed to the fact that the
      port flushing procedure from the reference manual was not closely
      followed.
      
      With flow control enabled on both the ingress port and the egress port,
      it may happen when a link goes down that Ethernet packets are in flight.
      In flow control mode, frames are held back and not dropped. When there
      is enough traffic in flight (example: iperf3 TCP), then the ingress port
      might enter congestion and never exit that state. This is a problem,
      because it is the egress port's link that went down, and that has caused
      the inability of the ingress port to send packets to any other port.
      This is solved by flushing the egress port's queues when it goes down.
      
      There is also a problem when performing stream splitting for
      IEEE 802.1CB traffic (not yet upstream, but a sort of multicast,
      basically). There, if one port from the destination ports mask goes
      down, splitting the stream towards the other destinations will no longer
      be performed. This can be traced down to this line:
      
      	ocelot_port_writel(ocelot_port, 0, DEV_MAC_ENA_CFG);
      
      which should have been instead, as per the reference manual:
      
      	ocelot_port_rmwl(ocelot_port, 0, DEV_MAC_ENA_CFG_RX_ENA,
      			 DEV_MAC_ENA_CFG);
      
      Basically only DEV_MAC_ENA_CFG_RX_ENA should be disabled, but not
      DEV_MAC_ENA_CFG_TX_ENA - I don't have further insight into why that is
      the case, but apparently multicasting to several ports will cause issues
      if at least one of them doesn't have DEV_MAC_ENA_CFG_TX_ENA set.
      
      I am not sure what the state of the Ocelot VSC7514 driver is, but
      probably not as bad as Felix/Seville, since VSC7514 uses phylib and has
      the following in ocelot_adjust_link:
      
      	if (!phydev->link)
      		return;
      
      therefore the port is not really put down when the link is lost, unlike
      the DSA drivers which use .phylink_mac_link_down for that.
      
      Nonetheless, I put ocelot_port_flush() in the common ocelot.c because it
      needs to access some registers from drivers/net/ethernet/mscc/ocelot_rew.h
      which are not exported in include/soc/mscc/ and a bugfix patch should
      probably not move headers around.
      
      Fixes: bdeced75 ("net: dsa: felix: Add PCS operations for PHYLINK")
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      eb4733d7
  5. 07 2月, 2021 8 次提交
  6. 30 1月, 2021 3 次提交
    • V
      net: dsa: felix: perform switch setup for tag_8021q · e21268ef
      Vladimir Oltean 提交于
      Unlike sja1105, the only other user of the software-defined tag_8021q.c
      tagger format, the implementation we choose for the Felix DSA switch
      driver preserves full functionality under a vlan_filtering bridge
      (i.e. IP termination works through the DSA user ports under all
      circumstances).
      
      The tag_8021q protocol just wants:
      - Identifying the ingress switch port based on the RX VLAN ID, as seen
        by the CPU. We achieve this by using the TCAM engines (which are also
        used for tc-flower offload) to push the RX VLAN as a second, outer
        tag, on egress towards the CPU port.
      - Steering traffic injected into the switch from the network stack
        towards the correct front port based on the TX VLAN, and consuming
        (popping) that header on the switch's egress.
      
      A tc-flower pseudocode of the static configuration done by the driver
      would look like this:
      
      $ tc qdisc add dev <cpu-port> clsact
      $ for eth in swp0 swp1 swp2 swp3; do \
      	tc filter add dev <cpu-port> egress flower indev ${eth} \
      		action vlan push id <rxvlan> protocol 802.1ad; \
      	tc filter add dev <cpu-port> ingress protocol 802.1Q flower
      		vlan_id <txvlan> action vlan pop \
      		action mirred egress redirect dev ${eth}; \
      done
      
      but of course since DSA does not register network interfaces for the CPU
      port, this configuration would be impossible for the user to do. Also,
      due to the same reason, it is impossible for the user to inadvertently
      delete these rules using tc. These rules do not collide in any way with
      tc-flower, they just consume some TCAM space, which is something we can
      live with.
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: NFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: NJakub Kicinski <kuba@kernel.org>
      e21268ef
    • V
      net: mscc: ocelot: don't use NPI tag prefix for the CPU port module · cacea62f
      Vladimir Oltean 提交于
      Context: Ocelot switches put the injection/extraction frame header in
      front of the Ethernet header. When used in NPI mode, a DSA master would
      see junk instead of the destination MAC address, and it would most
      likely drop the packets. So the Ocelot frame header can have an optional
      prefix, which is just "ff:ff:ff:ff:ff:fe > ff:ff:ff:ff:ff:ff" padding
      put before the actual tag (still before the real Ethernet header) such
      that the DSA master thinks it's looking at a broadcast frame with a
      strange EtherType.
      
      Unfortunately, a lesson learned in commit 69df578c ("net: mscc:
      ocelot: eliminate confusion between CPU and NPI port") seems to have
      been forgotten in the meanwhile.
      
      The CPU port module and the NPI port have independent settings for the
      length of the tag prefix. However, the driver is using the same variable
      to program both of them.
      
      There is no reason really to use any tag prefix with the CPU port
      module, since that is not connected to any Ethernet port. So this patch
      makes the inj_prefix and xtr_prefix variables apply only to the NPI
      port (which the switchdev ocelot_vsc7514 driver does not use).
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: NFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: NJakub Kicinski <kuba@kernel.org>
      cacea62f
    • V
      net: mscc: ocelot: reapply bridge forwarding mask on bonding join/leave · 9b521250
      Vladimir Oltean 提交于
      Applying the bridge forwarding mask currently is done only on the STP
      state changes for any port. But it depends on both STP state changes,
      and bonding interface state changes. Export the bit that recalculates
      the forwarding mask so that it could be reused, and call it when a port
      starts and stops offloading a bonding interface.
      
      Now that the logic is split into a separate function, we can rename "p"
      into "port", since the "port" variable was already taken in
      ocelot_bridge_stp_state_set. Also, we can rename "i" into "lag", to make
      it more clear what is it that we're iterating through.
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: NAlexandre Belloni <alexandre.belloni@bootlin.com>
      Reviewed-by: NFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: NJakub Kicinski <kuba@kernel.org>
      9b521250
  7. 21 1月, 2021 1 次提交
    • A
      net: mscc: ocelot: Fix multicast to the CPU port · 584b7cfc
      Alban Bedel 提交于
      Multicast entries in the MAC table use the high bits of the MAC
      address to encode the ports that should get the packets. But this port
      mask does not work for the CPU port, to receive these packets on the
      CPU port the MAC_CPU_COPY flag must be set.
      
      Because of this IPv6 was effectively not working because neighbor
      solicitations were never received. This was not apparent before commit
      9403c158 (net: mscc: ocelot: support IPv4, IPv6 and plain Ethernet mdb
      entries) as the IPv6 entries were broken so all incoming IPv6
      multicast was then treated as unknown and flooded on all ports.
      
      To fix this problem rework the ocelot_mact_learn() to set the
      MAC_CPU_COPY flag when a multicast entry that target the CPU port is
      added. For this we have to read back the ports endcoded in the pseudo
      MAC address by the caller. It is not a very nice design but that avoid
      changing the callers and should make backporting easier.
      Signed-off-by: NAlban Bedel <alban.bedel@aerq.com>
      Fixes: 9403c158 ("net: mscc: ocelot: support IPv4, IPv6 and plain Ethernet mdb entries")
      Link: https://lore.kernel.org/r/20210119140638.203374-1-alban.bedel@aerq.comSigned-off-by: NJakub Kicinski <kuba@kernel.org>
      584b7cfc
  8. 16 1月, 2021 3 次提交
    • V
      net: mscc: ocelot: configure watermarks using devlink-sb · f59fd9ca
      Vladimir Oltean 提交于
      Using devlink-sb, we can configure 12/16 (the important 75%) of the
      switch's controlling watermarks for congestion drops, and we can monitor
      50% of the watermark occupancies (we can monitor the reservation
      watermarks, but not the sharing watermarks, which are exposed as pool
      sizes).
      
      The following definitions can be made:
      
      SB_BUF=0 # The devlink-sb for frame buffers
      SB_REF=1 # The devlink-sb for frame references
      POOL_ING=0 # The pool for ingress traffic. Both devlink-sb instances
                 # have one of these.
      POOL_EGR=1 # The pool for egress traffic. Both devlink-sb instances
                 # have one of these.
      
      Editing the hardware watermarks is done in the following way:
      BUF_xxxx_I is accessed when sb=$SB_BUF and pool=$POOL_ING
      REF_xxxx_I is accessed when sb=$SB_REF and pool=$POOL_ING
      BUF_xxxx_E is accessed when sb=$SB_BUF and pool=$POOL_EGR
      REF_xxxx_E is accessed when sb=$SB_REF and pool=$POOL_EGR
      
      Configuring the sharing watermarks for COL_SHR(dp=0) is done implicitly
      by modifying the corresponding pool size. By default, the pool size has
      maximum size, so this can be skipped.
      
      devlink sb pool set pci/0000:00:00.5 sb $SB_BUF pool $POOL_ING \
      	size 129840 thtype static
      
      Since by default there is no buffer reservation, the above command has
      maxed out BUF_COL_SHR_I(dp=0).
      
      Configuring the per-port reservation watermark (P_RSRV) is done in the
      following way:
      
      devlink sb port pool set pci/0000:00:00.5/0 sb $SB_BUF \
      	pool $POOL_ING th 1000
      
      The above command sets BUF_P_RSRV_I(port 0) to 1000 bytes. After this
      command, the sharing watermarks are internally reconfigured with 1000
      bytes less, i.e. from 129840 bytes to 128840 bytes.
      
      Configuring the per-port-tc reservation watermarks (Q_RSRV) is done in
      the following way:
      
      for tc in {0..7}; do
      	devlink sb tc bind set pci/0000:00:00.5/0 sb 0 tc $tc \
      		type ingress pool $POOL_ING \
      		th 3000
      done
      
      The above command sets BUF_Q_RSRV_I(port 0, tc 0..7) to 3000 bytes.
      The sharing watermarks are again reconfigured with 24000 bytes less.
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: NJakub Kicinski <kuba@kernel.org>
      f59fd9ca
    • V
      net: mscc: ocelot: initialize watermarks to sane defaults · a4ae997a
      Vladimir Oltean 提交于
      This is meant to be a gentle introduction into the world of watermarks
      on ocelot. The code is placed in ocelot_devlink.c because it will be
      integrated with devlink, even if it isn't right now.
      
      My first step was intended to be to replicate the default configuration
      of the congestion watermarks programatically, since they are now going
      to be tuned by the user.
      
      But after studying and understanding through trial and error how they
      work, I now believe that the configuration used out of reset does not do
      justice to the word "reservation", since the sum of all reservations
      exceeds the total amount of resources (otherwise said, all reservations
      cannot be fulfilled at the same time, which means that, contrary to the
      reference manual, they don't guarantee anything).
      
      As an example, here's a dump of the reservation watermarks for frame
      buffers, for port 0 (for brevity, the ports 1-6 were omitted, but they
      have the same configuration):
      
      BUF_Q_RSRV_I(port 0, prio 0) = max 3000 bytes
      BUF_Q_RSRV_I(port 0, prio 1) = max 3000 bytes
      BUF_Q_RSRV_I(port 0, prio 2) = max 3000 bytes
      BUF_Q_RSRV_I(port 0, prio 3) = max 3000 bytes
      BUF_Q_RSRV_I(port 0, prio 4) = max 3000 bytes
      BUF_Q_RSRV_I(port 0, prio 5) = max 3000 bytes
      BUF_Q_RSRV_I(port 0, prio 6) = max 3000 bytes
      BUF_Q_RSRV_I(port 0, prio 7) = max 3000 bytes
      
      Otherwise said, every port-tc has an ingress reservation of 3000 bytes,
      and there are 7 ports in VSC9959 Felix (6 user ports and 1 CPU port).
      Concentrating only on the ingress reservations, there are, in total,
      8 [traffic classes] x 7 [ports] x 3000 [bytes] = 168,000 bytes of memory
      reserved on ingress.
      But, surprise, Felix only has 128 KB of packet buffer in total...
      A similar thing happens with Seville, which has a larger packet buffer,
      but also more ports, and the default configuration is also overcommitted.
      
      This patch disables the (apparently) bogus reservations and moves all
      resources to the shared area. This way, real reservations can be set up
      by the user, using devlink-sb.
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: NFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: NJakub Kicinski <kuba@kernel.org>
      a4ae997a
    • V
      net: mscc: ocelot: auto-detect packet buffer size and number of frame references · f6fe01d6
      Vladimir Oltean 提交于
      Instead of reading these values from the reference manual and writing
      them down into the driver, it appears that the hardware gives us the
      option of detecting them dynamically.
      
      The number of frame references corresponds to what the reference manual
      notes, however it seems that the frame buffers are reported as slightly
      less than the books would indicate. On VSC9959 (Felix), the books say it
      should have 128KB of packet buffer, but the registers indicate only
      129840 bytes (126.79 KB). Also, the unit of measurement for FREECNT from
      the documentation of all these devices is incorrect (taken from an older
      generation). This was confirmed by Younes Leroul from Microchip support.
      
      Not having anything better to do with these values at the moment* (this
      will change soon), let's just print them.
      
      *The frame buffer size is, in fact, used to calculate the tail dropping
      watermarks.
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: NFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: NJakub Kicinski <kuba@kernel.org>
      f6fe01d6
  9. 12 1月, 2021 1 次提交
    • V
      net: switchdev: remove the transaction structure from port attributes · bae33f2b
      Vladimir Oltean 提交于
      Since the introduction of the switchdev API, port attributes were
      transmitted to drivers for offloading using a two-step transactional
      model, with a prepare phase that was supposed to catch all errors, and a
      commit phase that was supposed to never fail.
      
      Some classes of failures can never be avoided, like hardware access, or
      memory allocation. In the latter case, merely attempting to move the
      memory allocation to the preparation phase makes it impossible to avoid
      memory leaks, since commit 91cf8ece ("switchdev: Remove unused
      transaction item queue") which has removed the unused mechanism of
      passing on the allocated memory between one phase and another.
      
      It is time we admit that separating the preparation from the commit
      phase is something that is best left for the driver to decide, and not
      something that should be baked into the API, especially since there are
      no switchdev callers that depend on this.
      
      This patch removes the struct switchdev_trans member from switchdev port
      attribute notifier structures, and converts drivers to not look at this
      member.
      
      In part, this patch contains a revert of my previous commit 2e554a7a
      ("net: dsa: propagate switchdev vlan_filtering prepare phase to
      drivers").
      
      For the most part, the conversion was trivial except for:
      - Rocker's world implementation based on Broadcom OF-DPA had an odd
        implementation of ofdpa_port_attr_bridge_flags_set. The conversion was
        done mechanically, by pasting the implementation twice, then only
        keeping the code that would get executed during prepare phase on top,
        then only keeping the code that gets executed during the commit phase
        on bottom, then simplifying the resulting code until this was obtained.
      - DSA's offloading of STP state, bridge flags, VLAN filtering and
        multicast router could be converted right away. But the ageing time
        could not, so a shim was introduced and this was left for a further
        commit.
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Acked-by: NLinus Walleij <linus.walleij@linaro.org>
      Acked-by: NJiri Pirko <jiri@nvidia.com>
      Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de> # hellcreek
      Reviewed-by: Linus Walleij <linus.walleij@linaro.org> # RTL8366RB
      Reviewed-by: NIdo Schimmel <idosch@nvidia.com>
      Reviewed-by: NFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: NJakub Kicinski <kuba@kernel.org>
      bae33f2b
  10. 15 12月, 2020 1 次提交
  11. 06 12月, 2020 1 次提交
    • V
      net: mscc: ocelot: fix dropping of unknown IPv4 multicast on Seville · edd2410b
      Vladimir Oltean 提交于
      The current assumption is that the felix DSA driver has flooding knobs
      per traffic class, while ocelot switchdev has a single flooding knob.
      This was correct for felix VSC9959 and ocelot VSC7514, but with the
      introduction of seville VSC9953, we see a switch driven by felix.c which
      has a single flooding knob.
      
      So it is clear that we must do what should have been done from the
      beginning, which is not to overwrite the configuration done by ocelot.c
      in felix, but instead to teach the common ocelot library about the
      differences in our switches, and set up the flooding PGIDs centrally.
      
      The effect that the bogus iteration through FELIX_NUM_TC has upon
      seville is quite dramatic. ANA_FLOODING is located at 0x00b548, and
      ANA_FLOODING_IPMC is located at 0x00b54c. So the bogus iteration will
      actually overwrite ANA_FLOODING_IPMC when attempting to write
      ANA_FLOODING[1]. There is no ANA_FLOODING[1] in sevile, just ANA_FLOODING.
      
      And when ANA_FLOODING_IPMC is overwritten with a bogus value, the effect
      is that ANA_FLOODING_IPMC gets the value of 0x0003CF7D:
      	MC6_DATA = 61,
      	MC6_CTRL = 61,
      	MC4_DATA = 60,
      	MC4_CTRL = 0.
      Because MC4_CTRL is zero, this means that IPv4 multicast control packets
      are not flooded, but dropped. An invalid configuration, and this is how
      the issue was actually spotted.
      Reported-by: NEldar Gasanov <eldargasanov2@gmail.com>
      Reported-by: NMaxim Kochetkov <fido_max@inbox.ru>
      Tested-by: NEldar Gasanov <eldargasanov2@gmail.com>
      Fixes: 84705fc1 ("net: dsa: felix: introduce support for Seville VSC9953 switch")
      Fixes: 3c7b51bd ("net: dsa: felix: allow flooding for all traffic classes")
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: NAlexandre Belloni <alexandre.belloni@bootlin.com>
      Link: https://lore.kernel.org/r/20201204175416.1445937-1-vladimir.oltean@nxp.comSigned-off-by: NJakub Kicinski <kuba@kernel.org>
      edd2410b
  12. 03 11月, 2020 6 次提交
    • V
      net: mscc: ocelot: deny changing the native VLAN from the prepare phase · 2f0402fe
      Vladimir Oltean 提交于
      Put the preparation phase of switchdev VLAN objects to some good use,
      and move the check we already had, for preventing the existence of more
      than one egress-untagged VLAN per port, to the preparation phase of the
      addition.
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: NJakub Kicinski <kuba@kernel.org>
      2f0402fe
    • V
      net: mscc: ocelot: move the logic to drop 802.1p traffic to the pvid deletion · be0576fe
      Vladimir Oltean 提交于
      Currently, the ocelot_port_set_native_vlan() function starts dropping
      untagged and prio-tagged traffic when the native VLAN is removed?
      
      What is the native VLAN? It is the only egress-untagged VLAN that ocelot
      supports on a port. If the port is a trunk with 100 VLANs, one of those
      VLANs can be transmitted as egress-untagged, and that's the native VLAN.
      
      Is it wrong to drop untagged and prio-tagged traffic if there's no
      native VLAN? Yes and no.
      
      In this case, which is more typical, it's ok to apply that drop
      configuration:
      $ bridge vlan add dev swp0 vid 1 pvid untagged <- this is the native VLAN
      $ bridge vlan add dev swp0 vid 100
      $ bridge vlan add dev swp0 vid 101
      $ bridge vlan del dev swp0 vid 1 <- delete the native VLAN
      But only because the pvid and the native VLAN have the same ID.
      
      In this case, it isn't:
      $ bridge vlan add dev swp0 vid 1 pvid
      $ bridge vlan add dev swp0 vid 100 untagged <- this is the native VLAN
      $ bridge vlan del dev swp0 vid 101
      $ bridge vlan del dev swp0 vid 100 <- delete the native VLAN
      
      It's wrong, because the switch will drop untagged and prio-tagged
      traffic now, despite having a valid pvid of 1.
      
      The confusion seems to stem from the fact that the native VLAN is an
      egress setting, while the PVID is an ingress setting. It would be
      correct to drop untagged and prio-tagged traffic only if there was no
      pvid on the port. So let's do just that.
      
      Background:
      https://lore.kernel.org/netdev/CA+h21hrRMrLH-RjBGhEJSTZd6_QPRSd3RkVRQF-wNKkrgKcRSA@mail.gmail.com/#tSigned-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: NJakub Kicinski <kuba@kernel.org>
      be0576fe
    • V
      net: mscc: ocelot: add a "valid" boolean to struct ocelot_vlan · e2b2e83e
      Vladimir Oltean 提交于
      Currently we are checking in some places whether the port has a native
      VLAN on egress or not, by comparing the ocelot_port->vid value with zero.
      
      That works, because VID 0 can never be a native VLAN configured by the
      bridge, but now we want to make similar checks for the pvid. That won't
      work, because there are cases when we do have the pvid set to 0 (not by
      the bridge, by ourselves, but still.. it's confusing). And we can't
      encode a negative value into an u16, so add a bool to the structure.
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: NJakub Kicinski <kuba@kernel.org>
      e2b2e83e
    • V
      net: mscc: ocelot: transform the pvid and native vlan values into a structure · c3e58a75
      Vladimir Oltean 提交于
      This is a mechanical patch only.
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: NJakub Kicinski <kuba@kernel.org>
      c3e58a75
    • V
      net: mscc: ocelot: don't reset the pvid to 0 when deleting it · 110e847c
      Vladimir Oltean 提交于
      I have no idea why this code is here, but I have 2 hypotheses:
      
      1.
      A desperate attempt to keep untagged traffic working when the bridge
      deletes the pvid on a port.
      
      There was a fairly okay discussion here:
      https://lore.kernel.org/netdev/CA+h21hrRMrLH-RjBGhEJSTZd6_QPRSd3RkVRQF-wNKkrgKcRSA@mail.gmail.com/#t
      which established that in vlan_filtering=1 mode, the absence of a pvid
      should denote that the ingress port should drop untagged and priority
      tagged traffic. While in vlan_filtering=0 mode, nothing should change.
      
      So in vlan_filtering=1 mode, we should simply let things happen, and not
      attempt to save the day. And in vlan_filtering=0 mode, the pvid is 0
      anyway, no need to do anything.
      
      2.
      The driver encodes the native VLAN (ocelot_port->vid) value of 0 as
      special, meaning "not valid". There are checks based on that. But there
      are no such checks for the ocelot_port->pvid value of 0. In fact, that's
      a perfectly valid value, which is used in standalone mode. Maybe there
      was some confusion and the author thought that 0 means "invalid" here as
      well.
      
      In conclusion, delete the code*.
      
      *in fact we'll add it back later, in a slightly different form, but for
      an entirely different reason than the one for which this exists now.
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: NJakub Kicinski <kuba@kernel.org>
      110e847c
    • V
      net: mscc: ocelot: use the pvid of zero when bridged with vlan_filtering=0 · 75e5a554
      Vladimir Oltean 提交于
      Currently, mscc_ocelot ports configure pvid=0 in standalone mode, and
      inherit the pvid from the bridge when one is present.
      
      When the bridge has vlan_filtering=0, the software semantics are that
      packets should be received regardless of whether there's a pvid
      configured on the ingress port or not. However, ocelot does not observe
      those semantics today.
      
      Moreover, changing the PVID is also a problem with vlan_filtering=0.
      We are privately remapping the VID of FDB, MDB entries to the port's
      PVID when those are VLAN-unaware (i.e. when the VID of these entries
      comes to us as 0). But we have no logic of adjusting that remapping when
      the user changes the pvid and vlan_filtering is 0. So stale entries
      would be left behind, and untagged traffic will stop matching on them.
      
      And even if we were to solve that, there's an even bigger problem. If
      swp0 has pvid 1, and swp1 has pvid 2, and both are under a vlan_filtering=0
      bridge, they should be able to forward traffic between one another.
      However, with ocelot they wouldn't do that.
      
      The simplest way of fixing this is to never configure the pvid based on
      what the bridge is asking for, when vlan_filtering is 0. Only if there
      was a VLAN that the bridge couldn't mangle, that we could use as pvid....
      So, turns out, there's 0 just for that. And for a reason: IEEE
      802.1Q-2018, page 247, Table 9-2-Reserved VID values says:
      
      	The null VID. Indicates that the tag header contains only
      	priority information; no VID is present in the frame.
      	This VID value shall not be configured as a PVID or a member
      	~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      	of a VID Set, or configured in any FDB entry, or used in any
      	Management operation.
      
      So, aren't we doing exactly what 802.1Q says not to? Well, in a way, but
      what we're doing here is just driver-level bookkeeping, all for the
      better. The fact that we're using a pvid of 0 is not observable behavior
      from the outside world: the network stack does not see the classified
      VLAN that the switch uses, in vlan_filtering=0 mode. And we're also more
      consistent with the standalone mode now.
      
      And now that we use the pvid of 0 in this mode, there's another advantage:
      we don't need to perform any VID remapping for FDB and MDB entries either,
      we can just use the VID of 0 that the bridge is passing to us.
      
      The only gotcha is that every time we change the vlan_filtering setting,
      we need to reapply the pvid (either to 0, or to the value from the bridge).
      A small side-effect visible in the patch is that ocelot_port_set_pvid
      needs to be moved above ocelot_port_vlan_filtering, so that it can be
      called from there without forward-declarations.
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: NJakub Kicinski <kuba@kernel.org>
      75e5a554
  13. 31 10月, 2020 5 次提交
  14. 12 10月, 2020 1 次提交
    • V
      net: mscc: ocelot: offload VLAN mangle action to VCAP IS1 · 70edfae1
      Vladimir Oltean 提交于
      The VCAP_IS1_ACT_VID_REPLACE_ENA action, from the VCAP IS1 ingress TCAM,
      changes the classified VLAN.
      
      We are only exposing this ability for switch ports that are under VLAN
      aware bridges. This is because in standalone ports mode and under a
      bridge with vlan_filtering=0, the ocelot driver configures the switch to
      operate as VLAN-unaware, so the classified VLAN is not derived from the
      802.1Q header from the packet, but instead is always equal to the
      port-based VLAN ID of the ingress port. We _can_ still change the
      classified VLAN for packets when operating in this mode, but the end
      result will most likely be a drop, since both the ingress and the egress
      port need to be members of the modified VLAN. And even if we install the
      new classified VLAN into the VLAN table of the switch, the result would
      still not be as expected: we wouldn't see, on the output port, the
      modified VLAN tag, but the original one, even though the classified VLAN
      was indeed modified. This is because of how the hardware works: on
      egress, what is pushed to the frame is a "port tag", which gives us the
      following options:
      
      - Tag all frames with port tag (derived from the classified VLAN)
      - Tag all frames with port tag, except if the classified VLAN is 0 or
        equal to the native VLAN of the egress port
      - No port tag
      
      Needless to say, in VLAN-unaware mode we are disabling the port tag.
      Otherwise, the existing VLAN tag would be ignored, and a second VLAN
      tag (the port tag), holding the classified VLAN, would be pushed
      (instead of replacing the existing 802.1Q tag). This is definitely not
      what the user wanted when installing a "vlan modify" action.
      
      So it is simply not worth bothering with VLAN modify rules under other
      configurations except when the ports are fully VLAN-aware.
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: NJakub Kicinski <kuba@kernel.org>
      70edfae1
  15. 06 10月, 2020 1 次提交
  16. 05 10月, 2020 1 次提交
    • V
      net: dsa: propagate switchdev vlan_filtering prepare phase to drivers · 2e554a7a
      Vladimir Oltean 提交于
      A driver may refuse to enable VLAN filtering for any reason beyond what
      the DSA framework cares about, such as:
      - having tc-flower rules that rely on the switch being VLAN-aware
      - the particular switch does not support VLAN, even if the driver does
        (the DSA framework just checks for the presence of the .port_vlan_add
        and .port_vlan_del pointers)
      - simply not supporting this configuration to be toggled at runtime
      
      Currently, when a driver rejects a configuration it cannot support, it
      does this from the commit phase, which triggers various warnings in
      switchdev.
      
      So propagate the prepare phase to drivers, to give them the ability to
      refuse invalid configurations cleanly and avoid the warnings.
      
      Since we need to modify all function prototypes and check for the
      prepare phase from within the drivers, take that opportunity and move
      the existing driver restrictions within the prepare phase where that is
      possible and easy.
      
      Cc: Florian Fainelli <f.fainelli@gmail.com>
      Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
      Cc: Hauke Mehrtens <hauke@hauke-m.de>
      Cc: Woojung Huh <woojung.huh@microchip.com>
      Cc: Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
      Cc: Sean Wang <sean.wang@mediatek.com>
      Cc: Landen Chao <Landen.Chao@mediatek.com>
      Cc: Andrew Lunn <andrew@lunn.ch>
      Cc: Vivien Didelot <vivien.didelot@gmail.com>
      Cc: Jonathan McDowell <noodles@earth.li>
      Cc: Linus Walleij <linus.walleij@linaro.org>
      Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
      Cc: Claudiu Manoil <claudiu.manoil@nxp.com>
      Signed-off-by: NVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      2e554a7a