RE: [PATCH net-next] qed: Utilize FW 8.20.0.0

[Mintz, Yuval]

> >> This pushes qed [and as result, all qed* drivers] into using 8.20.0.0
> >> firmware. The changes are mostly contained in qed with minor changes
> >> to qedi due to some HSI changes.
> >>
> >> Content-wise, the firmware contains fixes to various issues exposed
> >> since the release of the previous firmware, including:
> >>  - Corrects iSCSI fast retransmit when data digest is enabled.
> >>  - Stop draining packets when receiving several consecutive PFCs.
> >>  - Prevent possible assertion when consecutively opening/closing
> >>many connections.
> >>  - Prevent possible assertion due to too long BDQ fetch time.
> >>
> >> In addition, the new firmware would allow us to later add iWARP
> >> support in qed and qedr.
> >>
> >> Signed-off-by: Chad Dupuis 
> >> Signed-off-by: Ram Amrani 
> >> Signed-off-by: Tomer Tayar 
> >> Signed-off-by: Manish Rangankar 
> >> Signed-off-by: Yuval Mintz 
> >
> > Applied.
> 
> Actually I had to revert.  Please look at the compiler output before
> submitting changes:
> 
> drivers/net/ethernet/qlogic/qed/qed_debug.c: In function ‘qed_grc_dump’:
> drivers/net/ethernet/qlogic/qed/qed_debug.c:2425:6: warning: ‘addr’ may
> be used uninitialized in this function [-Wmaybe-uninitialized]
>   u32 byte_addr = DWORDS_TO_BYTES(addr), offset = 0, i;
>   ^
> drivers/net/ethernet/qlogic/qed/qed_debug.c:3534:7: note: ‘addr’ was
> declared here
>u32 addr, size = RSS_REG_RSS_RAM_DATA_SIZE;
>^
> 
> 'addr' is never, ever, assigned a value, yet it is passed into a function as 
> an
> argument.

Sorry about that. Will send v2 [hopefully] later today.

RE: [PATCH net-next] qed*: Utilize Firmware 8.15.3.0

[Mintz, Yuval]

> > We can't just require a new firmware version in the driver, as users
> > most likely won't have it by the time they install the new kernel.  So
> > you'll have to support the old firmware version as well.
> 
> Why not? That has been the paradigm forever.
> 
> The new firmware version is already available in linux-firmware.
> In any reasonable distro that would update the driver, you'd expect they'd
> also update the firmware version on their filesystem.

Just in case it wasn't clear from the original message - 
The firmware discussed here is the binary firmware, not the
management firmware.
For the management firmware [on persistent storage] there is
a backward/forward compatibility scheme in place.

RE: [PATCH net-next] qed*: Utilize Firmware 8.15.3.0

[Mintz, Yuval]

> We can't just require a new firmware version in the driver, as users most
> likely won't have it by the time they install the new kernel.  So you'll have 
> to
> support the old firmware version as well.

Why not? That has been the paradigm forever.

The new firmware version is already available in linux-firmware.
In any reasonable distro that would update the driver, you'd expect
they'd also update the firmware version on their filesystem.

RE: [Open-FCoE] [PATCH RFC net-next 1/5] qed: Add support for hardware offloaded FCoE.

[Mintz, Yuval]

