From b17ad2caae63b4fb546106b7de7c08ba9f3f36da Mon Sep 17 00:00:00 2001
From: Knut Anderssen <kanderssen@suse.com>
Date: Fri, 18 Oct 2024 10:08:03 +0100
Subject: [PATCH 01/15] Some code polishing

---
 src/modules/IscsiClientLib.rb | 356 ++++++++++++++--------------------
 test/iscsi_client_lib_test.rb |   4 +-
 2 files changed, 151 insertions(+), 209 deletions(-)

diff --git a/src/modules/IscsiClientLib.rb b/src/modules/IscsiClientLib.rb
index 0504ca9..9bf0b9c 100644
--- a/src/modules/IscsiClientLib.rb
+++ b/src/modules/IscsiClientLib.rb
@@ -43,6 +43,7 @@ class IscsiClientLibClass < Module
     # if the binary is in /sbin or in /usr/sbin (usr-merge in openSUSE!)
     # (bsc#1196086, bsc#1196086)
     OFFLOAD_SCRIPT = "iscsi_offload".freeze
+    DISCOVERY_CMD = "iscsiadm -m discovery -P 1".freeze
 
     # Driver modules that depend on the iscsiuio service/socket
     #
@@ -57,6 +58,15 @@ class IscsiClientLibClass < Module
 
     # Documentation for attributes that are initialized at #main
 
+    # @!attribute iface_file
+    #   Entries in the iscsi ifaces file which is initialized by #InitIfaceFile but could be
+    #   modified by other calls like the  configure_offload_engines. Every network interface that 
+    #   supports open-iscsi can have one o more iscsi ifaces associated with it.
+    #
+    #   Each entry associates the iscsi file name with the iscsi iface name which is usually the same.
+    #
+    #   @return [Hash{String => String}] ex. { "bnx2i.9c:dc:71:df:cf:29.ipv4.0" => "bnx2i.9c:dc:71:df:cf:29.ipv4.0"}
+    #
     # @!attribute sessions
     #   All connected nodes found via #readSessions
     #
@@ -252,8 +262,11 @@ def GetOffloadCard
       @offload_card
     end
 
+    # It selects the given card as the offload one and calls iscsi_offload script if it is
+    # not the selected one also not the default
     def SetOffloadCard(new_card)
-      Builtins.y2milestone("SetOffloadCard:%1 cur:%2", new_card, @offload_card)
+      log.info "SetOffloadCard:#{new_card} cur:#{@offload_card}"
+
       if new_card != @offload_card
         @offload_card = new_card
         CallConfigScript() if new_card != "default"
@@ -271,7 +284,7 @@ def SetOffloadCard(new_card)
     #
     def GetAdmCmd(params, do_log = true)
       ret = "LC_ALL=POSIX iscsiadm #{params}"
-      Builtins.y2milestone("GetAdmCmd: #{ret}") if do_log
+      log.info("GetAdmCmd: #{ret}") if do_log
       ret
     end
 
@@ -429,9 +442,9 @@ def getNode
       cmdline = GetAdmCmd(
         Builtins.sformat(
           "-S -m node -I %3 -T %1 -p %2",
-          Ops.get(@currentRecord, 1, "").shellescape,
-          Ops.get(@currentRecord, 0, "").shellescape,
-          Ops.get(@currentRecord, 2, "default").shellescape
+          @currentRecord[1].to_s.shellescape,
+          @currentRecord[0].to_s.shellescape,
+          @currentRecord.fetch(2, "default").shellescape
         )
       )
       cmd = SCR.Execute(path(".target.bash_output"), cmdline)
@@ -531,43 +544,39 @@ def setISNSConfig(address, port)
     #
     # @param data [Array<String>] output of the executed command, one array entry per line
     def ScanDiscovered(data)
-      data = deep_copy(data)
       ret = []
       target = ""
       portal = ""
       iface = ""
       dumped = true
-      Builtins.y2milestone("Got data: %1", data)
+      log.info "Got data: #{data}"
 
       # Each entry starts with Target:, the other two values are optional
       # (except the first entry) and, if missing, are inherited from previous
       # entry. Therefore: Dump whatever is cached on Target: entry and once
       # again at the end. Example input in the test case.
 
-      Builtins.foreach(data) do |row|
-        row = Builtins.substring(row, Builtins.findfirstnotof(row, "\t "), 999)
-        if Builtins.search(row, "Target:") != nil
+      data.each do |r|
+        row = r.strip
+        if row.include? "Target:"
           if !dumped
             # don't add Scope:Link IPv6 address
             ret << "#{portal} #{target} #{iface}" if !portal.start_with?("[fe80:")
           end
-          target = Ops.get(Builtins.splitstring(row, " "), 1, "")
+          target = row.split[1]
           dumped = false
-        elsif Builtins.search(row, "Portal:") != nil
-          if Builtins.search(row, "Current Portal:") != nil
-            portal = Ops.get(Builtins.splitstring(row, " "), 2, "")
-          elsif Builtins.search(row, "Persistent Portal:") != nil
+        elsif row.include? "Portal:"
+          if /[Current|Persistent] Portal:/.match?(row)
             # 'Persistent Portal' overwrites current (is used for login)
-            portal = Ops.get(Builtins.splitstring(row, " "), 2, "")
+            portal = row.split[2]
           else
             # read 'Portal' (from output of -m node)
-            portal = Ops.get(Builtins.splitstring(row, " "), 1, "")
+            portal = row.split[1]
           end
-          pos = Builtins.search(portal, ",")
-          portal = Builtins.substring(portal, 0, pos) if pos != nil
-        elsif Builtins.search(row, "Iface Name:") != nil
-          iface = Ops.get(Builtins.splitstring(row, " "), 2, "")
-          iface = Ops.get(@iface_file, iface, iface)
+          portal = portal.split(",")[0] if portal.include?(",")
+        elsif row.include? "Iface Name:"
+          iface = row.split[2]
+          iface = @iface_file.dig(iface, :name) || iface
           # don't add Scope:Link IPv6 address
           ret << "#{portal} #{target} #{iface}" if !portal.start_with?("[fe80:")
           dumped = true
@@ -578,21 +587,16 @@ def ScanDiscovered(data)
         ret << "#{portal} #{target} #{iface}" if !portal.start_with?("[fe80:")
       end
 
-      Builtins.y2milestone("ScanDiscovered ret:%1", ret)
-      deep_copy(ret)
+      log.info "ScanDiscovered ret:#{ret}"
+      ret
     end
 
     # Read all discovered targets from the local nodes database, storing the result in the
     # {#discovered} attribute
     def getDiscovered
-      @discovered = []
       retcode = SCR.Execute(path(".target.bash_output"), GetAdmCmd("-m node -P 1"))
-      if Builtins.size(Ops.get_string(retcode, "stderr", "")) == 0
-        @discovered = ScanDiscovered(
-          Builtins.splitstring(Ops.get_string(retcode, "stdout", ""), "\n")
-        )
-      end
-      deep_copy(@discovered)
+      @discovered =
+        retcode["stderr"].to_s.empty? ? ScanDiscovered(retcode["stdout"].to_s.split("\n")) : []
     end
 
     def start_services_initial
@@ -626,15 +630,10 @@ def start_iscsid_initial
 
     # Get all connected targets storing the result in the #sessions attribute
     def readSessions
-      Builtins.y2milestone("reading current settings")
-      retcode = SCR.Execute(path(".target.bash_output"), GetAdmCmd("-m session -P 1"))
-      @sessions = ScanDiscovered(
-        Builtins.splitstring(Ops.get_string(retcode, "stdout", ""), "\n")
-      )
-      Builtins.y2milestone(
-        "Return list from iscsiadm -m session: %1",
-        @sessions
-      )
+      log.info "reading current settings"
+      ret = SCR.Execute(path(".target.bash_output"), GetAdmCmd("-m session -P 1"))
+      @sessions = ScanDiscovered(ret.fetch("stdout", "").split("\n"))
+      log.info "Return list from iscsiadm -m session: #{@sessions}"
       true
     end
 
@@ -801,18 +800,13 @@ def deleteRecord
         GetAdmCmd(
           Builtins.sformat(
             "-m node -I %3 -T %1 -p %2 --logout",
-            Ops.get(@currentRecord, 1, "").shellescape,
-            Ops.get(@currentRecord, 0, "").shellescape,
-            Ops.get(@currentRecord, 2, "default").shellescape
+            @currentRecord[1].to_s.shellescape,
+            @currentRecord[0].to_s.shellescape,
+            @currentRecord.fetch(2, "default").shellescape,
           )
         )
       )
-      if Ops.greater_than(
-        Builtins.size(Ops.get_string(retcode, "stderr", "")),
-        0
-      )
-        return false
-      end
+      return false unless retcode["stderr"].to_s.empty?
 
       readSessions
       ret
@@ -835,9 +829,9 @@ def removeRecord
         GetAdmCmd(
           Builtins.sformat(
             "-m node -T %1 -p %2 -I %3 --op=delete",
-            Ops.get(@currentRecord, 1, "").shellescape,
-            Ops.get(@currentRecord, 0, "").shellescape,
-            Ops.get(@currentRecord, 2, "").shellescape
+            @currentRecord[1].to_s.shellescape,
+            @currentRecord[0].to_s.shellescape,
+            @currentRecord[2].to_s.shellescape,
           )
         )
       )
@@ -995,11 +989,7 @@ def ipEqual?(session_ip, current_ip)
 
     # check if given target is connected
     def connected(check_ip)
-      Builtins.y2internal(
-        "check connected status for %1 with IP check:%2",
-        @currentRecord,
-        check_ip
-      )
+      log.info "check connected status for #{@currentRecord} with IP check:#{check_ip}"
       !!find_session(check_ip)
     end
 
@@ -1008,30 +998,18 @@ def connected(check_ip)
     # @param check_ip [Boolean] whether the ip address must be considered in the comparison
     # @return [String, nil] corresponding entry from #sessions or nil if not found
     def find_session(check_ip)
-      Builtins.foreach(@sessions) do |row|
+      @sessions.find do |row|
         ip_ok = true
-        list_row = Builtins.splitstring(row, " ")
-        Builtins.y2milestone("Session row: %1", list_row)
+        list_row = row.split
+        log.info "Session row: #{list_row}"
         if check_ip
-          session_ip = Ops.get(
-            Builtins.splitstring(Ops.get(list_row, 0, ""), ","),
-            0, ""
-          )
-          current_ip = Ops.get(
-            Builtins.splitstring(Ops.get(@currentRecord, 0, ""), ","),
-            0, ""
-          )
+          session_ip = list_row[0].to_s.split(",")[0].to_s
+          current_ip = @currentRecord[0].to_s.split(",")[0].to_s
           ip_ok = ipEqual?(session_ip, current_ip)
         end
 
-        if Ops.get(list_row, 1, "") == Ops.get(@currentRecord, 1, "") &&
-            Ops.get(list_row, 2, "") == Ops.get(@currentRecord, 2, "") &&
-            ip_ok
-          return row
-        end
+        (list_row[1] == @currentRecord[1]) && (list_row[2] == @currentRecord[2]) && ip_ok
       end
-
-      nil
     end
 
     # change startup status (manual/onboot) for target
@@ -1047,9 +1025,9 @@ def setStartupStatus(status)
         GetAdmCmd(
           Builtins.sformat(
             "-m node -I %3 -T %1 -p %2 --op=update --name=node.conn[0].startup --value=%4",
-            Ops.get(@currentRecord, 1, "").shellescape,
-            Ops.get(@currentRecord, 0, "").shellescape,
-            Ops.get(@currentRecord, 2, "default").shellescape,
+            @currentRecord[1].to_s.shellescape,
+            @currentRecord[0].to_s.shellescape,
+            @currentRecord.fetch(2, "default").shellescape,
             status.shellescape
           )
         )
@@ -1065,9 +1043,9 @@ def setStartupStatus(status)
           GetAdmCmd(
             Builtins.sformat(
               "-m node -I %3 -T %1 -p %2 --op=update --name=node.startup --value=%4",
-              Ops.get(@currentRecord, 1, "").shellescape,
-              Ops.get(@currentRecord, 0, "").shellescape,
-              Ops.get(@currentRecord, 2, "default").shellescape,
+              @currentRecord[1].to_s.shellescape,
+              @currentRecord[0].to_s.shellescape,
+              @currentRecord.fetch(2, "default").shellescape,
               status.shellescape
             )
           )
@@ -1105,11 +1083,7 @@ def autoLogOn
     #   "password_in".
     def loginIntoTarget(target)
       target = deep_copy(target)
-      @currentRecord = [
-        Ops.get_string(target, "portal", ""),
-        Ops.get_string(target, "target", ""),
-        Ops.get_string(target, "iface", "")
-      ]
+      @currentRecord = [target["portal"].to_s, target["target"].to_s, target["iface"].to_s]
 
       auth = Y2IscsiClient::Authentication.new_from_legacy(target)
       login_into_current(auth)
