1. 14 Dec, 2021 8 commits
  2. 13 Dec, 2021 23 commits
  3. 12 Dec, 2021 9 commits
    • David S. Miller's avatar
      Merge branch 'dsa-tagger-storage' · 9b5bcb19
      David S. Miller authored
      Vladimir Oltean says:
      
      ====================
      Replace DSA dp->priv with tagger-owned storage
      
      Ansuel's recent work on qca8k register access over Ethernet:
      https://patchwork.kernel.org/project/netdevbpf/cover/20211207145942.7444-1-ansuelsmth@gmail.com/
      has triggered me to do something which I should've done for a longer
      time:
      https://patchwork.kernel.org/project/netdevbpf/patch/20211109095013.27829-7-martin.kaistra@linutronix.de/#24585521
      which is to replace dp->priv with something that has less caveats.
      
      The dp->priv was introduced when sja1105 needed to hold stateful
      information in the tagging protocol driver. In that design, dp->priv
      held memory allocated by the switch driver, because the tagging protocol
      driver design was 100% stateless.
      
      Some years have passed and others have started to feel the need for
      stateful information kept by the tagger, as well as passing data back
      and forth between the tagging protocol driver and the switch driver.
      This isn't possible cleanly in DSA due to a circular dependency which
      leads to broken module autoloading:
      https://lore.kernel.org/netdev/20210908220834.d7gmtnwrorhharna@skbuf/
      
      This patchset introduces a framework that resembles something normal,
      which allows data to be passed from the tagging protocol driver (things
      like switch management packets, which aren't intended for the network
      stack) to the switch driver, while the tagging protocol still remains
      more or less stateless. The overall design of the framework was
      discussed with Ansuel too and it appears to be flexible enough to cover
      the "register access over Ethernet" use case. Additionally, the existing
      uses of dp->priv, which have mainly to do with PTP timestamping, have
      also been migrated.
      
      Changes in v2:
      Fix transient build breakage in patch 5/11 due to a missing parenthesis,
      https://patchwork.hopto.org/static/nipa/592567/12665213/build_clang/
      and another transient build warning in patch 4/11 that for some reason
      doesn't appear in my W=1 C=1 build.
      https://patchwork.hopto.org/static/nipa/592567/12665209/build_clang/stderr
      ====================
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      9b5bcb19
    • Vladimir Oltean's avatar
      net: dsa: remove dp->priv · 4f3cb343
      Vladimir Oltean authored
      All current in-tree uses of dp->priv have been replaced with
      ds->tagger_data, which provides for a safer API especially when the
      connection isn't the regular 1:1 link between one switch driver and one
      tagging protocol driver, but could be either one switch to many taggers,
      or many switches to one tagger.
      
      Therefore, we can remove this unused pointer.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      4f3cb343
    • Vladimir Oltean's avatar
      net: dsa: tag_sja1105: split sja1105_tagger_data into private and public sections · 950a419d
      Vladimir Oltean authored
      The sja1105 driver messes with the tagging protocol's state when PTP RX
      timestamping is enabled/disabled. This is fundamentally necessary
      because the tagger needs to know what to do when it receives a PTP
      packet. If RX timestamping is enabled, then a metadata follow-up frame
      is expected, and this holds the (partial) timestamp. So the tagger plays
      hide-and-seek with the network stack until it also gets the metadata
      frame, and then presents a single packet, the timestamped PTP packet.
      But when RX timestamping isn't enabled, there is no metadata frame
      expected, so the hide-and-seek game must be turned off and the packet
      must be delivered right away to the network stack.
      
      Considering this, we create a pseudo isolation by devising two tagger
      methods callable by the switch: one to get the RX timestamping state,
      and one to set it. Since we can't export symbols between the tagger and
      the switch driver, these methods are exposed through function pointers.
      
      After this change, the public portion of the sja1105_tagger_data
      contains only function pointers.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      950a419d
    • Vladimir Oltean's avatar
      Revert "net: dsa: move sja1110_process_meta_tstamp inside the tagging protocol driver" · fcbf979a
      Vladimir Oltean authored
      This reverts commit 6d709cad.
      
      The above change was done to avoid calling symbols exported by the
      switch driver from the tagging protocol driver.
      
      With the tagger-owned storage model, we have a new option on our hands,
      and that is for the switch driver to provide a data consumer handler in
      the form of a function pointer inside the ->connect_tag_protocol()
      method. Having a function pointer avoids the problems of the exported
      symbols approach.
      
      By creating a handler for metadata frames holding TX timestamps on
      SJA1110, we are able to eliminate an skb queue from the tagger data, and
      replace it with a simple, and stateless, function pointer. This skb
      queue is now handled exclusively by sja1105_ptp.c, which makes the code
      easier to follow, as it used to be before the reverted patch.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      fcbf979a
    • Vladimir Oltean's avatar
      net: dsa: tag_sja1105: convert to tagger-owned data · c79e8486
      Vladimir Oltean authored
      Currently, struct sja1105_tagger_data is a part of struct
      sja1105_private, and is used by the sja1105 driver to populate dp->priv.
      
      With the movement towards tagger-owned storage, the sja1105 driver
      should not be the owner of this memory.
      
      This change implements the connection between the sja1105 switch driver
      and its tagging protocol, which means that sja1105_tagger_data no longer
      stays in dp->priv but in ds->tagger_data, and that the sja1105 driver
      now only populates the sja1105_port_deferred_xmit callback pointer.
      The kthread worker is now the responsibility of the tagger.
      
      The sja1105 driver also alters the tagger's state some more, especially
      with regard to the PTP RX timestamping state. This will be fixed up a
      bit in further changes.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      c79e8486
    • Vladimir Oltean's avatar
      net: dsa: sja1105: move ts_id from sja1105_tagger_data · 22ee9f8e
      Vladimir Oltean authored
      The TX timestamp ID is incremented by the SJA1110 PTP timestamping
      callback (->port_tx_timestamp) for every packet, when cloning it.
      It isn't used by the tagger at all, even though it sits inside the
      struct sja1105_tagger_data.
      
      Also, serialization to this structure is currently done through
      tagger_data->meta_lock, which is a cheap hack because the meta_lock
      isn't used for anything else on SJA1110 (sja1105_rcv_meta_state_machine
      isn't called).
      
      This change moves ts_id from sja1105_tagger_data to sja1105_private and
      introduces a dedicated spinlock for it, also in sja1105_private.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      22ee9f8e
    • Vladimir Oltean's avatar
      net: dsa: sja1105: make dp->priv point directly to sja1105_tagger_data · bfcf1425
      Vladimir Oltean authored
      The design of the sja1105 tagger dp->priv is that each port has a
      separate struct sja1105_port, and the sp->data pointer points to a
      common struct sja1105_tagger_data.
      
      We have removed all per-port members accessible by the tagger, and now
      only struct sja1105_tagger_data remains. Make dp->priv point directly to
      this.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      bfcf1425
    • Vladimir Oltean's avatar
      net: dsa: sja1105: remove hwts_tx_en from tagger data · 6f6770ab
      Vladimir Oltean authored
      This tagger property is in fact not used at all by the tagger, only by
      the switch driver. Therefore it makes sense to be moved to
      sja1105_private.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      6f6770ab
    • Vladimir Oltean's avatar
      net: dsa: sja1105: bring deferred xmit implementation in line with ocelot-8021q · d38049bb
      Vladimir Oltean authored
      When the ocelot-8021q driver was converted to deferred xmit as part of
      commit 8d5f7954 ("net: dsa: felix: break at first CPU port during
      init and teardown"), the deferred implementation was deliberately made
      subtly different from what sja1105 has.
      
      The implementation differences lied on the following observations:
      
      - There might be a race between these two lines in tag_sja1105.c:
      
             skb_queue_tail(&sp->xmit_queue, skb_get(skb));
             kthread_queue_work(sp->xmit_worker, &sp->xmit_work);
      
        and the skb dequeue logic in sja1105_port_deferred_xmit(). For
        example, the xmit_work might be already queued, however the work item
        has just finished walking through the skb queue. Because we don't
        check the return code from kthread_queue_work, we don't do anything if
        the work item is already queued.
      
        However, nobody will take that skb and send it, at least until the
        next timestampable skb is sent. This creates additional (and
        avoidable) TX timestamping latency.
      
        To close that race, what the ocelot-8021q driver does is it doesn't
        keep a single work item per port, and a skb timestamping queue, but
        rather dynamically allocates a work item per packet.
      
      - It is also unnecessary to have more than one kthread that does the
        work. So delete the per-port kthread allocations and replace them with
        a single kthread which is global to the switch.
      
      This change brings the two implementations in line by applying those
      observations to the sja1105 driver as well.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      d38049bb