> > +struct fcoe_tstorm_fcoe_task_st_ctx_read_write {
> > +   union fcoe_cleanup_addr_exp_ro_union
> cleanup_addr_exp_ro_union;
> > +   __le16 flags;
> > +#define
> FCOE_TSTORM_FCOE_TASK_ST_CTX_READ_WRITE_RX_SGL_MODE_MASK
> 0x7
> > +#define
> FCOE_TSTORM_FCOE_TASK_ST_CTX_READ_WRITE_RX_SGL_MODE_SHIFT  0
> > +#define
> FCOE_TSTORM_FCOE_TASK_ST_CTX_READ_WRITE_EXP_FIRST_FRAME_MASK
> 0x1
> > +#define
> FCOE_TSTORM_FCOE_TASK_ST_CTX_READ_WRITE_EXP_FIRST_FRAME_SHIFT
> 3
> > +#define
> FCOE_TSTORM_FCOE_TASK_ST_CTX_READ_WRITE_SEQ_ACTIVE_MASK
> 0x1
> > +#define
> FCOE_TSTORM_FCOE_TASK_ST_CTX_READ_WRITE_SEQ_ACTIVE_SHIFT   4
> > +#define
> FCOE_TSTORM_FCOE_TASK_ST_CTX_READ_WRITE_SEQ_TIMEOUT_MASK
> 0x1
> > +#define
> FCOE_TSTORM_FCOE_TASK_ST_CTX_READ_WRITE_SEQ_TIMEOUT_SHIFT  5
> > +#define
> FCOE_TSTORM_FCOE_TASK_ST_CTX_READ_WRITE_SINGLE_PKT_IN_EX_MASK
> 0x1
> > +#define
> FCOE_TSTORM_FCOE_TASK_ST_CTX_READ_WRITE_SINGLE_PKT_IN_EX_SHIFT
> 6
> > +#define
> FCOE_TSTORM_FCOE_TASK_ST_CTX_READ_WRITE_OOO_RX_SEQ_STAT_MAS
> K   0x1
> > +#define
> FCOE_TSTORM_FCOE_TASK_ST_CTX_READ_WRITE_OOO_RX_SEQ_STAT_SHIFT
> 7
> > +#define
> FCOE_TSTORM_FCOE_TASK_ST_CTX_READ_WRITE_CQ_ADD_ADV_MASK
> 0x3
> > +#define
> FCOE_TSTORM_FCOE_TASK_ST_CTX_READ_WRITE_CQ_ADD_ADV_SHIFT   8
> > +#define FCOE_TSTORM_FCOE_TASK_ST_CTX_READ_WRITE_RSRV1_MASK
> 0x3F
> > +#define FCOE_TSTORM_FCOE_TASK_ST_CTX_READ_WRITE_RSRV1_SHIFT
> 10
> 
> A very odd way of defining a bitfield ...
> Why not use a 'normal' bitfield here?

This is the format of our generated firmware HSI; We already have
Thousands of definitions using this same format.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RFC 2/6] qed: Add iSCSI out of order packet handling.

[Mintz, Yuval]

> > This patch adds out of order packet handling for hardware offloaded
> > iSCSI. Out of order packet handling requires driver buffer allocation
> > and assistance.
> >
> > Signed-off-by: Arun Easi 
> > Signed-off-by: Yuval Mintz 
> >
> Hmm. The entire out-of-order handling is pretty generic. I really wonder
> if this doesn't apply to iSCSI in general; surely iscsi_tcp suffers from
> the same problem, no?
> If so, wouldn't it be better to move it into generic (iSCSI) code so
> that all implementations would benefit from it?

[disclaimer - I'm far from knowledgeable in iscsi ]

I agree that the problem of out-of-order handling is probably generic,
but our solution is very device oriented.
As the device lacks [a lot of] internal memory, it uses the host memory
for out-of-order buffers and the driver assistance in pushing them when
they are needed.
>From driver perspective, all the data is completely opaque; All it does is
follow the firmware's guidance in storing & re-transmitting buffers when
required.

Now, I guess the logic could be divided between hardware-specifics -
Interaction with 'client' [in our case, device's firmware], to receive
new data, instructions regarding placement and re-transmission,
and a lower generic data structure which supports manipulation of
buffers [push-left, push-right, join, etc.].

But given that the data-structure would completely lacks all
protocol-knowledge [as our implementation doesn't have nor require such],
I think there would be very little gain - we might find out that as much
as 80% of the code is device interaction, and the remaining so-called
'generic' data-structure won't be that useful to other clients as it was
closely tied to our device needs and API.

Either way, placing this under iscsi would probably be insufficient for our
future needs, as our qed-iwarp driver would also require this functionality.

Thanks,
Yuval



--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html