@@ -1147,9 +1121,9 @@ def login_into_current(auth, silent: false)
         GetAdmCmd(
           Builtins.sformat(
             "-m node -I %3 -T %1 -p %2 --login",
-            Ops.get(@currentRecord, 1, "").shellescape,
-            Ops.get(@currentRecord, 0, "").shellescape,
-            Ops.get(@currentRecord, 2, "").shellescape
+            @currentRecord[1].to_s.shellescape,
+            @currentRecord[0].to_s.shellescape,
+            @currentRecord[2].to_s.shellescape
           )
         )
       )
@@ -1378,70 +1352,50 @@ def InitOffloadCard
       ifaces = []
       if retcode["stderr"].empty?
         ScanDiscovered(retcode["stdout"].split("\n")).each do |s|
-          sl = Builtins.splitstring(s, "  ")
-          iface_name = sl[2] || ""
+          iface_name = s.split[2].to_s
           next if iface_name.empty? || ifaces.include?(iface_name)
 
           ifaces << iface_name
         end
       end
-      Builtins.y2milestone("InitOffloadCard ifaces:%1", ifaces)
+      log.info "InitOffloadCard ifaces:#{ifaces}"
       if ifaces.size > 1
         ret = "all"
       elsif @iface_eth.include?(ifaces.first)
         ret = ifaces.first || "default"
       end
-      Builtins.y2milestone("InitOffloadCard ret:%1", ret)
+      log.info "InitOffloadCard ret:#{ret}"
       ret
     end
 
+    def iface_value(content, field)
+      content.find { |l| l.include? field }.to_s.gsub(/[[:space:]]/,'').split("=")[1]
+    end
+
     def InitIfaceFile
       @iface_file = {}
-      files = Convert.convert(
-        SCR.Read(path(".target.dir"), "/etc/iscsi/ifaces"),
-        :from => "any",
-        :to   => "list <string>"
-      )
-      Builtins.y2milestone("InitIfaceFile files:%1", files)
-      if files == nil || Builtins.size(files) == 0
+      files = SCR.Read(path(".target.dir"), "/etc/iscsi/ifaces") || []
+      log.info "InitIfaceFile files: #{files}"
+      if files.empty?
         cmd = GetAdmCmd("-m iface")
         res = SCR.Execute(path(".target.bash_output"), cmd)
-        Builtins.y2milestone("InitIfaceFile cmd:#{cmd}\nres:#{res.inspect}", cmd)
-        files = SCR.Read(path(".target.dir"), "/etc/iscsi/ifaces")
-        Builtins.y2milestone("InitIfaceFile files:%1", files)
+        log.info "InitIfaceFile cmd: #{cmd}\nres: #{res.inspect}"
+        files = SCR.Read(path(".target.dir"), "/etc/iscsi/ifaces") || []
+        log.info "InitIfaceFile files: #{files}"
       end
-      Builtins.foreach(files) do |file|
-        ls = Builtins.splitstring(
-          Convert.to_string(
-            SCR.Read(
-              path(".target.string"),
-              Ops.add("/etc/iscsi/ifaces/", file)
-            )
-          ),
-          "\n"
-        )
-        Builtins.y2milestone("InitIfaceFile file:%1", file)
-        Builtins.y2milestone("InitIfaceFile ls:%1", ls)
-        ls = Builtins.filter(ls) do |l|
-          Builtins.search(l, "iface.iscsi_ifacename") != nil
-        end
-        Builtins.y2milestone("InitIfaceFile ls:%1", ls)
-        if Ops.greater_than(Builtins.size(ls), 0)
-          Ops.set(
-            @iface_file,
-            Ops.get(
-              Builtins.splitstring(
-                Builtins.deletechars(Ops.get(ls, 0, ""), " "),
-                "="
-              ),
-              1,
-              ""
-            ),
-            file
-          )
+      files.each do |file|
+        ls = SCR.Read(path(".target.string"), "/etc/iscsi/ifaces/#{file}").split("\n")
+        log.info "InitIfaceFile file: #{file}\nInitIfaceFile ls: #{ls}"
+        iface_name = ls.find { |l| l.include? "iface.iscsi_ifacename" }.to_s
+        log.info "InitIfaceFile ls: #{iface_name}"
+        unless iface_name.empty?
+          name = iface_name.gsub(/[[:space:]]/,'').split("=")[1]
+          dev_name = iface_value(ls, "iface.net_ifacename")
+          transport = iface_value(ls, "iface.transport")
+          @iface_file[name] = { :name => name, :dev => dev_name, :transport => transport }
         end
       end
-      Builtins.y2milestone("InitIfaceFile iface_file:%1", @iface_file)
+      log.info "InitIfaceFile iface_file: #{@iface_file}"
 
       nil
     end
@@ -1462,13 +1416,13 @@ def GetOffloadItems
           entries[card[2]] = card_label(card, @offload[i][1])
         end
       end
-      Builtins.y2milestone("GetOffloadItems entries:%1", entries)
+      log.info "GetOffloadItems entries:#{entries}"
 
-      @iface_eth = Builtins.sort(Builtins.maplist(entries) { |e, _val| e })
-      Builtins.y2milestone("GetOffloadItems eth:%1", @iface_eth)
+      @iface_eth = entries.keys.sort
+      log.info "GetOffloadItems eth:#{@iface_eth}"
       if init
         @offload_card = InitOffloadCard()
-        Builtins.y2milestone("GetOffloadItems offload_card:%1", @offload_card)
+        log.info "GetOffloadItems offload_card:#{@offload_card}"
       end
       ret = [
         # Entry for "default"
@@ -1484,8 +1438,8 @@ def GetOffloadItems
       ret.concat(
         @iface_eth.map { |e| Item(Id(e), entries[e], @offload_card == e) }
       )
-      Builtins.y2milestone("GetOffloadItems ret:%1", ret)
-      deep_copy(ret)
+      log.info "GetOffloadItems ret:#{ret}"
+      ret
     end
 
     # Modules to use for all the cards detected in the system and that support hardware
@@ -1498,83 +1452,82 @@ def GetOffloadItems
     def GetOffloadModules
       GetOffloadItems() if @offload_valid == nil
       modules = []
-      Builtins.foreach(@offload_valid) do |i, _l|
-        modules = Convert.convert(
-          Builtins.union(modules, Ops.get_list(@offload, [i, 3], [])),
-          :from => "list",
-          :to   => "list <string>"
-        )
-      end
-      Builtins.y2milestone("GetOffloadModules %1", modules)
-      deep_copy(modules)
+      @offload_valid.each { |i, _| modules = modules.union(@offload[i][3]) }
+      log.info "GetOffloadModules #{modules}"
+      modules
     end
 
     def LoadOffloadModules
       mods = GetOffloadModules()
-      Builtins.foreach(mods) do |s|
-        Builtins.y2milestone("Loading module %1", s)
+      mods.each do |s|
+        log.info "Loading module #{s}"
         ModuleLoading.Load(s, "", "", "", false, true)
       end
-      deep_copy(mods)
+      mods
     end
 
+    # It returns a list of iscsi ifaces corresponding to the current offload card selection, "default" will return
+    # an array with the current offload card while all will return an array with all the offlocad valid ifaces
+    #
+    # @return [Array<String>] List os iscsi ifaces for the current offload card selection, ex. ["eth2-bnx2i"]
     def GetDiscIfaces
       ret = []
       if GetOffloadCard() == "all"
-        tl = Builtins.maplist(GetOffloadItems()) do |t|
-          Ops.get_string(Builtins.argsof(t), [0, 0], "")
-        end
-        Builtins.y2milestone("GetDiscIfaces:%1", tl)
-        ret = Builtins.filter(tl) { |s| s != "all" }
+        tl = GetOffloadItems().map {|t| t.params[0][0].to_s }
+        log.info("GetDiscIfaces:#{tl}")
+        ret = tl.select { |s| s != "all" }
       else
         ret = [GetOffloadCard()]
       end
-      Builtins.y2milestone("GetDiscIfaces:%1", ret)
-      deep_copy(ret)
+      log.info("GetDiscIfaces:#{ret}")
+      ret
     end
 
+    # It calls the iscsi_config.sh script for each of the network devices obtained from the @offload_valid variable
+    # and current @offload_card selection
     def CallConfigScript
-      sl = Builtins.filter(GetDiscIfaces()) { |s| s != "default" }
-      Builtins.y2milestone("CallConfigScript list:%1", sl)
-      Builtins.foreach(sl) do |s|
-        hw = []
-        hw = Ops.get(Builtins.maplist(Builtins.filter(@offload_valid) do |_i, eth|
-          Builtins.contains(
-            Builtins.flatten(
-              Convert.convert(eth, :from => "list", :to => "list <list>")
-            ),
-            s
-          )
-        end) { |_i, e| e }, 0, [])
-        Builtins.y2milestone("CallConfigScript hw:%1", hw)
-        hw = Builtins.find(
-          Convert.convert(hw, :from => "list", :to => "list <list>")
-        ) { |l| Ops.get_string(l, 2, "") == s }
-        Builtins.y2milestone("CallConfigScript hw:%1", hw)
+      sl = GetDiscIfaces().select { |s| s != "default" }
+      log.info "CallConfigScript list:#{sl}"
+      sl.each do |s|
+        hw = @offload_valid.select {|_, eth| eth.flatten.include? s }.values[0] || []
+        log.info "CallConfigScript hw:#{hw}"
+        hw = hw.find {|l| l[2] == s}
+        log.info "CallConfigScript hw:#{hw}"
         if hw != nil
-          cmd = "#{OFFLOAD_SCRIPT} #{Ops.get_string(hw, 0, "").shellescape}"
-          Builtins.y2milestone("CallConfigScript cmd:%1", cmd)
+          cmd = "#{OFFLOAD_SCRIPT} #{hw[0].shellescape}"
+          log.info "CallConfigScript cmd:#{cmd}"
           output = SCR.Execute(path(".target.bash_output"), cmd)
-          Builtins.y2milestone("CallConfigScript %1", output)
+          log.info "CallConfigScript #{output}"
         end
       end
 
       nil
     end
 
+    # Obtains the parameters for calling iscsiadm in discovery mode depending on the current
+    # iSNS configuration as well as the parameters given
+    #
+    # ex.
+    #   GetDiscoveryCmd("192.168.0.100", "3260") =>
+    #     ["iscsiadm", "-m", "discovery", "-P", "1", "-t", "st", "-p", "192.168.0.100:3260"]
+    #
+    # @param ip [String] Portal IP address
+    # @param port [String] Portal port number
+    # @param use_fw [Boolean] whether the target should be fw or not
+    # @only_new [Boolean] whether a new record should be created
     # @return [Array<String>]
     def GetDiscoveryCmd(ip, port, use_fw: false, only_new: false)
-      Builtins.y2milestone("GetDiscoveryCmd ip:%1 port:%2 fw:%3 only new:%4",
-        ip, port, use_fw, only_new)
-      command = ["iscsiadm", "-m", "discovery", "-P", "1"]
+      log.info "GetDiscoveryCmd ip:#{ip} port:#{port} fw:#{use_fw} only new:#{only_new}"
+
+      command = DISCOVERY_CMD.split
       isns_info = useISNS
       if isns_info["use"]
         command << "-t" << "isns"
       else
         ifs = GetDiscIfaces()
-        Builtins.y2milestone("ifs=%1", ifs)
+        log.info "ifs=#{ifs}"
         ifs = ifs.each_with_object([]) { |s, res| res << "-I" << s }
-        Builtins.y2milestone("ifs=%1", ifs)
+        log.info "ifs=#{ifs}"
         tgt = "st"
         tgt = "fw" if use_fw
         command << "-t" << tgt
@@ -1584,7 +1537,7 @@ def GetDiscoveryCmd(ip, port, use_fw: false, only_new: false)
       command << "-p" << "#{ip}:#{port}"
       command << "-o" << "new" if only_new
 
-      Builtins.y2milestone("GetDiscoveryCmd %1", command)
+      log.info "GetDiscoveryCmd #{command}"
       command
     end
 
@@ -1661,24 +1614,15 @@ def InitOffloadValid
           card_res["exit"].zero? && card_res["hwaddr"]
         end
       end
-      Builtins.y2milestone("GetOffloadItems offload_res:%1", offload_res)
-      Builtins.y2milestone("GetOffloadItems offload_valid:%1", @offload_valid)
+      log.info "GetOffloadItems offload_res:#{offload_res}"
+      log.info "GetOffloadItems offload_valid:#{@offload_valid}"
 
       # Sync the MAC with the hwaddr value from offload_res
-      @offload_valid.values.each do |cards|
-        cards.each do |card|
-          dev_name = card[0]
-          card[1] = offload_res[dev_name]["hwaddr"]
-        end
-      end
-      Builtins.y2milestone("GetOffloadItems offload_valid:%1", @offload_valid)
+      @offload_valid.values.each { |cards| cards.each { |c| c[1] = offload_res[c[0]]["hwaddr"] } }
+      log.info "GetOffloadItems offload_valid:#{@offload_valid}"
 
-      @offload_valid.values.each do |cards|
-        cards.each do |card|
-          card << ip_addr(card[0])
-        end
-      end
-      Builtins.y2milestone("GetOffloadItems offload_valid:%1", @offload_valid)
+      @offload_valid.values.each { |cards| cards.each { |c| c << ip_addr(c[0]) } }
+      log.info "GetOffloadItems offload_valid:#{@offload_valid}"
     end
 
     # List of modules for the given card description
@@ -1713,13 +1657,13 @@ def potential_offload_cards
       # Store into hw_mods information about all the cards in the system
       cards = SCR.Read(path(".probe.netcard"))
       hw_mods = cards.map do |c|
-        Builtins.y2milestone("GetOffloadItems card:%1", c)
+        log.info "GetOffloadItems card:#{c}"
         hw_mod = {
           "modules" => netcard_modules(c),
           "iface"   => c["dev_name"] || "",
           "macaddr" => Ops.get_string(c, ["resource", "hwaddr", 0, "addr"], "")
         }
-        Builtins.y2milestone("GetOffloadItems cinf:%1", hw_mod)
+        log.info "GetOffloadItems cinf:#{hw_mod}"
         hw_mod
       end
 
@@ -1733,8 +1677,8 @@ def potential_offload_cards
           # Ignore this card unless it has some module in common with the offload entry
           next if (modules & hw["modules"]).empty?
 
-          Builtins.y2milestone("GetOffloadItems l:%1", offload_entry)
-          Builtins.y2milestone("GetOffloadItems valid:%1", hw)
+          log.info "GetOffloadItems l:#{offload_entry}"
+          log.info "GetOffloadItems valid:#{hw}"
           result[idx] ||= []
           result[idx] << [
             hw["iface"],
@@ -1760,14 +1704,14 @@ def configure_offload_engines(cards)
 
       cards.each do |dev_name|
         cmd = "#{OFFLOAD_SCRIPT} #{dev_name.shellescape} | grep ..:..:..:.." # grep for lines containing MAC address
-        Builtins.y2milestone("GetOffloadItems cmd:%1", cmd)
+        log.info  "GetOffloadItems cmd #{cmd}"
         out = SCR.Execute(path(".target.bash_output"), cmd)
         # Example for output if offload is supported on interface:
         # cmd: iscsi_offload eth2
         # out: $["exit":0, "stderr":"", "stdout":"00:00:c9:b1:bc:7f ip \n"]
         cmd2 = "#{OFFLOAD_SCRIPT} #{dev_name.shellescape}"
         result = SCR.Execute(path(".target.bash_output"), cmd2)
-        Builtins.y2milestone("GetOffloadItems iscsi_offload out:%1", result)
+        log.info "GetOffloadItems iscsi_offload out:#{result}"
 
         offload_res[dev_name] = {}
         offload_res[dev_name]["exit"] = out["exit"]
@@ -1788,8 +1732,6 @@ def ip_addr(dev_name)
         stderr:             :capture,
         allowed_exitstatus: 0..127,
         env:                { "LC_ALL" => "POSIX" })[0]
-      log.info "IP Address config for #{dev_name}: #{stdout}"
-
       # Search for lines containing "inet", means IPv4 address.
       # Regarding the IPv6 support there are no changes needed here because
       # the IP address is not used farther.
diff --git a/test/iscsi_client_lib_test.rb b/test/iscsi_client_lib_test.rb
index 6f3ffa0..d97542a 100644
--- a/test/iscsi_client_lib_test.rb
+++ b/test/iscsi_client_lib_test.rb
@@ -690,14 +690,14 @@ def config_values(data)
                   "\tPortal: 192.168.20.20:3260,2",
                   "\t\tIface Name: default",
                   "\tPortal: 192.168.10.20:3260,1",
-                  "\t\tIface Name: default"]
+                  "\t\tIface Name: bnx2i.9c:dc:71:df:cf:29.ipv4.0"]
         )).to eq(
           [
             "[2620:113:80c0:8080:e051:f9ea:73c7:9171]:3260 iqn.2013-10.de.suse:test_file2 default",
             "10.120.66.182:3260 iqn.2013-10.de.suse:test_file2 default",
             "[2620:113:80c0:8080:a00:27ff:fe1b:a7fe]:3260 iqn.2013-10.de.suse:test_file2 default",
             "192.168.20.20:3260 iqn.2018-06.de.suse.zeus:01 default",
-            "192.168.10.20:3260 iqn.2018-06.de.suse.zeus:01 default"
+            "192.168.10.20:3260 iqn.2018-06.de.suse.zeus:01 bnx2i.9c:dc:71:df:cf:29.ipv4.0"
           ]
         )
       end

