1. 14 2月, 2014 14 次提交
    • Y
      tipc: remove bearer_lock from tipc_bearer struct · a8304529
      Ying Xue 提交于
      After the earlier commits ("tipc: remove 'links' list from
      tipc_bearer struct") and ("tipc: introduce new spinlock to protect
      struct link_req"), there is no longer any need to protect struct
      link_req or or any link list by use of bearer_lock. Furthermore,
      we have eliminated the need for using bearer_lock during downcalls
      (send) from the link to the bearer, since we have ensured that
      bearers always have a longer life cycle that their associated links,
      and always contain valid data.
      
      So, the only need now for a lock protecting bearers is for guaranteeing
      consistency of the bearer list itself. For this, it is sufficient, at
      least for the time being, to continue applying 'net_lock´ in write mode.
      
      By removing bearer_lock we also pre-empt introduction of issue b) descibed
      in the previous commit "tipc: remove 'links' list from tipc_bearer struct":
      
      "b) When the outer protection from net_lock is gone, taking
          bearer_lock and node_lock in opposite order of method 1) and 2)
          will become an obvious deadlock hazard".
      
      Therefore, we now eliminate the bearer_lock spinlock.
      Signed-off-by: NYing Xue <ying.xue@windriver.com>
      Reviewed-by: NPaul Gortmaker <paul.gortmaker@windriver.com>
      Signed-off-by: NJon Maloy <jon.maloy@ericsson.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      a8304529
    • J
      tipc: delay delete of link when failover is needed · 7d33939f
      Jon Paul Maloy 提交于
      When a bearer is disabled, all its attached links are deleted.
      Ideally, we should do link failover to redundant links on other bearers,
      if there are any, in such cases. This would be consistent with current
      behavior when a link is reset, but not deleted. However, due to the
      complexity involved, and the (wrongly) perceived low demand for this
      feature, it was never implemented until now.
      
      We mark the doomed link for deletion with a new flag, but wait until the
      failover process is finished before we actually delete it. With the
      improved link tunnelling/failover code introduced earlier in this commit
      series, it is now easy to identify a spot in the code where the failover
      is finished and it is safe to delete the marked link. Moreover, the test
      for the flag and the deletion can be done synchronously, and outside the
      most time critical data path.
      Signed-off-by: NJon Maloy <jon.maloy@ericsson.com>
      Reviewed-by: NYing Xue <ying.xue@windriver.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      7d33939f
    • J
      tipc: changes to general packet reception algorithm · a5377831
      Jon Paul Maloy 提交于
      We change the order of checking for destination users when processing
      incoming packets. By placing the checks for users that may potentially
      replace the processed buffer, i.e., CHANGEOVER_PROTOCOL and
      MSG_FRAGMENTER, in a separate step before we check for the true end
      users, we get rid of a label and a 'goto', at the same time making the
      code more comprehensible and easy to follow.
      
      This commit does not change any functionality, it is just a cosmetic
      code reshuffle.
      Signed-off-by: NJon Maloy <jon.maloy@ericsson.com>
      Reviewed-by: NYing Xue <ying.xue@windriver.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      a5377831
    • J
      tipc: rename stack variables in function tipc_link_tunnel_rcv · 02842f71
      Jon Paul Maloy 提交于
      After the previous redesign of the tunnel reception algorithm and
      functions, we finalize it by renaming a couple of stack variables
      in tipc_tunnel_rcv(). This makes it more consistent with the naming
      scheme elsewhere in this part of the code.
      
      This change is purely cosmetic, with no functional changes.
      Signed-off-by: NJon Maloy <jon.maloy@ericsson.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      02842f71
    • J
      tipc: more cleanup of tunnelling reception function · 1e9d47a9
      Jon Paul Maloy 提交于
      We simplify and slim down the code in function tipc_tunnel_rcv()
      No impact on the users of this function.
      Signed-off-by: NJon Maloy <jon.maloy@ericsson.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      1e9d47a9
    • J
      tipc: change signature of tunnelling reception function · 3bb53380
      Jon Paul Maloy 提交于
      After the earlier commits in this series related to the function
      tipc_link_tunnel_rcv(), we can now go further and simplify its
      signature.
      
      The function now consumes all DUPLICATE packets, and only returns such
      ORIGINAL packets that are ready for immediate delivery, i.e., no
      more link level protocol processing needs to be done by the caller.
      As a consequence, the the caller, tipc_rcv(), does not access the link
      pointer after call return, and it becomes unnecessary to pass a link
      pointer reference in the call. Instead, we now only pass it the tunnel
      link's owner node, which is sufficient to find the destination link for
      the tunnelled packet.
      Signed-off-by: NJon Maloy <jon.maloy@ericsson.com>
      Reviewed-by: NYing Xue <ying.xue@windriver.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      3bb53380
    • J
      tipc: change reception of tunnelled failover packets · f006c9c7
      Jon Paul Maloy 提交于
      When a link is reset, and there is a redundant link available, all
      sender sockets will steer their subsequent traffic through the
      remaining link. In order to guarantee preserved packet order and
      cardinality during the transition, we tunnel the failing link's send
      queue through the remaining link before we allow any sockets to use it.
      
      In this commit, we change the algorithm for receiving failover
      ("ORIGINAL_MSG") packets in tipc_link_tunnel_rcv(), at the same time
      delegating it to a new subfuncton, tipc_link_failover_rcv(). Instead
      of directly returning an extracted inner packet to the packet reception
      loop in tipc_rcv(), we first check if it is a message fragment, in which
      case we append it to the reset link's fragment chain. If the fragment
      chain is complete, we return the whole chain instead of the individual
      buffer, eliminating any need for the tipc_rcv() loop to do reassembly of
      tunneled packets.
      
      This change makes it possible to further simplify tipc_link_tunnel_rcv(),
      as well as the calling tipc_rcv() loop. We will do that in later
      commits. It also makes it possible to identify a single spot in the code
      where we can tell that a failover procedure is finished, something that
      is useful when we are deleting links after a failover. This will also
      be done in a later commit.
      Signed-off-by: NJon Maloy <jon.maloy@ericsson.com>
      Reviewed-by: NYing Xue <ying.xue@windriver.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      f006c9c7
    • J
      tipc: change reception of tunnelled duplicate packets · 1dab3d5a
      Jon Paul Maloy 提交于
      When a second link to a destination comes up, some sender sockets will
      steer their subsequent traffic through the new link. In order to
      guarantee preserved packet order and cardinality for those sockets, we
      tunnel a duplicate of the old link's send queue through the new link
      before we open it for regular traffic. The last arriving packet copy,
      on whichever link, will be dropped at the receiving end based on the
      original sequence number, to ensure that only one copy is delivered to
      the end receiver.
      
      In this commit, we change the algorithm for receiving DUPLICATE_MSG
      packets, at the same time delegating it to a new subfunction,
      tipc_link_dup_rcv(). Instead of returning an extracted inner packet to
      the packet reception loop in tipc_rcv(), we just add it to the receiving
      (new) link's deferred packet queue. The packet will then be processed by
      that link when it receives its first non-tunneled packet, i.e., at
      latest when the changeover procedure is finished.
      
      Because tipc_link_tunnel_rcv()/tipc_link_dup_rcv() now is consuming all
      packets of type DUPLICATE_MSG, the calling tipc_rcv() function can omit
      testing for this. This in turn means that the current conditional jump
      to the label 'protocol_check' becomes redundant, and we can remove that
      label.
      Signed-off-by: NJon Maloy <jon.maloy@ericsson.com>
      Signed-off-by: NYing Xue <ying.xue@windriver.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      1dab3d5a
    • Y
      tipc: remove 'links' list from tipc_bearer struct · c61dd61d
      Ying Xue 提交于
      In our ongoing effort to simplify the TIPC locking structure,
      we see a need to remove the linked list for tipc_links
      in the bearer. This can be explained as follows.
      
      Currently, we have three different ways to access a link,
      via three different lists/tables:
      
      1: Via a node hash table:
         Used by the time-critical outgoing/incoming data paths.
         (e.g. link_send_sections_fast() and tipc_recv_msg() ):
      
      grab net_lock(read)
         find node from node hash table
         grab node_lock
             select link
             grab bearer_lock
                send_msg()
             release bearer_lock
         release node lock
      release net_lock
      
      2: Via a global linked list for nodes:
         Used by configuration commands (link_cmd_set_value())
      
      grab net_lock(read)
         find node and link from global node list (using link name)
         grab node_lock
             update link
         release node lock
      release net_lock
      
      (Same locking order as above. No problem.)
      
      3: Via the bearer's linked link list:
         Used by notifications from interface (e.g. tipc_disable_bearer() )
      
      grab net_lock(write)
         grab bearer_lock
            get link ptr from bearer's link list
            get node from link
            grab node_lock
               delete link
            release node lock
         release bearer_lock
      release net_lock
      
      (Different order from above, but works because we grab the
      outer net_lock in write mode first, excluding all other access.)
      
      The first major goal in our simplification effort is to get rid
      of the "big" net_lock, replacing it with rcu-locks when accessing
      the node list and node hash array. This will come in a later patch
      series.
      
      But to get there we first need to rewrite access methods ##2 and 3,
      since removal of net_lock would introduce three major problems:
      
      a) In access method #2, we access the link before taking the
         protecting node_lock. This will not work once net_lock is gone,
         so we will have to change the access order. We will deal with
         this in a later commit in this series, "tipc: add node lock
         protection to link found by link_find_link()".
      
      b) When the outer protection from net_lock is gone, taking
         bearer_lock and node_lock in opposite order of method 1) and 2)
         will become an obvious deadlock hazard. This is fixed in the
         commit ("tipc: remove bearer_lock from tipc_bearer struct")
         later in this series.
      
      c) Similar to what is described in problem a), access method #3
         starts with using a link pointer that is unprotected by node_lock,
         in order to via that pointer find the correct node struct and
         lock it. Before we remove net_lock, this access order must be
         altered. This is what we do with this commit.
      
      We can avoid introducing problem problem c) by even here using the
      global node list to find the node, before accessing its links. When
      we loop though the node list we use the own bearer identity as search
      criteria, thus easily finding the links that are associated to the
      resetting/disabling bearer. It should be noted that although this
      method is somewhat slower than the current list traversal, it is in
      no way time critical. This is only about resetting or deleting links,
      something that must be considered relatively infrequent events.
      
      As a bonus, we can get rid of the mutual pointers between links and
      bearers. After this commit, pointer dependency go in one direction
      only: from the link to the bearer.
      
      This commit pre-empts introduction of problem c) as described above.
      Signed-off-by: NYing Xue <ying.xue@windriver.com>
      Reviewed-by: NPaul Gortmaker <paul.gortmaker@windriver.com>
      Signed-off-by: NJon Maloy <jon.maloy@ericsson.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      c61dd61d
    • Y
      tipc: redefine 'started' flag in struct link to bitmap · 135daee6
      Ying Xue 提交于
      Currently, the 'started' field in struct tipc_link represents only a
      binary state, 'started' or 'not started'. We need it to represent
      more link execution states in the coming commits in this series.
      Hence, we rename the field to 'flags', and define the current
      started/non-started state to be represented by the LSB bit of
      that field.
      Signed-off-by: NYing Xue <ying.xue@windriver.com>
      Reviewed-by: NPaul Gortmaker <paul.gortmaker@windriver.com>
      Signed-off-by: NJon Maloy <jon.maloy@ericsson.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      135daee6
    • Y
      tipc: move code for deleting links from bearer.c to link.c · 8d8439b6
      Ying Xue 提交于
      We break out the code for deleting attached links in the
      function bearer_disable(), and define a new function named
      tipc_link_delete_list() to do this job.
      
      This commit incurs no functional changes, but makes the code of
      function bearer_disable() cleaner. It is also a preparation
      for a more important change to the bearer code, in a subsequent
      commit in this series.
      Signed-off-by: NYing Xue <ying.xue@windriver.com>
      Reviewed-by: NPaul Gortmaker <paul.gortmaker@windriver.com>
      Signed-off-by: NJon Maloy <jon.maloy@ericsson.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      8d8439b6
    • Y
      tipc: move code for resetting links from bearer.c to link.c · e0ca2c30
      Ying Xue 提交于
      We break out the code for resetting attached links in the
      function tipc_reset_bearer(), and define a new function named
      tipc_link_reset_list() to do this job.
      
      This commit incurs no functional changes, but makes the code
      of function tipc_reset_bearer() cleaner. It is also a preparation
      for a more important change to the bearer code, in a subsequent
      commit in this series.
      Signed-off-by: NYing Xue <ying.xue@windriver.com>
      Reviewed-by: NPaul Gortmaker <paul.gortmaker@windriver.com>
      Signed-off-by: NJon Maloy <jon.maloy@ericsson.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      e0ca2c30
    • J
      tipc: stricter behavior of message reassembly function · 03b92017
      Jon Paul Maloy 提交于
      The function tipc_link_recv_fragment(struct sk_buff **buf) currently
      leaves the value of the input buffer pointer undefined when it returns,
      except when the return code indicates that the reassembly is complete.
      This despite the fact that it always consumes the input buffer.
      
      Here, we enforce a stricter behavior by this function, ensuring that
      the returned buffer pointer is non-NULL if and only if the reassembly
      is complete. This makes it possible to test for the buffer pointer as
      criteria for successful reassembly.
      
      We also rename the function to tipc_link_frag_rcv(), which is both
      shorter and more in line with common naming practice in the network
      subsystem.
      
      Apart from the new name, these changes have no impact on current
      users of the function, but makes it more practical for use in some
      planned future commits.
      Signed-off-by: NJon Maloy <jon.maloy@ericsson.com>
      Reviewed-by: NYing Xue <ying.xue@windriver.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      03b92017
    • A
      tipc: explicitly include core.h in addr.h · b3f0f5c3
      Andreas Bofjäll 提交于
      The inline functions in addr.h uses tipc_own_addr which is exported by
      core.h, but addr.h never actually includes it. It works because it is
      explicitly included where this is used, but it looks a bit strange.
      
      Include core.h in addr.h explicitly to make the dependency clearer.
      Signed-off-by: NAndreas Bofjäll <andreas.bofjall@ericsson.com>
      Reviewed-by: NJon Maloy <jon.maloy@ericsson.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      b3f0f5c3
  2. 13 2月, 2014 6 次提交
  3. 11 2月, 2014 12 次提交
    • E
      6lowpan: fix lockdep splats · 20e7c4e8
      Eric Dumazet 提交于
      When a device ndo_start_xmit() calls again dev_queue_xmit(),
      lockdep can complain because dev_queue_xmit() is re-entered and the
      spinlocks protecting tx queues share a common lockdep class.
      
      Same issue was fixed for bonding/l2tp/ppp in commits
      
      0daa2303 ("[PATCH] bonding: lockdep annotation")
      49ee4920 ("bonding: set qdisc_tx_busylock to avoid LOCKDEP splat")
      23d3b8bf ("net: qdisc busylock needs lockdep annotations ")
      303c07db ("ppp: set qdisc_tx_busylock to avoid LOCKDEP splat ")
      Reported-by: NAlexander Aring <alex.aring@gmail.com>
      Signed-off-by: NEric Dumazet <edumazet@google.com>
      Tested-by: NAlexander Aring <alex.aring@gmail.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      20e7c4e8
    • R
      9p/trans_virtio.c: Fix broken zero-copy on vmalloc() buffers · b6f52ae2
      Richard Yao 提交于
      The 9p-virtio transport does zero copy on things larger than 1024 bytes
      in size. It accomplishes this by returning the physical addresses of
      pages to the virtio-pci device. At present, the translation is usually a
      bit shift.
      
      That approach produces an invalid page address when we read/write to
      vmalloc buffers, such as those used for Linux kernel modules. Any
      attempt to load a Linux kernel module from 9p-virtio produces the
      following stack.
      
      [<ffffffff814878ce>] p9_virtio_zc_request+0x45e/0x510
      [<ffffffff814814ed>] p9_client_zc_rpc.constprop.16+0xfd/0x4f0
      [<ffffffff814839dd>] p9_client_read+0x15d/0x240
      [<ffffffff811c8440>] v9fs_fid_readn+0x50/0xa0
      [<ffffffff811c84a0>] v9fs_file_readn+0x10/0x20
      [<ffffffff811c84e7>] v9fs_file_read+0x37/0x70
      [<ffffffff8114e3fb>] vfs_read+0x9b/0x160
      [<ffffffff81153571>] kernel_read+0x41/0x60
      [<ffffffff810c83ab>] copy_module_from_fd.isra.34+0xfb/0x180
      
      Subsequently, QEMU will die printing:
      
      qemu-system-x86_64: virtio: trying to map MMIO memory
      
      This patch enables 9p-virtio to correctly handle this case. This not
      only enables us to load Linux kernel modules off virtfs, but also
      enables ZFS file-based vdevs on virtfs to be used without killing QEMU.
      
      Special thanks to both Avi Kivity and Alexander Graf for their
      interpretation of QEMU backtraces. Without their guidence, tracking down
      this bug would have taken much longer. Also, special thanks to Linus
      Torvalds for his insightful explanation of why this should use
      is_vmalloc_addr() instead of is_vmalloc_or_module_addr():
      
      https://lkml.org/lkml/2014/2/8/272Signed-off-by: NRichard Yao <ryao@gentoo.org>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      b6f52ae2
    • J
      tcp: tsq: fix nonagle handling · bf06200e
      John Ogness 提交于
      Commit 46d3ceab ("tcp: TCP Small Queues") introduced a possible
      regression for applications using TCP_NODELAY.
      
      If TCP session is throttled because of tsq, we should consult
      tp->nonagle when TX completion is done and allow us to send additional
      segment, especially if this segment is not a full MSS.
      Otherwise this segment is sent after an RTO.
      
      [edumazet] : Cooked the changelog, added another fix about testing
      sk_wmem_alloc twice because TX completion can happen right before
      setting TSQ_THROTTLED bit.
      
      This problem is particularly visible with recent auto corking,
      but might also be triggered with low tcp_limit_output_bytes
      values or NIC drivers delaying TX completion by hundred of usec,
      and very low rtt.
      
      Thomas Glanzmann for example reported an iscsi regression, caused
      by tcp auto corking making this bug quite visible.
      
      Fixes: 46d3ceab ("tcp: TCP Small Queues")
      Signed-off-by: NJohn Ogness <john.ogness@linutronix.de>
      Signed-off-by: NEric Dumazet <edumazet@google.com>
      Reported-by: NThomas Glanzmann <thomas@glanzmann.de>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      bf06200e
    • T
      bridge: Prevent possible race condition in br_fdb_change_mac_address · ac4c8868
      Toshiaki Makita 提交于
      br_fdb_change_mac_address() calls fdb_insert()/fdb_delete() without
      br->hash_lock.
      
      These hash list updates are racy with br_fdb_update()/br_fdb_cleanup().
      Signed-off-by: NToshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
      Acked-by: NVlad Yasevich <vyasevic@redhat.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      ac4c8868
    • T
      bridge: Properly check if local fdb entry can be deleted when deleting vlan · 424bb9c9
      Toshiaki Makita 提交于
      Vlan codes unconditionally delete local fdb entries.
      We should consider the possibility that other ports have the same
      address and vlan.
      
      Example of problematic case:
        ip link set eth0 address 12:34:56:78:90:ab
        ip link set eth1 address aa:bb:cc:dd:ee:ff
        brctl addif br0 eth0
        brctl addif br0 eth1 # br0 will have mac address 12:34:56:78:90:ab
        bridge vlan add dev eth0 vid 10
        bridge vlan add dev eth1 vid 10
        bridge vlan add dev br0 vid 10 self
      We will have fdb entry such that f->dst == eth0, f->vlan_id == 10 and
      f->addr == 12:34:56:78:90:ab at this time.
      Next, delete eth0 vlan 10.
        bridge vlan del dev eth0 vid 10
      In this case, we still need the entry for br0, but it will be deleted.
      
      Note that br0 needs the entry even though its mac address is not set
      manually. To delete the entry with proper condition checking,
      fdb_delete_local() is suitable to use.
      Signed-off-by: NToshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
      Acked-by: NVlad Yasevich <vyasevic@redhat.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      424bb9c9
    • T
      bridge: Properly check if local fdb entry can be deleted in br_fdb_delete_by_port · a778e6d1
      Toshiaki Makita 提交于
      br_fdb_delete_by_port() doesn't care about vlan and mac address of the
      bridge device.
      
      As the check is almost the same as mac address changing, slightly modify
      fdb_delete_local() and use it.
      
      Note that we can always set added_by_user to 0 in fdb_delete_local() because
      - br_fdb_delete_by_port() calls fdb_delete_local() for local entries
        regardless of its added_by_user. In this case, we have to check if another
        port has the same address and vlan, and if found, we have to create the
        entry (by changing dst). This is kernel-added entry, not user-added.
      - br_fdb_changeaddr() doesn't call fdb_delete_local() for user-added entry.
      Signed-off-by: NToshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
      Acked-by: NVlad Yasevich <vyasevic@redhat.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      a778e6d1
    • T
      bridge: Properly check if local fdb entry can be deleted in br_fdb_change_mac_address · 960b589f
      Toshiaki Makita 提交于
      br_fdb_change_mac_address() doesn't check if the local entry has the
      same address as any of bridge ports.
      Although I'm not sure when it is beneficial, current implementation allow
      the bridge device to receive any mac address of its ports.
      To preserve this behavior, we have to check if the mac address of the
      entry being deleted is identical to that of any port.
      
      As this check is almost the same as that in br_fdb_changeaddr(), create
      a common function fdb_delete_local() and call it from
      br_fdb_changeadddr() and br_fdb_change_mac_address().
      Signed-off-by: NToshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
      Acked-by: NVlad Yasevich <vyasevic@redhat.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      960b589f
    • T
      bridge: Fix the way to check if a local fdb entry can be deleted · 2b292fb4
      Toshiaki Makita 提交于
      We should take into account the followings when deleting a local fdb
      entry.
      
      - nbp_vlan_find() can be used only when vid != 0 to check if an entry is
        deletable, because a fdb entry with vid 0 can exist at any time while
        nbp_vlan_find() always return false with vid 0.
      
        Example of problematic case:
          ip link set eth0 address 12:34:56:78:90:ab
          ip link set eth1 address 12:34:56:78:90:ab
          brctl addif br0 eth0
          brctl addif br0 eth1
          ip link set eth0 address aa:bb:cc:dd:ee:ff
        Then, the fdb entry 12:34:56:78:90:ab will be deleted even though the
        bridge port eth1 still has that address.
      
      - The port to which the bridge device is attached might needs a local entry
        if its mac address is set manually.
      
        Example of problematic case:
          ip link set eth0 address 12:34:56:78:90:ab
          brctl addif br0 eth0
          ip link set br0 address 12:34:56:78:90:ab
          ip link set eth0 address aa:bb:cc:dd:ee:ff
        Then, the fdb still must have the entry 12:34:56:78:90:ab, but it will be
        deleted.
      
      We can use br->dev->addr_assign_type to check if the address is manually
      set or not, but I propose another approach.
      
      Since we delete and insert local entries whenever changing mac address
      of the bridge device, we can change dst of the entry to NULL regardless of
      addr_assign_type when deleting an entry associated with a certain port,
      and if it is found to be unnecessary later, then delete it.
      That is, if changing mac address of a port, the entry might be changed
      to its dst being NULL first, but is eventually deleted when recalculating
      and changing bridge id.
      
      This approach is especially useful when we want to share the code with
      deleting vlan in which the bridge device might want such an entry regardless
      of addr_assign_type, and makes things easy because we don't have to consider
      if mac address of the bridge device will be changed or not at the time we
      delete a local entry of a port, which means fdb code will not be bothered
      even if the bridge id calculating logic is changed in the future.
      
      Also, this change reduces inconsistent state, where frames whose dst is the
      mac address of the bridge, can't reach the bridge because of premature fdb
      entry deletion. This change reduces the possibility that the bridge device
      replies unreachable mac address to arp requests, which could occur during
      the short window between calling del_nbp() and br_stp_recalculate_bridge_id()
      in br_del_if(). This will effective after br_fdb_delete_by_port() starts to
      use the same code by following patch.
      Signed-off-by: NToshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
      Acked-by: NVlad Yasevich <vyasevic@redhat.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      2b292fb4
    • T
      bridge: Change local fdb entries whenever mac address of bridge device changes · a4b816d8
      Toshiaki Makita 提交于
      Vlan code may need fdb change when changing mac address of bridge device
      even if it is caused by the mac address changing of a bridge port.
      
      Example configuration:
        ip link set eth0 address 12:34:56:78:90:ab
        ip link set eth1 address aa:bb:cc:dd:ee:ff
        brctl addif br0 eth0
        brctl addif br0 eth1 # br0 will have mac address 12:34:56:78:90:ab
        bridge vlan add dev br0 vid 10 self
        bridge vlan add dev eth0 vid 10
      We will have fdb entry such that f->dst == NULL, f->vlan_id == 10 and
      f->addr == 12:34:56:78:90:ab at this time.
      Next, change the mac address of eth0 to greater value.
        ip link set eth0 address ee:ff:12:34:56:78
      Then, mac address of br0 will be recalculated and set to aa:bb:cc:dd:ee:ff.
      However, an entry aa:bb:cc:dd:ee:ff will not be created and we will be not
      able to communicate using br0 on vlan 10.
      
      Address this issue by deleting and adding local entries whenever
      changing the mac address of the bridge device.
      
      If there already exists an entry that has the same address, for example,
      in case that br_fdb_changeaddr() has already inserted it,
      br_fdb_change_mac_address() will simply fail to insert it and no
      duplicated entry will be made, as it was.
      
      This approach also needs br_add_if() to call br_fdb_insert() before
      br_stp_recalculate_bridge_id() so that we don't create an entry whose
      dst == NULL in this function to preserve previous behavior.
      
      Note that this is a slight change in behavior where the bridge device can
      receive the traffic to the new address before calling
      br_stp_recalculate_bridge_id() in br_add_if().
      However, it is not a problem because we have already the address on the
      new port and such a way to insert new one before recalculating bridge id
      is taken in br_device_event() as well.
      Signed-off-by: NToshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
      Acked-by: NVlad Yasevich <vyasevic@redhat.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      a4b816d8
    • T
      bridge: Fix the way to find old local fdb entries in br_fdb_change_mac_address · a3ebb7ef
      Toshiaki Makita 提交于
      We have been always failed to delete the old entry at
      br_fdb_change_mac_address() because br_set_mac_address() updates
      dev->dev_addr before calling br_fdb_change_mac_address() and
      br_fdb_change_mac_address() uses dev->dev_addr to find the old entry.
      
      That update of dev_addr is completely unnecessary because the same work
      is done in br_stp_change_bridge_id() which is called right away after
      calling br_fdb_change_mac_address().
      Signed-off-by: NToshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
      Acked-by: NVlad Yasevich <vyasevic@redhat.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      a3ebb7ef
    • T
      bridge: Fix the way to insert new local fdb entries in br_fdb_changeaddr · 2836882f
      Toshiaki Makita 提交于
      Since commit bc9a25d2 ("bridge: Add vlan support for local fdb entries"),
      br_fdb_changeaddr() has inserted a new local fdb entry only if it can
      find old one. But if we have two ports where they have the same address
      or user has deleted a local entry, there will be no entry for one of the
      ports.
      
      Example of problematic case:
        ip link set eth0 address aa:bb:cc:dd:ee:ff
        ip link set eth1 address aa:bb:cc:dd:ee:ff
        brctl addif br0 eth0
        brctl addif br0 eth1 # eth1 will not have a local entry due to dup.
        ip link set eth1 address 12:34:56:78:90:ab
      Then, the new entry for the address 12:34:56:78:90:ab will not be
      created, and the bridge device will not be able to communicate.
      
      Insert new entries regardless of whether we can find old entries or not.
      Signed-off-by: NToshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
      Acked-by: NVlad Yasevich <vyasevic@redhat.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      2836882f
    • T
      bridge: Fix the way to find old local fdb entries in br_fdb_changeaddr · a5642ab4
      Toshiaki Makita 提交于
      br_fdb_changeaddr() assumes that there is at most one local entry per port
      per vlan. It used to be true, but since commit 36fd2b63 ("bridge: allow
      creating/deleting fdb entries via netlink"), it has not been so.
      Therefore, the function might fail to search a correct previous address
      to be deleted and delete an arbitrary local entry if user has added local
      entries manually.
      
      Example of problematic case:
        ip link set eth0 address ee:ff:12:34:56:78
        brctl addif br0 eth0
        bridge fdb add 12:34:56:78:90:ab dev eth0 master
        ip link set eth0 address aa:bb:cc:dd:ee:ff
      Then, the address 12:34:56:78:90:ab might be deleted instead of
      ee:ff:12:34:56:78, the original mac address of eth0.
      
      Address this issue by introducing a new flag, added_by_user, to struct
      net_bridge_fdb_entry.
      
      Note that br_fdb_delete_by_port() has to set added_by_user to 0 in cases
      like:
        ip link set eth0 address 12:34:56:78:90:ab
        ip link set eth1 address aa:bb:cc:dd:ee:ff
        brctl addif br0 eth0
        bridge fdb add aa:bb:cc:dd:ee:ff dev eth0 master
        brctl addif br0 eth1
        brctl delif br0 eth0
      In this case, kernel should delete the user-added entry aa:bb:cc:dd:ee:ff,
      but it also should have been added by "brctl addif br0 eth1" originally,
      so we don't delete it and treat it a new kernel-created entry.
      Signed-off-by: NToshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      a5642ab4
  4. 10 2月, 2014 8 次提交