From 99bc1e8931a354ce3c425091b8e277ec4266f2f4 Mon Sep 17 00:00:00 2001
From: Knut Anderssen <kanderssen@suse.com>
Date: Tue, 29 Oct 2024 17:37:12 +0000
Subject: [PATCH 02/15] Replaced offload card by iface

---
 src/include/iscsi-client/dialogs.rb |  9 ++---
 src/include/iscsi-client/widgets.rb | 48 ++++++++----------------
 src/modules/IscsiClientLib.rb       | 58 +++++++++++++++++++++++------
 3 files changed, 64 insertions(+), 51 deletions(-)

diff --git a/src/include/iscsi-client/dialogs.rb b/src/include/iscsi-client/dialogs.rb
index 383662b..82606fc 100644
--- a/src/include/iscsi-client/dialogs.rb
+++ b/src/include/iscsi-client/dialogs.rb
@@ -90,12 +90,9 @@ def widgets
                 MinWidth(
                   8,
                   ComboBox(
-                    Id(:offload_card),
+                    Id(:iscsi_iface),
                     Opt(:notify),
-                    # prefer to not translate 'Offload' unless there is a well
-                    # known word for this technology (it's special hardware
-                    # shifting load from processor to card)
-                    _("Offload Car&d"),
+                    _("i&SCSI Iface"),
                     []
                   )
                 )
@@ -113,7 +110,7 @@ def widgets
             "symbol (string, map)"
           ),
           "handle"            => fun_ref(
-            method(:handleOffload),
+            method(:handleIface),
             "symbol (string, map)"
           ),
           "help"              => Ops.get_string(@HELPS, "initiator_name", "")
diff --git a/src/include/iscsi-client/widgets.rb b/src/include/iscsi-client/widgets.rb
index 5f7d178..60c138a 100644
--- a/src/include/iscsi-client/widgets.rb
+++ b/src/include/iscsi-client/widgets.rb
@@ -227,15 +227,10 @@ def storeISNS(_key, event)
     def initInitName(_key)
       Builtins.y2milestone("initiatorname %1", IscsiClientLib.initiatorname)
       UI.ChangeWidget(:initiator_name, :Value, IscsiClientLib.initiatorname)
-      UI.ChangeWidget(:offload_card, :Items, IscsiClientLib.GetOffloadItems)
-      UI.ChangeWidget(:offload_card, :Value, IscsiClientLib.GetOffloadCard)
-      Builtins.y2milestone("OffloadCard %1", IscsiClientLib.GetOffloadCard)
-      if Ops.greater_than(
-        Builtins.size(
-          Ops.get_string(IscsiClientLib.getiBFT, "iSCSI_INITIATOR_NAME", "")
-        ),
-        0
-      )
+      UI.ChangeWidget(:iface, :Items, IscsiClientLib.iface_items)
+      UI.ChangeWidget(:iface, :Value, IscsiClientLib.selected_iface)
+      log.info "Selected Iface: #{IscsiClientLib.selected_iface}"
+      unless IscsiClientLib.getiBFT["iSCSI_INITIATOR_NAME"].to_s.empty?
         UI.ChangeWidget(:initiator_name, :Enabled, false)
         # Not sure if there is such a widget called :write
         UI.ChangeWidget(:write, :Enabled, false)
@@ -290,6 +285,10 @@ def validateInitName(_key, event)
       end
     end
 
+    def iface_value
+      UI.QueryWidget(:iface, :Value).to_s
+    end
+
     def storeInitName(_key, event)
       event = deep_copy(event)
       if Convert.to_string(UI.QueryWidget(:initiator_name, :Value)) !=
@@ -304,34 +303,17 @@ def storeInitName(_key, event)
         else
           Service.Restart("iscsid")
         end
-        Builtins.y2milestone(
-          "write initiatorname %1",
-          IscsiClientLib.initiatorname
-        )
-      end
-      if Convert.to_string(UI.QueryWidget(:offload_card, :Value)) !=
-          IscsiClientLib.GetOffloadCard
-        IscsiClientLib.SetOffloadCard(
-          Convert.to_string(UI.QueryWidget(:offload_card, :Value))
-        )
-        Builtins.y2milestone("OffloadCard %1", IscsiClientLib.GetOffloadCard)
+        log.info "write initiatorname #{IscsientLib.initiatorname}"
       end
+      IscsiClientLib.iface = iface_value if iface_value != IscsiClientLib.selected_iface
       nil
     end
 
-    def handleOffload(_key, event)
-      event = deep_copy(event)
-      if event["EventReason"] || "" == "ValueChanged" &&
-          event["ID"] || :none == :offload_card
-        if Convert.to_string(UI.QueryWidget(:offload_card, :Value)) !=
-            IscsiClientLib.GetOffloadCard
-          IscsiClientLib.SetOffloadCard(
-            Convert.to_string(UI.QueryWidget(:offload_card, :Value))
-          )
-          Builtins.y2milestone(
-            "handleOffload OffloadCard %1",
-            IscsiClientLib.GetOffloadCard
-          )
+    def handleIface(_key, event)
+      if event["EventReason"].to_s == "ValueChanged" && event["ID"] == :iface
+        if iface_value != IscsiClientLib.iface
+          IscsiClientLib.iface = iface_value
+          log.info "handleIface iface: #{IscsiClientLib.iface}"
         end
       end
       nil
diff --git a/src/modules/IscsiClientLib.rb b/src/modules/IscsiClientLib.rb
index 9bf0b9c..c0933ac 100644
--- a/src/modules/IscsiClientLib.rb
+++ b/src/modules/IscsiClientLib.rb
@@ -130,8 +130,10 @@ def main
       @initiatorname = ""
       # map used for autoYaST
       @ay_settings = nil
-      # interface type for hardware offloading
+      # iscsi interface for hardware offloading
       @offload_card = "default"
+      # iscsi iface for discovering
+      @iface = "default"
 
       # Types of offload cards
       # [<id>, <label>, <matching_modules>, <load_modules>]
@@ -262,6 +264,14 @@ def GetOffloadCard
       @offload_card
     end
 
+    def selected_iface
+      @iface || "default"
+    end
+
+    def iface=(iface)
+      log.info "Selecting the iface: #{iface} cur:#{@iface}"
+    end
+
     # It selects the given card as the offload one and calls iscsi_offload script if it is
     # not the selected one also not the default
     def SetOffloadCard(new_card)
@@ -1392,7 +1402,9 @@ def InitIfaceFile
           name = iface_name.gsub(/[[:space:]]/,'').split("=")[1]
           dev_name = iface_value(ls, "iface.net_ifacename")
           transport = iface_value(ls, "iface.transport")
-          @iface_file[name] = { :name => name, :dev => dev_name, :transport => transport }
+          hwaddress = iface_value(ls, "iface.hwaddress")
+          ipaddress = iface_value(ls, "iface.ipaddress")
+          @iface_file[name] = { :name => name, :dev => dev_name, :transport => transport, :hwaddress => hwaddress, :ip => ipaddress }
         end
       end
       log.info "InitIfaceFile iface_file: #{@iface_file}"
@@ -1400,6 +1412,26 @@ def InitIfaceFile
       nil
     end
 
+    def default_item
+      Item(Id(@offload[0][0]), @offload[0][1], @iface == @offload[0][0])
+    end
+
+    def all_item
+      Item(Id(@offload[1][0]), @offload[1][1], @iface== @offload[1][0])
+    end
+
+    def iface_items
+      InitIfaceFile() unless @iface_file
+
+      items = [default_item]
+      items << all_item if @iface_file.any?
+
+      @iface_file.each { |n, e| items << Item(Id(n), iface_label(e), @iface == n) }
+    end
+
+    # Return an array of OffloadCard Items
+    #
+    # [Item(Id("default"), "Default", true), Item(Id("all"), "All", false), Item(Id("eth0-bnx2i"), "eth2 - 00:00:00: - bnx2/bnx2x/bnx2i", false)]
     def GetOffloadItems
       init = false
       if @offload_valid.nil?
@@ -1471,16 +1503,9 @@ def LoadOffloadModules
     #
     # @return [Array<String>] List os iscsi ifaces for the current offload card selection, ex. ["eth2-bnx2i"]
     def GetDiscIfaces
-      ret = []
-      if GetOffloadCard() == "all"
-        tl = GetOffloadItems().map {|t| t.params[0][0].to_s }
-        log.info("GetDiscIfaces:#{tl}")
-        ret = tl.select { |s| s != "all" }
-      else
-        ret = [GetOffloadCard()]
-      end
-      log.info("GetDiscIfaces:#{ret}")
-      ret
+      ifaces = @iface == "all" ? @iface_file.keys : [@iface]
+      log.info "GetDiscIfaces:#{ifaces}" 
+      ifaces
     end
 
     # It calls the iscsi_config.sh script for each of the network devices obtained from the @offload_valid variable
@@ -1602,9 +1627,14 @@ def load_modules
 
   private
 
+    def bring_up(card_names)
+      card_names.each { |n| Yast::Execute.locally!("ip", "link", "set", "dev", "up", n) }
+    end
+
     def InitOffloadValid
       @offload_valid = potential_offload_cards
       card_names = @offload_valid.values.flatten(1).map(&:first)
+      # bring_up(card_names)
       offload_res = configure_offload_engines(card_names)
 
       # Filter only those cards for which we have a hwaddr value in offload_res
@@ -1748,6 +1778,10 @@ def ip_addr(dev_name)
       ipaddr
     end
 
+    def iface_label(data) 
+      [data.name, data.hwaddress, data.transport_name].compact.join(" - ")
+    end
+
     def card_label(card, type_label)
       dev_name = card[0]
       hwaddr = card[1]

From f79cb4ab71e8202ef419f22aa9fa037fbc40108a Mon Sep 17 00:00:00 2001
From: Knut Anderssen <kanderssen@suse.com>
Date: Wed, 30 Oct 2024 14:31:24 +0000
Subject: [PATCH 03/15] Getting rid of offload card related methods

---
 src/include/iscsi-client/dialogs.rb |   4 +-
 src/include/iscsi-client/widgets.rb |   6 +-
 src/modules/IscsiClientLib.rb       | 240 ++++++----------------------
 3 files changed, 56 insertions(+), 194 deletions(-)

diff --git a/src/include/iscsi-client/dialogs.rb b/src/include/iscsi-client/dialogs.rb
index 82606fc..670543c 100644
--- a/src/include/iscsi-client/dialogs.rb
+++ b/src/include/iscsi-client/dialogs.rb
@@ -90,9 +90,9 @@ def widgets
                 MinWidth(
                   8,
                   ComboBox(
-                    Id(:iscsi_iface),
+                    Id(:iface),
                     Opt(:notify),
-                    _("i&SCSI Iface"),
+                    _("iSCSI I&face"),
                     []
                   )
                 )
diff --git a/src/include/iscsi-client/widgets.rb b/src/include/iscsi-client/widgets.rb
index 60c138a..9083a8b 100644
--- a/src/include/iscsi-client/widgets.rb
+++ b/src/include/iscsi-client/widgets.rb
@@ -33,6 +33,8 @@
 
 module Yast
   module IscsiClientWidgetsInclude
+    include Logger
+
     def initialize_iscsi_client_widgets(_include_target)
       textdomain "iscsi-client"
       Yast.import "IP"
@@ -311,9 +313,9 @@ def storeInitName(_key, event)
 
     def handleIface(_key, event)
       if event["EventReason"].to_s == "ValueChanged" && event["ID"] == :iface
-        if iface_value != IscsiClientLib.iface
+        if iface_value != IscsiClientLib.selected_iface
           IscsiClientLib.iface = iface_value
-          log.info "handleIface iface: #{IscsiClientLib.iface}"
+          log.info "handleIface iface: #{IscsiClientLib.selected_iface}"
         end
       end
       nil
diff --git a/src/modules/IscsiClientLib.rb b/src/modules/IscsiClientLib.rb
index c0933ac..14320f0 100644
--- a/src/modules/IscsiClientLib.rb
+++ b/src/modules/IscsiClientLib.rb
@@ -117,7 +117,7 @@ def main
       @discovered = []
       @targets = []
       @currentRecord = []
-      @iface_file = {}
+      @iface_file = nil
       @iface_eth = []
 
       # Content of the main configuration file (/etc/iscsi/iscsid.conf)
@@ -272,19 +272,6 @@ def iface=(iface)
       log.info "Selecting the iface: #{iface} cur:#{@iface}"
     end
 
-    # It selects the given card as the offload one and calls iscsi_offload script if it is
-    # not the selected one also not the default
-    def SetOffloadCard(new_card)
-      log.info "SetOffloadCard:#{new_card} cur:#{@offload_card}"
-
-      if new_card != @offload_card
-        @offload_card = new_card
-        CallConfigScript() if new_card != "default"
-      end
-
-      nil
-    end
-
     # Create and return complete iscsciadm command by adding the string
     # argument as options. If allowed, write the command to y2log file.
     #
@@ -1263,100 +1250,52 @@ def autoyastWrite
       portals = []
       ifaces = []
       ifacepar = ""
-      Builtins.foreach(Ops.get_list(@ay_settings, "targets", [])) do |target|
-        iface = Ops.get_string(target, "iface", "default")
+      @ay_settings.fetch("targets", []).each do |target|
+        iface = target.fetch("iface", "default")
         next if ifaces.include?(iface) # already added
 
         ifacepar << " " unless ifacepar.empty?
         ifacepar << "-I " << iface.shellescape
         ifaces << iface
       end
-      if Ops.greater_than(Builtins.size(Builtins.filter(ifaces) do |s|
-                                          s != "default"
-                                        end), 0)
-        CallConfigScript()
-      end
-      Builtins.foreach(Ops.get_list(@ay_settings, "targets", [])) do |target|
-        if !Builtins.contains(portals, Ops.get_string(target, "portal", ""))
+
+      @ay_settings.fetch("targets", []).each do |target|
+        unless portals.include? target["portal"]
           SCR.Execute(
             path(".target.bash"),
-            GetAdmCmd(
-              Builtins.sformat(
-                "-m discovery %1 -t st -p %2",
-                ifacepar,
-                Ops.get_string(target, "portal", "").shellescape
-              )
-            )
+            GetAdmCmd(%|-m discovery #{ifacepar} -t st -p #{target["portal"].shellescape}|)
           )
-          portals = Builtins.add(portals, Ops.get_string(target, "portal", ""))
+          portals << target["portal"]
         end
       end
-      Builtins.foreach(Ops.get_list(@ay_settings, "targets", [])) do |target|
-        Builtins.y2internal("login into target %1", target)
+      @ay_settings.fetch("targets", []).each do |target|
+        log.info "login into target #{target}"
         loginIntoTarget(target)
-        @currentRecord = [
-          Ops.get_string(target, "portal", ""),
-          Ops.get_string(target, "target", ""),
-          Ops.get_string(target, "iface", "")
-        ]
-        setStartupStatus(Ops.get_string(target, "startup", "manual"))
+        @currentRecord = [target["portal"], target["target"], target["iface"]]
+        setStartupStatus(target.fetch("startup", "manual"))
       end
       true
     end
 
     def Overview
       overview = _("Configuration summary...")
-      if Ops.greater_than(Builtins.size(@ay_settings), 0)
+      unless (@ay_settings || {}).empty?
         overview = ""
-        if Ops.greater_than(
-          Builtins.size(Ops.get_string(@ay_settings, "initiatorname", "")),
-          0
-        )
-          overview = Ops.add(
-            Ops.add(
-              Ops.add(overview, "<p><b>Initiatorname: </b>"),
-              Ops.get_string(@ay_settings, "initiatorname", "")
-            ),
-            "</p>"
-          )
+        initiatorname = @ay_settings.fetch("initiatorname", "")
+        targets = @ay_settings.fetch("targets", [])
+        unless initiatorname.empty?
+          overview << "<p><b>Initiatorname: </b>#{initiatorname}</p>"
         end
-        if Ops.greater_than(
-          Builtins.size(Ops.get_list(@ay_settings, "targets", [])),
-          0
-        )
-          Builtins.foreach(Ops.get_list(@ay_settings, "targets", [])) do |target|
-            overview = Ops.add(
-              Ops.add(
-                Ops.add(
-                  Ops.add(
-                    Ops.add(
-                      Ops.add(
-                        Ops.add(
-                          Ops.add(
-                            Ops.add(overview, "<p>"),
-                            Ops.get_string(target, "portal", "")
-                          ),
-                          ", "
-                        ),
-                        Ops.get_string(target, "target", "")
-                      ),
-                      ", "
-                    ),
-                    Ops.get_string(target, "iface", "")
-                  ),
-                  ", "
-                ),
-                Ops.get_string(target, "startup", "")
-              ),
-              "</p>"
-            )
+        unless targets.empty?
+          targets.each do |t|
+            overview << "<p>#{t["portal"]}, #{t["target"]}, #{t["iface"]}, #{t["startup"]}</p>"
           end
         end
       end
       overview
     end
 
-    def InitOffloadCard
+    def InitIface
       ret = "default"
       retcode = SCR.Execute(path(".target.bash_output"), GetAdmCmd("-m node -P 1"))
       ifaces = []
@@ -1368,14 +1307,14 @@ def InitOffloadCard
           ifaces << iface_name
         end
       end
-      log.info "InitOffloadCard ifaces:#{ifaces}"
+      log.info "InitIface ifaces:#{ifaces}"
       if ifaces.size > 1
         ret = "all"
-      elsif @iface_eth.include?(ifaces.first)
-        ret = ifaces.first || "default"
+      elsif (@iface_file || {}).keys.include?(ifaces.first)
+        ret = ifaces.first
       end
-      log.info "InitOffloadCard ret:#{ret}"
-      ret
+      log.info "InitIface ret:#{ret}"
+      @iface = ret
     end
 
     def iface_value(content, field)
@@ -1421,57 +1360,16 @@ def all_item
     end
 
     def iface_items
-      InitIfaceFile() unless @iface_file
+      if @iface_file.nil?
+        InitIfaceFile()
+        InitIface()
+      end
 
       items = [default_item]
       items << all_item if @iface_file.any?
 
       @iface_file.each { |n, e| items << Item(Id(n), iface_label(e), @iface == n) }
-    end
-
-    # Return an array of OffloadCard Items
-    #
-    # [Item(Id("default"), "Default", true), Item(Id("all"), "All", false), Item(Id("eth0-bnx2i"), "eth2 - 00:00:00: - bnx2/bnx2x/bnx2i", false)]
-    def GetOffloadItems
-      init = false
-      if @offload_valid.nil?
-        init = true
-        InitIfaceFile()
-        InitOffloadValid()
-      end
-
-      entries = {}
-      @offload_valid.each do |i, cards|
-        cards.each do |card|
-          next if card[0].nil? || card[0].empty?
-
-          entries[card[2]] = card_label(card, @offload[i][1])
-        end
-      end
-      log.info "GetOffloadItems entries:#{entries}"
-
-      @iface_eth = entries.keys.sort
-      log.info "GetOffloadItems eth:#{@iface_eth}"
-      if init
-        @offload_card = InitOffloadCard()
-        log.info "GetOffloadItems offload_card:#{@offload_card}"
-      end
-      ret = [
-        # Entry for "default"
-        Item(Id(@offload[0][0]), @offload[0][1], @offload_card == @offload[0][0])
-      ]
-      if @offload_valid.any?
-        # Entry for "all"
-        ret << Item(
-          Id(@offload[1][0]), @offload[1][1], @offload_card == @offload[1][0]
-        )
-      end
-      # Entries for the valid cards
-      ret.concat(
-        @iface_eth.map { |e| Item(Id(e), entries[e], @offload_card == e) }
-      )
-      log.info "GetOffloadItems ret:#{ret}"
-      ret
+      items
     end
 
     # Modules to use for all the cards detected in the system and that support hardware
@@ -1482,11 +1380,11 @@ def GetOffloadItems
     #
     # @return [Array<String>]
     def GetOffloadModules
-      GetOffloadItems() if @offload_valid == nil
+      InitOffloadValid() if @offload_valid == nil
       modules = []
-      @offload_valid.each { |i, _| modules = modules.union(@offload[i][3]) }
+      @offload_valid.each { |i, _| modules.concat(@offload[i][3]) }
       log.info "GetOffloadModules #{modules}"
-      modules
+      modules.uniq
     end
 
     def LoadOffloadModules
@@ -1508,27 +1406,6 @@ def GetDiscIfaces
       ifaces
     end
 
-    # It calls the iscsi_config.sh script for each of the network devices obtained from the @offload_valid variable
-    # and current @offload_card selection
-    def CallConfigScript
-      sl = GetDiscIfaces().select { |s| s != "default" }
-      log.info "CallConfigScript list:#{sl}"
-      sl.each do |s|
-        hw = @offload_valid.select {|_, eth| eth.flatten.include? s }.values[0] || []
-        log.info "CallConfigScript hw:#{hw}"
-        hw = hw.find {|l| l[2] == s}
-        log.info "CallConfigScript hw:#{hw}"
-        if hw != nil
-          cmd = "#{OFFLOAD_SCRIPT} #{hw[0].shellescape}"
-          log.info "CallConfigScript cmd:#{cmd}"
-          output = SCR.Execute(path(".target.bash_output"), cmd)
-          log.info "CallConfigScript #{output}"
-        end
-      end
-
-      nil
-    end
-
     # Obtains the parameters for calling iscsiadm in discovery mode depending on the current
     # iSNS configuration as well as the parameters given
     #
@@ -1587,8 +1464,6 @@ def load_modules
     publish :variable => :currentRecord, :type => "list <string>"
     publish :variable => :initiatorname, :type => "string"
     publish :variable => :ay_settings, :type => "map"
-    publish :function => :GetOffloadCard, :type => "string ()"
-    publish :function => :SetOffloadCard, :type => "void (string)"
     publish :function => :GetAdmCmd, :type => "string (string)"
     publish :function => :hidePassword, :type => "map <string, any> (map <string, any>)"
     publish :function => :getiBFT, :type => "map <string, any> ()"
@@ -1619,7 +1494,7 @@ def load_modules
     publish :function => :autoyastPrepare, :type => "boolean ()"
     publish :function => :autoyastWrite, :type => "boolean ()"
     publish :function => :Overview, :type => "string ()"
-    publish :function => :GetOffloadItems, :type => "list <term> ()"
+    publish :function => :iface_items, :type => "list <term> ()"
     publish :function => :GetOffloadModules, :type => "list <string> ()"
     publish :function => :LoadOffloadModules, :type => "list <string> ()"
     publish :function => :getCurrentNodeValues, :type => "map <string, any> ()"
@@ -1628,31 +1503,20 @@ def load_modules
   private
 
     def bring_up(card_names)
-      card_names.each { |n| Yast::Execute.locally!("ip", "link", "set", "dev", "up", n) }
+      card_names.each { |n| Yast::Execute.locally!("ip", "link", "set", "dev", n, "up") }
     end
 
     def InitOffloadValid
-      @offload_valid = potential_offload_cards
-      card_names = @offload_valid.values.flatten(1).map(&:first)
-      # bring_up(card_names)
-      offload_res = configure_offload_engines(card_names)
-
-      # Filter only those cards for which we have a hwaddr value in offload_res
-      @offload_valid.values.each do |cards|
-        cards.select! do |card|
-          card_res = offload_res[card[0]]
-          card_res["exit"].zero? && card_res["hwaddr"]
-        end
+      if @iface_file.nil?
+        InitIfaceFile()
+        InitIface()
       end
-      log.info "GetOffloadItems offload_res:#{offload_res}"
-      log.info "GetOffloadItems offload_valid:#{@offload_valid}"
 
-      # Sync the MAC with the hwaddr value from offload_res
-      @offload_valid.values.each { |cards| cards.each { |c| c[1] = offload_res[c[0]]["hwaddr"] } }
-      log.info "GetOffloadItems offload_valid:#{@offload_valid}"
-
-      @offload_valid.values.each { |cards| cards.each { |c| c << ip_addr(c[0]) } }
-      log.info "GetOffloadItems offload_valid:#{@offload_valid}"
+      @offload_valid = potential_offload_cards
+      card_names = @offload_valid.values.flatten(1).map {|c| c["iface"] }.uniq
+      bring_up(card_names)
+      log.info "OffloadValid entries#{@offload_valid}"
+      nil
     end
 
     # List of modules for the given card description
@@ -1686,14 +1550,14 @@ def netcard_modules(card)
     def potential_offload_cards
       # Store into hw_mods information about all the cards in the system
       cards = SCR.Read(path(".probe.netcard"))
-      hw_mods = cards.map do |c|
+      hw_mods = cards.select {|c| c["iscsioffload"] }.map do |c|
         log.info "GetOffloadItems card:#{c}"
         hw_mod = {
           "modules" => netcard_modules(c),
           "iface"   => c["dev_name"] || "",
-          "macaddr" => Ops.get_string(c, ["resource", "hwaddr", 0, "addr"], "")
+          "macaddr" => c.dig("resource", "hwaddr", 0, "addr") || ""
         }
-        log.info "GetOffloadItems cinf:#{hw_mod}"
+        log.info "OffloadCards hw:#{hw_mod}"
         hw_mod
       end
 
@@ -1710,11 +1574,7 @@ def potential_offload_cards
           log.info "GetOffloadItems l:#{offload_entry}"
           log.info "GetOffloadItems valid:#{hw}"
           result[idx] ||= []
-          result[idx] << [
-            hw["iface"],
-            hw["macaddr"], # In fact, this is going to be overwritten at a later point
-            "#{hw["iface"]}-#{offload_entry[3].first}"
-          ]
+          result[idx] << hw
         end
       end
 
@@ -1778,8 +1638,8 @@ def ip_addr(dev_name)
       ipaddr
     end
 
-    def iface_label(data) 
-      [data.name, data.hwaddress, data.transport_name].compact.join(" - ")
+    def iface_label(data)
+      [data[:name], data[:ip]].compact.join(" - ")
     end
 
     def card_label(card, type_label)

From 8260a9c8efcaa0a005b88890769447827c78a785 Mon Sep 17 00:00:00 2001
From: Knut Anderssen <kanderssen@suse.com>
Date: Wed, 30 Oct 2024 17:48:35 +0000
Subject: [PATCH 04/15] Some more polishing

---
 src/modules/IscsiClientLib.rb | 118 +++++++++++++---------------------
 1 file changed, 44 insertions(+), 74 deletions(-)

diff --git a/src/modules/IscsiClientLib.rb b/src/modules/IscsiClientLib.rb
index 14320f0..6794ef0 100644
--- a/src/modules/IscsiClientLib.rb
+++ b/src/modules/IscsiClientLib.rb
@@ -59,9 +59,8 @@ class IscsiClientLibClass < Module
     # Documentation for attributes that are initialized at #main
 
     # @!attribute iface_file
-    #   Entries in the iscsi ifaces file which is initialized by #InitIfaceFile but could be
-    #   modified by other calls like the  configure_offload_engines. Every network interface that 
-    #   supports open-iscsi can have one o more iscsi ifaces associated with it.
+    #   Entries in the iscsi ifaces file which is initialized by #InitIfaceFile . Every network
+    #   interface that supports open-iscsi can have one o more iscsi ifaces associated with it.
     #
     #   Each entry associates the iscsi file name with the iscsi iface name which is usually the same.
     #
@@ -171,13 +170,10 @@ def main
       #
       # Eg.
       # {
-      #   2 => [ ["eth4", "00:11:22:33:44:55:66", "eth4-bnx2i", "191.212.2.2"] ],
-      #   6 => [
-      #          ["eth0", "00:...:33", "eth0-be2iscsi", "12.16.0.1"],
-      #          ["eth1", "00:...:11", "eth1-be2iscsi", "19.2.20.1"]
-      #        ]
+      #   2 => [ { "iface" => "eth4", "macaddr" =>  "00:11:22:33:44:55:66", "modules" => ["bnx2i"] } ],
+      #   6 => [ { "iface" => "eth0", "macaddr" => "00:....:33", "modules => "be2iscsi" }
+      #          { "iface" => "eth0", "macaddr" => "00:....:11", "modules => "be2iscsi" }]
       # }
-      #
       @offload_valid = nil
 
       # Very likely, these two instance variables should NOT be directly accessed everywhere in
@@ -435,15 +431,9 @@ def oldConfig
       nil
     end
 
+
     def getNode
-      cmdline = GetAdmCmd(
-        Builtins.sformat(
-          "-S -m node -I %3 -T %1 -p %2",
-          @currentRecord[1].to_s.shellescape,
-          @currentRecord[0].to_s.shellescape,
-          @currentRecord.fetch(2, "default").shellescape
-        )
-      )
+      cmdline = GetAdmCmd("-S -m node -I #{current_iface} -T #{current_target} -p #{current_portal}")
       cmd = SCR.Execute(path(".target.bash_output"), cmdline)
       return {} if Ops.get_integer(cmd, "exit", 0) != 0
       auth = {}
@@ -790,18 +780,11 @@ def checkInitiatorName(silent: false)
     #   {#sessions} gets refreshed in the latter case)
     def deleteRecord
       ret = true
-      Builtins.y2milestone("Delete record %1", @currentRecord)
+      log.info "Delete record #{@currentRecord}"
 
       retcode = SCR.Execute(
         path(".target.bash_output"),
-        GetAdmCmd(
-          Builtins.sformat(
-            "-m node -I %3 -T %1 -p %2 --logout",
-            @currentRecord[1].to_s.shellescape,
-            @currentRecord[0].to_s.shellescape,
-            @currentRecord.fetch(2, "default").shellescape,
-          )
-        )
+        GetAdmCmd("-m node -I #{current_iface} -T #{current_target} -p #{current_portal} --logout")
       )
       return false unless retcode["stderr"].to_s.empty?
 
@@ -819,20 +802,13 @@ def deleteRecord
     #
     # @return [Boolean] whether the operation succeeded with no incidences
     def removeRecord
-      Builtins.y2milestone("Remove record %1", @currentRecord)
+      log.info "Remove record #{@currentRecord}"
 
       result = SCR.Execute(
         path(".target.bash_output"),
-        GetAdmCmd(
-          Builtins.sformat(
-            "-m node -T %1 -p %2 -I %3 --op=delete",
-            @currentRecord[1].to_s.shellescape,
-            @currentRecord[0].to_s.shellescape,
-            @currentRecord[2].to_s.shellescape,
-          )
-        )
+        GetAdmCmd("-m node -T #{current_target} -p #{current_portal} -I #{current_iface} --op=delete")
       )
-      Builtins.y2milestone(result.inspect)
+      log.info(result.inspect)
       result["exit"].zero?
     end
 
@@ -842,7 +818,7 @@ def removeRecord
     #                   converted to a hash
     def getCurrentNodeValues
       ret = SCR.Execute(path(".target.bash_output"),
-        GetAdmCmd("-m node -I #{(@currentRecord[2] || "default").shellescape} -T #{(@currentRecord[1] || "").shellescape} -p #{(@currentRecord[0] || "").shellescape}"))
+        GetAdmCmd("-m node -I #{current_iface} -T #{current_target} -p #{current_portal}"))
       return {} if ret["exit"] != 0
 
       nodeInfoToMap(ret["stdout"] || "")
@@ -923,28 +899,24 @@ def default_startup_status
 
     # update authentication value
     def setValue(name, value)
-      rec = @currentRecord
-      Builtins.y2milestone("set %1  for record %2", name, rec)
+      log.info "set #{name} for record #{@currentRecord}"
 
       log = !name.include?("password")
-      cmd = "-m node -I #{(rec[2] || "default").shellescape} -T #{(rec[1] || "").shellescape} -p #{(rec[0] || "").shellescape} --op=update --name=#{name.shellescape}"
+      cmd = "-m node -I #{current_iface} -T #{current_target} -p #{current_portal} --op=update --name=#{name.shellescape}"
 
       command = GetAdmCmd("#{cmd} --value=#{value.shellescape}", log)
       if !log
         value = "*****" if !value.empty?
-        Builtins.y2milestone("AdmCmd:LC_ALL=POSIX iscsiadm #{cmd} --value=#{value}")
+        log.info "AdmCmd:LC_ALL=POSIX iscsiadm #{cmd} --value=#{value}"
       end
 
       ret = true
       retcode = SCR.Execute(path(".target.bash_output"), command)
-      if Ops.greater_than(
-        Builtins.size(Ops.get_string(retcode, "stderr", "")),
-        0
-      )
-        Builtins.y2error("%1", Ops.get_string(retcode, "stderr", ""))
-        ret = false
+      unless retcode["stderr"].to_s.empty?
+        log.error "#{retcode["stderr"]}"
+        return false
       end
-      Builtins.y2milestone("return value %1", ret)
+      log.info "return value #{ret}"
       ret
     end
 
@@ -1011,45 +983,38 @@ def find_session(check_ip)
 
     # change startup status (manual/onboot) for target
     def setStartupStatus(status)
-      Builtins.y2milestone(
-        "Set startup status for %1 to %2",
-        @currentRecord,
-        status
-      )
+      log.info "Set startup status for #{@currentRecord} to #{status}"
       ret = true
       retcode = SCR.Execute(
         path(".target.bash_output"),
         GetAdmCmd(
           Builtins.sformat(
             "-m node -I %3 -T %1 -p %2 --op=update --name=node.conn[0].startup --value=%4",
-            @currentRecord[1].to_s.shellescape,
-            @currentRecord[0].to_s.shellescape,
-            @currentRecord.fetch(2, "default").shellescape,
+            current_target,
+            current_portal,
+            current_iface,
             status.shellescape
           )
         )
       )
-      if Ops.greater_than(
-        Builtins.size(Ops.get_string(retcode, "stderr", "")),
-        0
-      )
-        return false
-      else
+      if retcode["stderr"].to_s.empty?
         retcode = SCR.Execute(
           path(".target.bash_output"),
           GetAdmCmd(
             Builtins.sformat(
               "-m node -I %3 -T %1 -p %2 --op=update --name=node.startup --value=%4",
-              @currentRecord[1].to_s.shellescape,
-              @currentRecord[0].to_s.shellescape,
-              @currentRecord.fetch(2, "default").shellescape,
+              current_target,
+              current_portal,
+              current_iface,
               status.shellescape
             )
           )
         )
+      else
+        ret = false
       end
 
-      Builtins.y2internal("retcode %1", retcode)
+      log.info "retcode #{retcode}"
       ret
     end
 
@@ -1115,14 +1080,7 @@ def login_into_current(auth, silent: false)
 
       output = SCR.Execute(
         path(".target.bash_output"),
-        GetAdmCmd(
-          Builtins.sformat(
-            "-m node -I %3 -T %1 -p %2 --login",
-            @currentRecord[1].to_s.shellescape,
-            @currentRecord[0].to_s.shellescape,
-            @currentRecord[2].to_s.shellescape
-          )
-        )
+        GetAdmCmd("-m node -I #{current_iface} -T #{current_target} -p #{current_portal} --login")
       )
 
       Builtins.y2internal("output %1", output)
@@ -1502,6 +1460,18 @@ def load_modules
 
   private
 
+    def current_target
+      @currentRecord[1].to_s.shellescape
+    end
+
+    def current_portal
+      @currentRecord[0].to_s.shellescape
+    end
+
+    def current_iface
+      @currentRecord.fetch(2, "default").shellescape
+    end
+
     def bring_up(card_names)
       card_names.each { |n| Yast::Execute.locally!("ip", "link", "set", "dev", n, "up") }
     end

From 6e46553c4263988a0f22bbc77dbda57173422169 Mon Sep 17 00:00:00 2001
From: Knut Anderssen <kanderssen@suse.com>
Date: Thu, 31 Oct 2024 10:16:01 +0000
Subject: [PATCH 05/15] Adapted unit test for the iface items

---
 src/modules/IscsiClientLib.rb                 |  56 +++---
 .../ifaces/bnx2i.ab:cd:de:fa:cf:29.ipv4.0     |  26 +++
 test/iscsi_client_lib_test.rb                 | 183 +++++++-----------
 3 files changed, 126 insertions(+), 139 deletions(-)
 create mode 100644 test/data/chroot1/etc/iscsi/ifaces/bnx2i.ab:cd:de:fa:cf:29.ipv4.0

diff --git a/src/modules/IscsiClientLib.rb b/src/modules/IscsiClientLib.rb
index 6794ef0..7569eee 100644
--- a/src/modules/IscsiClientLib.rb
+++ b/src/modules/IscsiClientLib.rb
@@ -431,7 +431,6 @@ def oldConfig
       nil
     end
 
-
     def getNode
       cmdline = GetAdmCmd("-S -m node -I #{current_iface} -T #{current_target} -p #{current_portal}")
       cmd = SCR.Execute(path(".target.bash_output"), cmdline)
@@ -553,7 +552,7 @@ def ScanDiscovered(data)
           target = row.split[1]
           dumped = false
         elsif row.include? "Portal:"
-          if /[Current|Persistent] Portal:/.match?(row)
+          if /(Current|Persistent) Portal:/.match?(row)
             # 'Persistent Portal' overwrites current (is used for login)
             portal = row.split[2]
           else
@@ -563,7 +562,7 @@ def ScanDiscovered(data)
           portal = portal.split(",")[0] if portal.include?(",")
         elsif row.include? "Iface Name:"
           iface = row.split[2]
-          iface = @iface_file.dig(iface, :name) || iface
+          iface = (@iface_file || {}).dig(iface, :name) || iface
           # don't add Scope:Link IPv6 address
           ret << "#{portal} #{target} #{iface}" if !portal.start_with?("[fe80:")
           dumped = true
@@ -901,11 +900,11 @@ def default_startup_status
     def setValue(name, value)
       log.info "set #{name} for record #{@currentRecord}"
 
-      log = !name.include?("password")
+      login = !name.include?("password")
       cmd = "-m node -I #{current_iface} -T #{current_target} -p #{current_portal} --op=update --name=#{name.shellescape}"
 
-      command = GetAdmCmd("#{cmd} --value=#{value.shellescape}", log)
-      if !log
+      command = GetAdmCmd("#{cmd} --value=#{value.shellescape}", login)
+      if !login
         value = "*****" if !value.empty?
         log.info "AdmCmd:LC_ALL=POSIX iscsiadm #{cmd} --value=#{value}"
       end
@@ -913,7 +912,7 @@ def setValue(name, value)
       ret = true
       retcode = SCR.Execute(path(".target.bash_output"), command)
       unless retcode["stderr"].to_s.empty?
-        log.error "#{retcode["stderr"]}"
+        log.error retcode["stderr"]
         return false
       end
       log.info "return value #{ret}"
@@ -1218,13 +1217,12 @@ def autoyastWrite
       end
 
       @ay_settings.fetch("targets", []).each do |target|
-        unless portals.include? target["portal"]
-          SCR.Execute(
-            path(".target.bash"),
-            GetAdmCmd(%|-m discovery #{ifacepar} -t st -p #{target["portal"].shellescape}|)
-          )
-          portals << target["portal"]
-        end
+        next if portals.include? target["portal"]
+        SCR.Execute(
+          path(".target.bash"),
+          GetAdmCmd(%(-m discovery #{ifacepar} -t st -p #{target["portal"].shellescape}))
+        )
+        portals << target["portal"]
       end
       @ay_settings.fetch("targets", []).each do |target|
         log.info "login into target #{target}"
@@ -1276,7 +1274,7 @@ def InitIface
     end
 
     def iface_value(content, field)
-      content.find { |l| l.include? field }.to_s.gsub(/[[:space:]]/,'').split("=")[1]
+      content.find { |l| l.include? field }.to_s.gsub(/[[:space:]]/, "").split("=")[1]
     end
 
     def InitIfaceFile
@@ -1293,16 +1291,16 @@ def InitIfaceFile
       files.each do |file|
         ls = SCR.Read(path(".target.string"), "/etc/iscsi/ifaces/#{file}").split("\n")
         log.info "InitIfaceFile file: #{file}\nInitIfaceFile ls: #{ls}"
+        ls.select! { |l| !l.start_with?(/\s*#/) }
         iface_name = ls.find { |l| l.include? "iface.iscsi_ifacename" }.to_s
         log.info "InitIfaceFile ls: #{iface_name}"
-        unless iface_name.empty?
-          name = iface_name.gsub(/[[:space:]]/,'').split("=")[1]
-          dev_name = iface_value(ls, "iface.net_ifacename")
-          transport = iface_value(ls, "iface.transport")
-          hwaddress = iface_value(ls, "iface.hwaddress")
-          ipaddress = iface_value(ls, "iface.ipaddress")
-          @iface_file[name] = { :name => name, :dev => dev_name, :transport => transport, :hwaddress => hwaddress, :ip => ipaddress }
-        end
+        next if iface_name.empty?
+        name = iface_name.gsub(/[[:space:]]/, "").split("=")[1]
+        dev_name = iface_value(ls, "iface.net_ifacename")
+        transport = iface_value(ls, "iface.transport")
+        hwaddress = iface_value(ls, "iface.hwaddress")
+        ipaddress = iface_value(ls, "iface.ipaddress")
+        @iface_file[name] = { :name => name, :dev => dev_name, :transport => transport, :hwaddress => hwaddress, :ip => ipaddress }
       end
       log.info "InitIfaceFile iface_file: #{@iface_file}"
 
@@ -1314,7 +1312,7 @@ def default_item
     end
 
     def all_item
-      Item(Id(@offload[1][0]), @offload[1][1], @iface== @offload[1][0])
+      Item(Id(@offload[1][0]), @offload[1][1], @iface == @offload[1][0])
     end
 
     def iface_items
@@ -1359,8 +1357,8 @@ def LoadOffloadModules
     #
     # @return [Array<String>] List os iscsi ifaces for the current offload card selection, ex. ["eth2-bnx2i"]
     def GetDiscIfaces
-      ifaces = @iface == "all" ? @iface_file.keys : [@iface]
-      log.info "GetDiscIfaces:#{ifaces}" 
+      ifaces = (@iface == "all") ? @iface_file.keys : [@iface]
+      log.info "GetDiscIfaces:#{ifaces}"
       ifaces
     end
 
@@ -1483,7 +1481,7 @@ def InitOffloadValid
       end
 
       @offload_valid = potential_offload_cards
-      card_names = @offload_valid.values.flatten(1).map {|c| c["iface"] }.uniq
+      card_names = @offload_valid.values.flatten(1).map { |c| c["iface"] }.uniq
       bring_up(card_names)
       log.info "OffloadValid entries#{@offload_valid}"
       nil
@@ -1520,7 +1518,7 @@ def netcard_modules(card)
     def potential_offload_cards
       # Store into hw_mods information about all the cards in the system
       cards = SCR.Read(path(".probe.netcard"))
-      hw_mods = cards.select {|c| c["iscsioffload"] }.map do |c|
+      hw_mods = cards.select { |c| c["iscsioffload"] }.map do |c|
         log.info "GetOffloadItems card:#{c}"
         hw_mod = {
           "modules" => netcard_modules(c),
@@ -1564,7 +1562,7 @@ def configure_offload_engines(cards)
 
       cards.each do |dev_name|
         cmd = "#{OFFLOAD_SCRIPT} #{dev_name.shellescape} | grep ..:..:..:.." # grep for lines containing MAC address
-        log.info  "GetOffloadItems cmd #{cmd}"
+        log.info "GetOffloadItems cmd #{cmd}"
         out = SCR.Execute(path(".target.bash_output"), cmd)
         # Example for output if offload is supported on interface:
         # cmd: iscsi_offload eth2
diff --git a/test/data/chroot1/etc/iscsi/ifaces/bnx2i.ab:cd:de:fa:cf:29.ipv4.0 b/test/data/chroot1/etc/iscsi/ifaces/bnx2i.ab:cd:de:fa:cf:29.ipv4.0
new file mode 100644
index 0000000..6edc63c
--- /dev/null
+++ b/test/data/chroot1/etc/iscsi/ifaces/bnx2i.ab:cd:de:fa:cf:29.ipv4.0
@@ -0,0 +1,26 @@
+# BEGIN RECORD 2.1.9
+iface.iscsi_ifacename = bnx2i.ab:cd:de:fa:cf:29.ipv4.0
+iface.net_ifacename = eth0
+iface.ipaddress = 192.168.100.29
+iface.prefix_len = 0
+iface.hwaddress = ab:cd:de:fa:cf:29
+iface.transport_name = bnx2i
+iface.initiatorname = iqn.2015-02.com.hpe:oneview-93044906-8f80-4d1a-9ddb-5f4a70cb47b4
+iface.vlan_id = 0
+iface.vlan_priority = 0
+iface.iface_num = 0
+iface.mtu = 0
+iface.port = 0
+iface.subnet_mask = 255.255.255.0
+iface.tos = 0
+iface.ttl = 0
+iface.tcp_wsf = 0
+iface.tcp_timer_scale = 0
+iface.def_task_mgmt_timeout = 0
+iface.erl = 0
+iface.max_receive_data_len = 0
+iface.first_burst_len = 0
+iface.max_outstanding_r2t = 0
+iface.max_burst_len = 0
+# END RECORD
+
diff --git a/test/iscsi_client_lib_test.rb b/test/iscsi_client_lib_test.rb
index d97542a..c44d7e1 100644
--- a/test/iscsi_client_lib_test.rb
+++ b/test/iscsi_client_lib_test.rb
@@ -805,7 +805,7 @@ def config_values(data)
     end
   end
 
-  describe ".GetOffloadItems" do
+  describe ".InitIfaceFile" do
     around(:each) do |example|
       # The directory /etc/iscsi/ifaces/ is always scanned to look for interface definitions,
       # let's chroot the YaST agents so we can use an /etc directory with mocked content
@@ -813,13 +813,54 @@ def config_values(data)
       change_scr_root(root, &example)
     end
 
-    before do
-      # The agent .probe.netcard is used to inspect the network cards in the system, this
-      # intercepts that call and mocks the result based on the scenario we want to simulate
-      allow(Yast::SCR).to receive(:Read).and_call_original
-      allow(Yast::SCR).to receive(:Read).with(Yast::Path.new(".probe.netcard"))
-        .and_return probe_netcard
+    it "reads the existent iface files" do
+      expect(Yast::SCR).to receive(:Read).with(Yast.path(".target.dir"), "/etc/iscsi/ifaces").twice
+      subject.send(:InitIfaceFile)
+    end
+
+    context "there is no iface files" do
+      it "tries to generate them calling iscsiadm -m iface" do
+        expect(Yast::SCR).to receive(:Read).with(Yast.path(".target.dir"), "/etc/iscsi/ifaces").and_return([])
+        path = Yast::Path.new(".target.bash_output")
+        cmd = "LC_ALL=POSIX iscsiadm -m iface"
+        expect(Yast::SCR).to receive(:Execute).with(path, cmd)
+        allow(Yast::SCR).to receive(:Read).with(Yast.path(".target.dir"), "/etc/iscsi/ifaces").and_return([])
+        subject.send(:InitIfaceFile)
+      end
+    end
+
+    context "there are iface files" do
+      it "does not call iscsiadm -m iface" do
+        path = Yast::Path.new(".target.bash_output")
+        cmd = "LC_ALL=POSIX iscsiadm -m iface"
+        expect(Yast::SCR).to_not receive(:Execute).with(path, cmd)
+        subject.send(:InitIfaceFile)
+      end
+
+      it "populates the ifaces_file variable with the read data" do
+        iface_file = subject.instance_variable_get(:@iface_file)
+        expect(iface_file).to be_nil
+        subject.send(:InitIfaceFile)
+        iface_file = subject.instance_variable_get(:@iface_file)
+        iface, data = iface_file.first
+        expect(iface).to eq("bnx2i.ab:cd:de:fa:cf:29.ipv4.0")
+        expect(data[:name]).to eq("bnx2i.ab:cd:de:fa:cf:29.ipv4.0")
+        expect(data[:hwaddress]).to eq("ab:cd:de:fa:cf:29")
+        expect(data[:ip]).to eq("192.168.100.29")
+        expect(data[:transport]).to eq("bnx2i")
+      end
+    end
+  end
+
+  describe ".iface_items" do
+    around(:each) do |example|
+      # The directory /etc/iscsi/ifaces/ is always scanned to look for interface definitions,
+      # let's chroot the YaST agents so we can use an /etc directory with mocked content
+      root = File.join(File.dirname(__FILE__), "data", "chroot1")
+      change_scr_root(root, &example)
+    end
 
+    before do
       # iscsiadm is always called, mock it to find no active ISCSI interfaces by default
       mock_iscsiadm_mode([])
     end
@@ -843,7 +884,7 @@ def ui_item_label(item)
 
     RSpec.shared_examples "returns UI items" do
       it "returns an array of UI items" do
-        items = subject.GetOffloadItems
+        items = subject.iface_items
         expect(items).to be_an(Array)
         expect(items).to all be_a(Yast::Term)
         expect(items.map(&:value)).to all eq(:item)
@@ -852,132 +893,54 @@ def ui_item_label(item)
 
     RSpec.shared_examples "only default" do
       it "provides 'default' as the only item" do
-        items = subject.GetOffloadItems
+        items = subject.iface_items
         expect(items.size).to eq 1
         expect(ui_item_label(items.first)).to eq "default (Software)"
         expect(ui_item_id(items.first)).to eq "default"
       end
     end
 
-    context "with no network cards in the system" do
-      let(:probe_netcard) { [] }
-
-      include_examples "returns UI items"
-      include_examples "only default"
-    end
-
-    context "with network cards not expected to support offloading" do
-      let(:probe_netcard) do
-        [
-          probed_card("enp0s1", "r8152", "12:34:56:78:90:ab"),
-          probed_card("enp5s6", "tg3", "23:45:67:89:0a:bc")
-        ]
+    context "with no iscsi ifaces in the system" do
+      before do
+        allow(Yast::SCR).to receive(:Read).with(Yast.path(".target.dir"), "/etc/iscsi/ifaces")
+        mock_bash_out(/iscsiadm -m iface/, { "exit" => 0, "stdout" => "", "stderr" => "" })
       end
 
       include_examples "returns UI items"
       include_examples "only default"
     end
 
-    context "with several network cards that could support offloading" do
-      let(:probe_netcard) do
-        [
-          probed_card("p6p1_1", "qede",  "12:34:56:78:90:ab"),
-          probed_card("em1",    "bnx2x", "23:45:67:89:0a:bc"),
-          probed_card("p6p2_1", "qede",  "34:56:78:90:ab:cd"),
-          probed_card("em2",    "bnx2x", "45:67:89:0a:bc:de"),
-          probed_card("em3",    "bnx2x", "56:78:90:ab:cd:ef")
-        ]
-      end
-
-      context "if none of the cards support offloading" do
-        before do
-          mock_iscsi_offload("em1", false)
-          mock_iscsi_offload("em2", false)
-          mock_iscsi_offload("em3", false)
-          mock_iscsi_offload("p6p1_1", false)
-          mock_iscsi_offload("p6p2_1", false)
-        end
-
-        include_examples "returns UI items"
-
-        # NOTE: this is likely an unwanted behavior caused because
-        # @offload_valid == {2=>[], 7=>[]}
-        # which should be considered as an empty list but it's not
-        it "includes only 'default' and 'all'" do
-          labels = subject.GetOffloadItems.map { |i| ui_item_label(i) }
-          expect(labels).to contain_exactly("default (Software)", "all")
-        end
-      end
-
-      context "if some cards indeed support offloading" do
-        before do
-          mock_iscsi_offload("em1",    true, "23:45:67:89:0a:bc")
-          mock_iscsi_offload("em2",    false)
-          mock_iscsi_offload("em3",    true, "56:78:90:ab:cd:ef")
-          mock_iscsi_offload("p6p1_1", false)
-          mock_iscsi_offload("p6p2_1", true, "34:56:78:90:ab:cd")
-
-          mock_ip_addr("em1")
-          mock_ip_addr("em3")
-          mock_ip_addr("p6p2_1")
-        end
-
-        include_examples "returns UI items"
+    context "with iscsi ifaces in the system" do
+      include_examples "returns UI items"
 
-        it "includes 'default', 'all' and an entry for each offload card" do
-          items = subject.GetOffloadItems
+      it "includes 'default', 'all' and an entry for each offload card" do
+          items = subject.iface_items
 
           labels = items.map { |i| ui_item_label(i) }
           expect(labels).to contain_exactly(
-            "default (Software)", "all", "em1 - 23:45:67:89:0a:bc - bnx2/bnx2i/bnx2x",
-            "em3 - 56:78:90:ab:cd:ef - bnx2/bnx2i/bnx2x", "p6p2_1 - 34:56:78:90:ab:cd - qede/qedi"
+            "default (Software)", "all", "bnx2i.ab:cd:de:fa:cf:29.ipv4.0 - 192.168.100.29"
           )
 
           ids = items.map { |i| ui_item_id(i) }
-          expect(ids).to contain_exactly("default", "all", "em1-bnx2i", "em3-bnx2i", "p6p2_1-qedi")
-        end
-
-        context "and ip is not found" do
-          # NOTE: testing the state of internal variables should be out of the scope of unit tests,
-          # but we want to prove a point here (see next context right below)
-          it "sets the IPs in @offload_valid to 'unknown'" do
-            subject.GetOffloadItems
-            cards = subject.instance_variable_get("@offload_valid").values.flatten(1)
-            expect(cards.map(&:last)).to eq ["unknown", "unknown", "unknown"]
-          end
-        end
-
-        context "and ip is found" do
-          before do
-            mock_ip_addr("em1",    "192.10.9.8/24")
-            mock_ip_addr("em3",    "")
-            mock_ip_addr("p6p2_1", "10.11.12.13/24")
-          end
-
-          it "sets the IPs in @offload_valid to 'unknown'" do
-            subject.GetOffloadItems
-            cards = subject.instance_variable_get("@offload_valid").values.flatten(1)
-            expect(cards.map(&:last)).to eq ["192.10.9.8", "unknown", "10.11.12.13"]
-          end
+          expect(ids).to contain_exactly("default", "all", "bnx2i.ab:cd:de:fa:cf:29.ipv4.0")
         end
+    end
 
-        context "and no active card is reported by iscsiadm " do
-          before { mock_iscsiadm_mode([]) }
+    context "and no iscsi iface is reported by iscsiadm " do
+      before { mock_iscsiadm_mode([]) }
 
-          it "pre-selects the item 'default'" do
-            selected = subject.GetOffloadItems.find { |i| i.params[2] }
-            expect(ui_item_id(selected)).to eq "default"
-          end
-        end
+      it "pre-selects the item 'default'" do
+        selected = subject.iface_items.find { |i| i.params[2] }
+        expect(ui_item_id(selected)).to eq "default"
+      end
+    end
 
-        context "and one card is already associated to the first target" do
-          before { mock_iscsiadm_mode(["em3-bnx2i"]) }
+    context "and one card is already associated to the first target" do
+      before { mock_iscsiadm_mode(["bnx2i.ab:cd:de:fa:cf:29.ipv4.0"]) }
 
-          it "selects by default the current offload card" do
-            selected = subject.GetOffloadItems.find { |i| i.params[2] }
-            expect(ui_item_id(selected)).to eq "em3-bnx2i"
-          end
-        end
+      it "selects by default the current iface" do
+        selected = subject.iface_items.find { |i| i.params[2] }
+        expect(ui_item_id(selected)).to eq "bnx2i.ab:cd:de:fa:cf:29.ipv4.0"
       end
     end
   end

From fe56d2db5a7bfde35edbbff63bedf47732e2e628 Mon Sep 17 00:00:00 2001
From: Knut Anderssen <kanderssen@suse.com>
Date: Thu, 31 Oct 2024 11:06:44 +0000
Subject: [PATCH 06/15] Increase the min width for the iface selection

---
 src/include/iscsi-client/dialogs.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/include/iscsi-client/dialogs.rb b/src/include/iscsi-client/dialogs.rb
index 670543c..1f1fbd9 100644
--- a/src/include/iscsi-client/dialogs.rb
+++ b/src/include/iscsi-client/dialogs.rb
@@ -83,12 +83,12 @@ def widgets
           "widget"            => :custom,
           "custom_widget"     => HBox(
             MinWidth(
-              16,
+              72,
               HBox(
                 # name of iscsi client (/etc/iscsi/initiatorname.iscsi)
                 TextEntry(Id(:initiator_name), _("&Initiator Name")),
                 MinWidth(
-                  8,
+                  36,
                   ComboBox(
                     Id(:iface),
                     Opt(:notify),

From 7e21b889a863335f0e366f20dbc300061acda3fd Mon Sep 17 00:00:00 2001
From: Knut Anderssen <kanderssen@suse.com>
Date: Thu, 31 Oct 2024 11:20:42 +0000
Subject: [PATCH 07/15] Some rubocop fixes

---
 src/modules/IscsiClientLib.rb |  6 +-----
 test/iscsi_client_lib_test.rb | 16 ++++++++--------
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/src/modules/IscsiClientLib.rb b/src/modules/IscsiClientLib.rb
index 7569eee..616e640 100644
--- a/src/modules/IscsiClientLib.rb
+++ b/src/modules/IscsiClientLib.rb
@@ -1214,17 +1214,13 @@ def autoyastWrite
         ifacepar << " " unless ifacepar.empty?
         ifacepar << "-I " << iface.shellescape
         ifaces << iface
-      end
 
-      @ay_settings.fetch("targets", []).each do |target|
         next if portals.include? target["portal"]
         SCR.Execute(
           path(".target.bash"),
           GetAdmCmd(%(-m discovery #{ifacepar} -t st -p #{target["portal"].shellescape}))
         )
         portals << target["portal"]
-      end
-      @ay_settings.fetch("targets", []).each do |target|
         log.info "login into target #{target}"
         loginIntoTarget(target)
         @currentRecord = [target["portal"], target["target"], target["iface"]]
@@ -1291,7 +1287,7 @@ def InitIfaceFile
       files.each do |file|
         ls = SCR.Read(path(".target.string"), "/etc/iscsi/ifaces/#{file}").split("\n")
         log.info "InitIfaceFile file: #{file}\nInitIfaceFile ls: #{ls}"
-        ls.select! { |l| !l.start_with?(/\s*#/) }
+        ls.reject! { |l| l.start_with?(/\s*#/) }
         iface_name = ls.find { |l| l.include? "iface.iscsi_ifacename" }.to_s
         log.info "InitIfaceFile ls: #{iface_name}"
         next if iface_name.empty?
diff --git a/test/iscsi_client_lib_test.rb b/test/iscsi_client_lib_test.rb
index c44d7e1..aee6598 100644
--- a/test/iscsi_client_lib_test.rb
+++ b/test/iscsi_client_lib_test.rb
@@ -914,16 +914,16 @@ def ui_item_label(item)
       include_examples "returns UI items"
 
       it "includes 'default', 'all' and an entry for each offload card" do
-          items = subject.iface_items
+        items = subject.iface_items
 
-          labels = items.map { |i| ui_item_label(i) }
-          expect(labels).to contain_exactly(
-            "default (Software)", "all", "bnx2i.ab:cd:de:fa:cf:29.ipv4.0 - 192.168.100.29"
-          )
+        labels = items.map { |i| ui_item_label(i) }
+        expect(labels).to contain_exactly(
+          "default (Software)", "all", "bnx2i.ab:cd:de:fa:cf:29.ipv4.0 - 192.168.100.29"
+        )
 
-          ids = items.map { |i| ui_item_id(i) }
-          expect(ids).to contain_exactly("default", "all", "bnx2i.ab:cd:de:fa:cf:29.ipv4.0")
-        end
+        ids = items.map { |i| ui_item_id(i) }
+        expect(ids).to contain_exactly("default", "all", "bnx2i.ab:cd:de:fa:cf:29.ipv4.0")
+      end
     end
 
     context "and no iscsi iface is reported by iscsiadm " do

From 5888ca59dab1660ab7d8cddaab11bedb2440c7c5 Mon Sep 17 00:00:00 2001
From: Knut Anderssen <kanderssen@suse.com>
Date: Thu, 31 Oct 2024 11:35:00 +0000
Subject: [PATCH 08/15] We cannot combine both loops

---
 src/modules/IscsiClientLib.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/modules/IscsiClientLib.rb b/src/modules/IscsiClientLib.rb
index 616e640..e2e6b0a 100644
--- a/src/modules/IscsiClientLib.rb
+++ b/src/modules/IscsiClientLib.rb
@@ -1214,7 +1214,10 @@ def autoyastWrite
         ifacepar << " " unless ifacepar.empty?
         ifacepar << "-I " << iface.shellescape
         ifaces << iface
+      end
 
+      # rubocop:disable Style/CombinableLoops
+      @ay_settings.fetch("targets", []).each do |target|
         next if portals.include? target["portal"]
         SCR.Execute(
           path(".target.bash"),
@@ -1226,6 +1229,7 @@ def autoyastWrite
         @currentRecord = [target["portal"], target["target"], target["iface"]]
         setStartupStatus(target.fetch("startup", "manual"))
       end
+      # rubocop:enable Style/CombinableLoops
       true
     end
 

From b711d60fc00df59397fcf1663efb6c28076590b8 Mon Sep 17 00:00:00 2001
From: Knut Anderssen <kanderssen@suse.com>
Date: Thu, 31 Oct 2024 11:42:13 +0000
Subject: [PATCH 09/15] Fix yardoc

---
 src/modules/IscsiClientLib.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/modules/IscsiClientLib.rb b/src/modules/IscsiClientLib.rb
index e2e6b0a..fb0da92 100644
--- a/src/modules/IscsiClientLib.rb
+++ b/src/modules/IscsiClientLib.rb
@@ -1372,7 +1372,7 @@ def GetDiscIfaces
     # @param ip [String] Portal IP address
     # @param port [String] Portal port number
     # @param use_fw [Boolean] whether the target should be fw or not
-    # @only_new [Boolean] whether a new record should be created
+    # @param only_new [Boolean] whether a new record should be created
     # @return [Array<String>]
     def GetDiscoveryCmd(ip, port, use_fw: false, only_new: false)
       log.info "GetDiscoveryCmd ip:#{ip} port:#{port} fw:#{use_fw} only new:#{only_new}"

From 743cfb9aaa50c9ff7bd26516e38c4d376b5ba0aa Mon Sep 17 00:00:00 2001
From: Knut Anderssen <kanderssen@suse.com>
Date: Thu, 31 Oct 2024 11:48:30 +0000
Subject: [PATCH 10/15] Bump version & changelog

---
 package/yast2-iscsi-client.changes | 9 +++++++++
 package/yast2-iscsi-client.spec    | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/package/yast2-iscsi-client.changes b/package/yast2-iscsi-client.changes
index e6edd99..bf880fb 100644
--- a/package/yast2-iscsi-client.changes
+++ b/package/yast2-iscsi-client.changes
@@ -1,3 +1,12 @@
+-------------------------------------------------------------------
+Thu Oct 31 11:46:26 UTC 2024 - Knut Anderssen <kanderssen@suse.com>
+
+- Fixes for bsc##1231385
+   - Do not call iscsi_offload.sh script anymore using the iscsi 
+     ifaces created by autoLogOn directly and exposing them in the
+     UI instead of the offload card selection.
+- 4.6.4
+
 -------------------------------------------------------------------
 Fri Sep 27 14:37:24 UTC 2024 - Knut Anderssen <kanderssen@suse.com>
 
diff --git a/package/yast2-iscsi-client.spec b/package/yast2-iscsi-client.spec
index df7e450..446db0d 100644
--- a/package/yast2-iscsi-client.spec
+++ b/package/yast2-iscsi-client.spec
@@ -17,7 +17,7 @@
 
 
 Name:           yast2-iscsi-client
-Version:        4.6.3
+Version:        4.6.4
 Release:        0
 Summary:        YaST2 - iSCSI Client Configuration
 License:        GPL-2.0-only

From c7601d9aa891720640a8e6fb1c2b392646a3a72d Mon Sep 17 00:00:00 2001
From: Knut Anderssen <kanderssen@suse.com>
Date: Mon, 4 Nov 2024 09:40:37 +0000
Subject: [PATCH 11/15] Added iSCSI Iface help

---
 src/include/iscsi-client/dialogs.rb | 2 +-
 src/include/iscsi-client/helps.rb   | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/include/iscsi-client/dialogs.rb b/src/include/iscsi-client/dialogs.rb
index 1f1fbd9..68c2bf6 100644
--- a/src/include/iscsi-client/dialogs.rb
+++ b/src/include/iscsi-client/dialogs.rb
@@ -113,7 +113,7 @@ def widgets
             method(:handleIface),
             "symbol (string, map)"
           ),
-          "help"              => Ops.get_string(@HELPS, "initiator_name", "")
+          "help"              => @HELPS["initiator_name"].to_s
         },
         # table of connected targets
         "connected_table"  => {
diff --git a/src/include/iscsi-client/helps.rb b/src/include/iscsi-client/helps.rb
index 632fc65..63af156 100644
--- a/src/include/iscsi-client/helps.rb
+++ b/src/include/iscsi-client/helps.rb
@@ -120,7 +120,10 @@ def initialize_iscsi_client_helps(_include_target)
           ),
         "initiator_name" => _(
           "<p><b>Initiator Name</b> is a value from <tt>/etc/iscsi/initiatorname.iscsi</tt>. \nIn case you have iBFT, this value will be added from there and you are only able to change it in the BIOS setup.</p>"
-        ),
+        ) +
+          _(
+            "<p><b>iSCSI Iface</b> allows to select an specific iSCSI iface to be used for discovering targets.</p>"
+          ),
         "isns"           => _(
           "If you want to use <b>iSNS</b> (Internet  Storage  Name Service) for discovering targets instead of the default SendTargets method,\nfill in the IP address of the iSNS server and port. The default port should be 3205.\n"
         ),

From be44f899d974c2498bd1c8603385b4aa0b32ec1a Mon Sep 17 00:00:00 2001
From: Knut Anderssen <kanderssen@suse.com>
Date: Mon, 4 Nov 2024 09:57:23 +0000
Subject: [PATCH 12/15] Dropping not used anymore method

---
 src/modules/IscsiClientLib.rb | 34 ----------------------------------
 1 file changed, 34 deletions(-)

diff --git a/src/modules/IscsiClientLib.rb b/src/modules/IscsiClientLib.rb
index fb0da92..f82b9b7 100644
--- a/src/modules/IscsiClientLib.rb
+++ b/src/modules/IscsiClientLib.rb
@@ -1549,40 +1549,6 @@ def potential_offload_cards
       result
     end
 
-    # Configures iSCSI offload engines for the given cards
-    #
-    # Tries to create an open-iscsi interface definition for each of the given cards.
-    #
-    # @param cards [Array<String>] list of interface names
-    # @return [Hash{String => Hash}] results of the operation, keys are the names of the interfaces
-    #   and values the corresponding result as a hash with three fields: "exit" (0 means the
-    #   definition was created), "hwaddr" and "ntype".
-    def configure_offload_engines(cards)
-      offload_res = {}
-
-      cards.each do |dev_name|
-        cmd = "#{OFFLOAD_SCRIPT} #{dev_name.shellescape} | grep ..:..:..:.." # grep for lines containing MAC address
-        log.info "GetOffloadItems cmd #{cmd}"
-        out = SCR.Execute(path(".target.bash_output"), cmd)
-        # Example for output if offload is supported on interface:
-        # cmd: iscsi_offload eth2
-        # out: $["exit":0, "stderr":"", "stdout":"00:00:c9:b1:bc:7f ip \n"]
-        cmd2 = "#{OFFLOAD_SCRIPT} #{dev_name.shellescape}"
-        result = SCR.Execute(path(".target.bash_output"), cmd2)
-        log.info "GetOffloadItems iscsi_offload out:#{result}"
-
-        offload_res[dev_name] = {}
-        offload_res[dev_name]["exit"] = out["exit"]
-        next unless out["exit"].zero?
-
-        sl = Builtins.splitstring(out["stdout"], " \n")
-        offload_res[dev_name]["hwaddr"] = sl[0]
-        offload_res[dev_name]["ntype"] = sl[1]
-      end
-
-      offload_res
-    end
-
     # Current IP address of the given network interface
     def ip_addr(dev_name)
       stdout = Yast::Execute.on_target!("ip", "addr", "show", dev_name,

From 22fe2d342a290cac1cb43ca8ad1dbdd16dd0d378 Mon Sep 17 00:00:00 2001
From: Knut Anderssen <kanderssen@suse.com>
Date: Tue, 5 Nov 2024 11:19:37 +0000
Subject: [PATCH 13/15] Changes based on CR.

---
 package/yast2-iscsi-client.changes | 2 +-
 src/modules/IscsiClientLib.rb      | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/package/yast2-iscsi-client.changes b/package/yast2-iscsi-client.changes
index bf880fb..efbf630 100644
--- a/package/yast2-iscsi-client.changes
+++ b/package/yast2-iscsi-client.changes
@@ -1,7 +1,7 @@
 -------------------------------------------------------------------
 Thu Oct 31 11:46:26 UTC 2024 - Knut Anderssen <kanderssen@suse.com>
 
-- Fixes for bsc##1231385
+- Fixes for bsc#1231385
    - Do not call iscsi_offload.sh script anymore using the iscsi 
      ifaces created by autoLogOn directly and exposing them in the
      UI instead of the offload card selection.
diff --git a/src/modules/IscsiClientLib.rb b/src/modules/IscsiClientLib.rb
index f82b9b7..4bfce57 100644
--- a/src/modules/IscsiClientLib.rb
+++ b/src/modules/IscsiClientLib.rb
@@ -266,6 +266,7 @@ def selected_iface
 
     def iface=(iface)
       log.info "Selecting the iface: #{iface} cur:#{@iface}"
+      @iface = iface
     end
 
     # Create and return complete iscsciadm command by adding the string

From 1b802cdc7625110f06b2d9ae65e7be2e4217763d Mon Sep 17 00:00:00 2001
From: Knut Anderssen <kanderssen@suse.com>
Date: Tue, 5 Nov 2024 12:03:10 +0000
Subject: [PATCH 14/15] Force a read of ifaces in case of autoLogon

---
 src/clients/inst_iscsi-client.rb |  5 +++--
 src/modules/IscsiClientLib.rb    | 10 ++++++----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/clients/inst_iscsi-client.rb b/src/clients/inst_iscsi-client.rb
index 7825c52..d5ec036 100644
--- a/src/clients/inst_iscsi-client.rb
+++ b/src/clients/inst_iscsi-client.rb
@@ -69,12 +69,13 @@ def main
       )
       # check initiator name, create if not exists
       IscsiClientLib.checkInitiatorName
-
-      IscsiClientLib.getiBFT
       IscsiClientLib.LoadOffloadModules
+      IscsiClientLib.getiBFT
 
       # try auto login to target
       auto_login = IscsiClientLib.autoLogOn
+      # force a read of ifaces
+      IscsiClientLib.read_ifaces if auto_login
       # force a read of sessions in case of auto_login (bsc#1228084)
       IscsiClientLib.readSessions if auto_login
 
diff --git a/src/modules/IscsiClientLib.rb b/src/modules/IscsiClientLib.rb
index 4bfce57..097bfc1 100644
--- a/src/modules/IscsiClientLib.rb
+++ b/src/modules/IscsiClientLib.rb
@@ -1278,6 +1278,11 @@ def iface_value(content, field)
       content.find { |l| l.include? field }.to_s.gsub(/[[:space:]]/, "").split("=")[1]
     end
 
+    def read_ifaces
+      InitIfaceFile()
+      InitIface()
+    end
+
     def InitIfaceFile
       @iface_file = {}
       files = SCR.Read(path(".target.dir"), "/etc/iscsi/ifaces") || []
@@ -1476,10 +1481,7 @@ def bring_up(card_names)
     end
 
     def InitOffloadValid
-      if @iface_file.nil?
-        InitIfaceFile()
-        InitIface()
-      end
+      read_ifaces if @iface_file.nil?
 
       @offload_valid = potential_offload_cards
       card_names = @offload_valid.values.flatten(1).map { |c| c["iface"] }.uniq

From 8b339646c6e49f07d8be693afe73738cfff66228 Mon Sep 17 00:00:00 2001
From: Knut Anderssen <kanderssen@suse.com>
Date: Fri, 8 Nov 2024 09:34:59 +0000
Subject: [PATCH 15/15] Changes based on CR

---
 src/modules/IscsiClientLib.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/modules/IscsiClientLib.rb b/src/modules/IscsiClientLib.rb
index 097bfc1..9ea14a7 100644
--- a/src/modules/IscsiClientLib.rb
+++ b/src/modules/IscsiClientLib.rb
@@ -265,7 +265,8 @@ def selected_iface
     end
 
     def iface=(iface)
-      log.info "Selecting the iface: #{iface} cur:#{@iface}"
+      return if @iface == iface
+      log.info "Changing the iface from #{iface} to #{@iface}"
       @iface = iface
     end